Here's version 4 of the patch, which I believe covers all the things you've raised. The changes since version 3 are: (a) 'sed' snippets reformulated as you suggested (b) const-correctness changes added to this patch (c) We don't include the generated files virsh-net-edit.c or virsh-pool-edit.c in CVS (or in the tarball). 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 14:18:16 -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/.cvsignore =================================================================== RCS file: /data/cvs/libvirt/src/.cvsignore,v retrieving revision 1.5 diff -u -r1.5 .cvsignore --- src/.cvsignore 20 Feb 2008 15:58:38 -0000 1.5 +++ src/.cvsignore 1 Aug 2008 14:18:16 -0000 @@ -11,3 +11,5 @@ *.gcov *.cov libvirt_parthelper +virsh-net-edit.c +virsh-pool-edit.c 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 14:18:16 -0000 @@ -138,6 +138,38 @@ virsh_DEPENDENCIES = $(DEPS) virsh_LDADD = $(LDADDS) $(VIRSH_LIBS) virsh_CFLAGS = $(COVERAGE_CFLAGS) $(READLINE_CFLAGS) +BUILT_SOURCES = virsh-net-edit.c virsh-pool-edit.c + +virsh-net-edit.c: virsh.c Makefile.am + rm -f $@-tmp + echo '/* Automatically generated from: $^ */' > $@-tmp + echo 'static int' >> $@-tmp + awk '/^cmdEdit/, /^}/' $< \ + | sed -e 's/domain/network/g' \ + -e 's/Domain/Network/g' \ + -e 's/cmdEdit/cmdNetworkEdit/g' \ + -e 's/dom/network/g' \ + >> $@-tmp + chmod a-w $@-tmp + rm -f $@ + mv $@-tmp $@ + +virsh-pool-edit.c: virsh.c Makefile.am + rm -f $@-tmp + echo '/* Automatically generated from: $^ */' > $@-tmp + echo 'static int' >> $@-tmp + 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' \ + >> $@-tmp + chmod a-w $@-tmp + rm -f $@ + mv $@-tmp $@ if WITH_STORAGE_DISK if WITH_LIBVIRTD Index: src/virsh.c =================================================================== RCS file: /data/cvs/libvirt/src/virsh.c,v retrieving revision 1.159 diff -u -r1.159 virsh.c --- src/virsh.c 1 Aug 2008 13:51:18 -0000 1.159 +++ src/virsh.c 1 Aug 2008 14:18:17 -0000 @@ -5058,6 +5058,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 const 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 const 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, const 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 const 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 const 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 const 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 const 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 */ @@ -5101,6 +5358,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}, @@ -5111,6 +5369,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}, @@ -5127,6 +5386,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