Thanks for reviewing this patch.
On Fri, Aug 01, 2008 at 10:02:50AM +0200, Jim Meyering wrote:
> What if either contains shell meta-characters?
> To accommodate you'd have to shell-quote as needed, or (as I prefer)
> simply detect the bogosity and refuse to run the command.
Yes, not quite sure what I was thinking of there. The new version
should reject characters in both the editor & filename which are
not in a small recognized set of characters.
> You can mark entries as "const":
> static const vshCmdOptDef opts_edit[] = {
This gives an error because the vshCmdDef array where we aggregate all
of these doesn't expect the const pointer. Obviously it's a general
const-correctness problem which needs a separate patch to fix
throughout the whole file.
A new patch is attached which should address everything that you
mentioned.
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
Index: docs/virsh.pod
===================================================================
RCS file: /data/cvs/libvirt/docs/virsh.pod,v
retrieving revision 1.16
diff -u -r1.16 virsh.pod
--- docs/virsh.pod 15 May 2008 06:12:32 -0000 1.16
+++ docs/virsh.pod 1 Aug 2008 10:33:22 -0000
@@ -277,6 +277,19 @@
Output the domain information as an XML dump to stdout, this format can be used by the B<create> command.
+=item B<edit> I<domain-id>
+
+Edit the XML configuration file for a domain.
+
+This is equivalent to:
+ virsh dumpxml domain > domain.xml
+ edit domain.xml
+ virsh define domain.xml
+except that it does some error checking.
+
+The editor used can be supplied by the C<$EDITOR> environment
+variable, or if that is not defined defaults to C<vi>.
+
=item B<migrate> optional I<--live> I<domain-id> I<desturi> I<migrateuri>
Migrate domain to another host. Add --live for live migration. The I<desturi>
@@ -480,6 +493,19 @@
Output the virtual network information as an XML dump to stdout.
+=item B<net-edit> I<network>
+
+Edit the XML configuration file for a network.
+
+This is equivalent to:
+ virsh net-dumpxml network > network.xml
+ edit network.xml
+ virsh define network.xml
+except that it does some error checking.
+
+The editor used can be supplied by the C<$EDITOR> environment
+variable, or if that is not defined defaults to C<vi>.
+
=item B<net-list> optional I<--inactive> or I<--all>
Returns the list of active networks, if I<--all> is specified this will also
Index: src/Makefile.am
===================================================================
RCS file: /data/cvs/libvirt/src/Makefile.am,v
retrieving revision 1.86
diff -u -r1.86 Makefile.am
--- src/Makefile.am 11 Jul 2008 16:23:36 -0000 1.86
+++ src/Makefile.am 1 Aug 2008 10:33:22 -0000
@@ -138,6 +138,31 @@
virsh_DEPENDENCIES = $(DEPS)
virsh_LDADD = $(LDADDS) $(VIRSH_LIBS)
virsh_CFLAGS = $(COVERAGE_CFLAGS) $(READLINE_CFLAGS)
+BUILT_SOURCES = virsh-net-edit.c virsh-pool-edit.c
+EXTRA_DIST += virsh-net-edit.c virsh-pool-edit.c
+
+virsh-net-edit.c: virsh.c Makefile.am
+ echo '/* Automatically generated from the Makefile and virsh.c */' > $@
+ echo 'static int' >> $@
+ awk '/^cmdEdit/, /^}/' $< | \
+ sed -e 's/domain/network/g' \
+ -e 's/Domain/Network/g' \
+ -e 's/cmdEdit/cmdNetworkEdit/g' \
+ -e 's/dom/network/g' \
+ >> $@
+
+virsh-pool-edit.c: virsh.c Makefile.am
+ echo '/* Automatically generated from the Makefile and virsh.c */' > $@
+ echo 'static int' >> $@
+ awk '/^cmdEdit/, /^}/' $< | \
+ sed -e 's/domain/pool/g' \
+ -e 's/vshCommandOptDomain/vshCommandOptPool/g' \
+ -e 's/Domain %s/Pool %s/g' \
+ -e 's/Domain/StoragePool/g' \
+ -e 's/cmdEdit/cmdPoolEdit/g' \
+ -e 's/\(virStoragePoolDefineXML.*\));/\1, 0);/' \
+ -e 's/dom/pool/g' \
+ >> $@
if WITH_STORAGE_DISK
if WITH_LIBVIRTD
Index: src/virsh-net-edit.c
===================================================================
RCS file: src/virsh-net-edit.c
diff -N src/virsh-net-edit.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ src/virsh-net-edit.c 1 Aug 2008 10:33:22 -0000
@@ -0,0 +1,85 @@
+/* Automatically generated from the Makefile and virsh.c */
+static int
+cmdNetworkEdit (vshControl *ctl, vshCmd *cmd)
+{
+ int ret = FALSE;
+ virNetworkPtr network = NULL;
+ char *tmp = NULL;
+ char *doc = NULL;
+ char *doc_edited = NULL;
+ char *doc_reread = NULL;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ goto cleanup;
+
+ network = vshCommandOptNetwork (ctl, cmd, "network", NULL);
+ if (network == NULL)
+ goto cleanup;
+
+ /* Get the XML configuration of the network. */
+ doc = virNetworkGetXMLDesc (network, 0);
+ if (!doc)
+ goto cleanup;
+
+ /* Create and open the temporary file. */
+ tmp = editWriteToTempFile (ctl, doc);
+ if (!tmp) goto cleanup;
+
+ /* Start the editor. */
+ if (editFile (ctl, tmp) == -1) goto cleanup;
+
+ /* Read back the edited file. */
+ doc_edited = editReadBackFile (ctl, tmp);
+ if (!doc_edited) goto cleanup;
+
+ unlink (tmp);
+ tmp = NULL;
+
+ /* Compare original XML with edited. Has it changed at all? */
+ if (STREQ (doc, doc_edited)) {
+ vshPrint (ctl, _("Network %s XML configuration not changed.\n"),
+ virNetworkGetName (network));
+ ret = TRUE;
+ goto cleanup;
+ }
+
+ /* Now re-read the network XML. Did someone else change it while
+ * it was being edited? This also catches problems such as us
+ * losing a connection or the network going away.
+ */
+ doc_reread = virNetworkGetXMLDesc (network, 0);
+ if (!doc_reread)
+ goto cleanup;
+
+ if (STRNEQ (doc, doc_reread)) {
+ vshError (ctl, FALSE,
+ _("ERROR: the XML configuration was changed by another user"));
+ goto cleanup;
+ }
+
+ /* Everything checks out, so redefine the network. */
+ virNetworkFree (network);
+ network = virNetworkDefineXML (ctl->conn, doc_edited);
+ if (!network)
+ goto cleanup;
+
+ vshPrint (ctl, _("Network %s XML configuration edited.\n"),
+ virNetworkGetName(network));
+
+ ret = TRUE;
+
+ cleanup:
+ if (network)
+ virNetworkFree (network);
+
+ free (doc);
+ free (doc_edited);
+ free (doc_reread);
+
+ if (tmp) {
+ unlink (tmp);
+ free (tmp);
+ }
+
+ return ret;
+}
Index: src/virsh-pool-edit.c
===================================================================
RCS file: src/virsh-pool-edit.c
diff -N src/virsh-pool-edit.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ src/virsh-pool-edit.c 1 Aug 2008 10:33:22 -0000
@@ -0,0 +1,85 @@
+/* Automatically generated from the Makefile and virsh.c */
+static int
+cmdPoolEdit (vshControl *ctl, vshCmd *cmd)
+{
+ int ret = FALSE;
+ virStoragePoolPtr pool = NULL;
+ char *tmp = NULL;
+ char *doc = NULL;
+ char *doc_edited = NULL;
+ char *doc_reread = NULL;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ goto cleanup;
+
+ pool = vshCommandOptPool (ctl, cmd, "pool", NULL);
+ if (pool == NULL)
+ goto cleanup;
+
+ /* Get the XML configuration of the pool. */
+ doc = virStoragePoolGetXMLDesc (pool, 0);
+ if (!doc)
+ goto cleanup;
+
+ /* Create and open the temporary file. */
+ tmp = editWriteToTempFile (ctl, doc);
+ if (!tmp) goto cleanup;
+
+ /* Start the editor. */
+ if (editFile (ctl, tmp) == -1) goto cleanup;
+
+ /* Read back the edited file. */
+ doc_edited = editReadBackFile (ctl, tmp);
+ if (!doc_edited) goto cleanup;
+
+ unlink (tmp);
+ tmp = NULL;
+
+ /* Compare original XML with edited. Has it changed at all? */
+ if (STREQ (doc, doc_edited)) {
+ vshPrint (ctl, _("Pool %s XML configuration not changed.\n"),
+ virStoragePoolGetName (pool));
+ ret = TRUE;
+ goto cleanup;
+ }
+
+ /* Now re-read the pool XML. Did someone else change it while
+ * it was being edited? This also catches problems such as us
+ * losing a connection or the pool going away.
+ */
+ doc_reread = virStoragePoolGetXMLDesc (pool, 0);
+ if (!doc_reread)
+ goto cleanup;
+
+ if (STRNEQ (doc, doc_reread)) {
+ vshError (ctl, FALSE,
+ _("ERROR: the XML configuration was changed by another user"));
+ goto cleanup;
+ }
+
+ /* Everything checks out, so redefine the pool. */
+ virStoragePoolFree (pool);
+ pool = virStoragePoolDefineXML (ctl->conn, doc_edited, 0);
+ if (!pool)
+ goto cleanup;
+
+ vshPrint (ctl, _("Pool %s XML configuration edited.\n"),
+ virStoragePoolGetName(pool));
+
+ ret = TRUE;
+
+ cleanup:
+ if (pool)
+ virStoragePoolFree (pool);
+
+ free (doc);
+ free (doc_edited);
+ free (doc_reread);
+
+ if (tmp) {
+ unlink (tmp);
+ free (tmp);
+ }
+
+ return ret;
+}
Index: src/virsh.c
===================================================================
RCS file: /data/cvs/libvirt/src/virsh.c,v
retrieving revision 1.157
diff -u -r1.157 virsh.c
--- src/virsh.c 22 Jul 2008 16:12:01 -0000 1.157
+++ src/virsh.c 1 Aug 2008 10:33:23 -0000
@@ -5069,6 +5069,263 @@
return ret;
}
+/* Common code for the edit / net-edit / pool-edit functions which follow. */
+static char *
+editWriteToTempFile (vshControl *ctl, const char *doc)
+{
+ char *ret;
+ const char *tmpdir;
+ int fd;
+
+ ret = malloc (PATH_MAX);
+ if (!ret) {
+ vshError(ctl, FALSE,
+ _("malloc: failed to allocate temporary file name: %s"),
+ strerror (errno));
+ return NULL;
+ }
+
+ tmpdir = getenv ("TMPDIR");
+ if (!tmpdir) tmpdir = "/tmp";
+ snprintf (ret, PATH_MAX, "%s/virshXXXXXX", tmpdir);
+ fd = mkstemp (ret);
+ if (fd == -1) {
+ vshError(ctl, FALSE,
+ _("mkstemp: failed to create temporary file: %s"),
+ strerror (errno));
+ return NULL;
+ }
+
+ if (safewrite (fd, doc, strlen (doc)) == -1) {
+ vshError(ctl, FALSE,
+ _("write: %s: failed to write to temporary file: %s"),
+ ret, strerror (errno));
+ close (fd);
+ unlink (ret);
+ free (ret);
+ return NULL;
+ }
+ if (close (fd) == -1) {
+ vshError(ctl, FALSE,
+ _("close: %s: failed to write or close temporary file: %s"),
+ ret, strerror (errno));
+ unlink (ret);
+ free (ret);
+ return NULL;
+ }
+
+ /* Temporary filename: caller frees. */
+ return ret;
+}
+
+/* Characters permitted in $EDITOR environment variable and temp filename. */
+#define ACCEPTED_CHARS \
+ "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-/_.:@"
+
+static int
+editFile (vshControl *ctl, const char *filename)
+{
+ const char *editor;
+ char *command;
+ int command_ret;
+
+ editor = getenv ("EDITOR");
+ if (!editor) editor = "vi"; /* could be cruel & default to ed(1) here */
+
+ /* Check the editor doesn't contain shell meta-characters, and if
+ * it does, refuse to run.
+ */
+ if (strspn (editor, ACCEPTED_CHARS) != strlen (editor)) {
+ vshError(ctl, FALSE,
+ _("%s: $EDITOR environment variable contains shell meta or other unacceptable characters"),
+ editor);
+ return -1;
+ }
+ /* Same for the filename. */
+ if (strspn (filename, ACCEPTED_CHARS) != strlen (filename)) {
+ vshError(ctl, FALSE,
+ _("%s: temporary filename contains shell meta or other unacceptable characters (is $TMPDIR wrong?)"),
+ filename);
+ return -1;
+ }
+
+ if (asprintf (&command, "%s %s", editor, filename) == -1) {
+ vshError(ctl, FALSE,
+ _("asprintf: could not create editing command: %s"),
+ strerror (errno));
+ return -1;
+ }
+
+ command_ret = system (command);
+ if (command_ret == -1) {
+ vshError(ctl, FALSE,
+ _("%s: edit command failed: %s"), command, strerror (errno));
+ free (command);
+ return -1;
+ }
+ if (command_ret != WEXITSTATUS (0)) {
+ vshError(ctl, FALSE,
+ _("%s: command exited with non-zero status"), command);
+ free (command);
+ return -1;
+ }
+ free (command);
+ return 0;
+}
+
+static char *
+editReadBackFile (vshControl *ctl, const char *filename)
+{
+ char *ret;
+
+ if (virFileReadAll (filename, VIRSH_MAX_XML_FILE, &ret) == -1) {
+ vshError(ctl, FALSE,
+ _("%s: failed to read temporary file: %s"),
+ filename, strerror (errno));
+ return NULL;
+ }
+ return ret;
+}
+
+/*
+ * "edit" command
+ */
+static vshCmdInfo info_edit[] = {
+ {"syntax", "edit <domain>"},
+ {"help", gettext_noop("edit XML configuration for a domain")},
+ {"desc", gettext_noop("Edit the XML configuration for a domain.")},
+ {NULL, NULL}
+};
+
+static vshCmdOptDef opts_edit[] = {
+ {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("domain name, id or uuid")},
+ {NULL, 0, 0, NULL}
+};
+
+/* This function also acts as a template to generate cmdNetworkEdit
+ * and cmdPoolEdit functions (below) using a sed script in the Makefile.
+ */
+static int
+cmdEdit (vshControl *ctl, vshCmd *cmd)
+{
+ int ret = FALSE;
+ virDomainPtr dom = NULL;
+ char *tmp = NULL;
+ char *doc = NULL;
+ char *doc_edited = NULL;
+ char *doc_reread = NULL;
+
+ if (!vshConnectionUsability(ctl, ctl->conn, TRUE))
+ goto cleanup;
+
+ dom = vshCommandOptDomain (ctl, cmd, "domain", NULL);
+ if (dom == NULL)
+ goto cleanup;
+
+ /* Get the XML configuration of the domain. */
+ doc = virDomainGetXMLDesc (dom, 0);
+ if (!doc)
+ goto cleanup;
+
+ /* Create and open the temporary file. */
+ tmp = editWriteToTempFile (ctl, doc);
+ if (!tmp) goto cleanup;
+
+ /* Start the editor. */
+ if (editFile (ctl, tmp) == -1) goto cleanup;
+
+ /* Read back the edited file. */
+ doc_edited = editReadBackFile (ctl, tmp);
+ if (!doc_edited) goto cleanup;
+
+ unlink (tmp);
+ tmp = NULL;
+
+ /* Compare original XML with edited. Has it changed at all? */
+ if (STREQ (doc, doc_edited)) {
+ vshPrint (ctl, _("Domain %s XML configuration not changed.\n"),
+ virDomainGetName (dom));
+ ret = TRUE;
+ goto cleanup;
+ }
+
+ /* Now re-read the domain XML. Did someone else change it while
+ * it was being edited? This also catches problems such as us
+ * losing a connection or the domain going away.
+ */
+ doc_reread = virDomainGetXMLDesc (dom, 0);
+ if (!doc_reread)
+ goto cleanup;
+
+ if (STRNEQ (doc, doc_reread)) {
+ vshError (ctl, FALSE,
+ _("ERROR: the XML configuration was changed by another user"));
+ goto cleanup;
+ }
+
+ /* Everything checks out, so redefine the domain. */
+ virDomainFree (dom);
+ dom = virDomainDefineXML (ctl->conn, doc_edited);
+ if (!dom)
+ goto cleanup;
+
+ vshPrint (ctl, _("Domain %s XML configuration edited.\n"),
+ virDomainGetName(dom));
+
+ ret = TRUE;
+
+ cleanup:
+ if (dom)
+ virDomainFree (dom);
+
+ free (doc);
+ free (doc_edited);
+ free (doc_reread);
+
+ if (tmp) {
+ unlink (tmp);
+ free (tmp);
+ }
+
+ return ret;
+}
+
+/*
+ * "net-edit" command
+ */
+static vshCmdInfo info_network_edit[] = {
+ {"syntax", "net-edit <network>"},
+ {"help", gettext_noop("edit XML configuration for a network")},
+ {"desc", gettext_noop("Edit the XML configuration for a network.")},
+ {NULL, NULL}
+};
+
+static vshCmdOptDef opts_network_edit[] = {
+ {"network", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("network name, id or uuid")},
+ {NULL, 0, 0, NULL}
+};
+
+/* This is generated from this file by a sed script in the Makefile. */
+#include "virsh-net-edit.c"
+
+/*
+ * "pool-edit" command
+ */
+static vshCmdInfo info_pool_edit[] = {
+ {"syntax", "pool-edit <domain>"},
+ {"help", gettext_noop("edit XML configuration for a storage pool")},
+ {"desc", gettext_noop("Edit the XML configuration for a storage pool.")},
+ {NULL, NULL}
+};
+
+static vshCmdOptDef opts_pool_edit[] = {
+ {"pool", VSH_OT_DATA, VSH_OFLAG_REQ, gettext_noop("pool name or uuid")},
+ {NULL, 0, 0, NULL}
+};
+
+/* This is generated from this file by a sed script in the Makefile. */
+#include "virsh-pool-edit.c"
+
/*
* "quit" command
*/
@@ -5112,6 +5369,7 @@
{"domblkstat", cmdDomblkstat, opts_domblkstat, info_domblkstat},
{"domifstat", cmdDomIfstat, opts_domifstat, info_domifstat},
{"dumpxml", cmdDumpXML, opts_dumpxml, info_dumpxml},
+ {"edit", cmdEdit, opts_edit, info_edit},
{"freecell", cmdFreecell, opts_freecell, info_freecell},
{"hostname", cmdHostname, NULL, info_hostname},
{"list", cmdList, opts_list, info_list},
@@ -5122,6 +5380,7 @@
{"net-define", cmdNetworkDefine, opts_network_define, info_network_define},
{"net-destroy", cmdNetworkDestroy, opts_network_destroy, info_network_destroy},
{"net-dumpxml", cmdNetworkDumpXML, opts_network_dumpxml, info_network_dumpxml},
+ {"net-edit", cmdNetworkEdit, opts_network_edit, info_network_edit},
{"net-list", cmdNetworkList, opts_network_list, info_network_list},
{"net-name", cmdNetworkName, opts_network_name, info_network_name},
{"net-start", cmdNetworkStart, opts_network_start, info_network_start},
@@ -5138,6 +5397,7 @@
{"pool-destroy", cmdPoolDestroy, opts_pool_destroy, info_pool_destroy},
{"pool-delete", cmdPoolDelete, opts_pool_delete, info_pool_delete},
{"pool-dumpxml", cmdPoolDumpXML, opts_pool_dumpxml, info_pool_dumpxml},
+ {"pool-edit", cmdPoolEdit, opts_pool_edit, info_pool_edit},
{"pool-info", cmdPoolInfo, opts_pool_info, info_pool_info},
{"pool-list", cmdPoolList, opts_pool_list, info_pool_list},
{"pool-name", cmdPoolName, opts_pool_name, info_pool_name},
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list