Currently, we either generate some cmd*Edit commands (cmdPoolEdit and cmdNetworkEdit) via sed script or copy the body of cmdEdit (e.g. cmdInterfaceEdit, cmdNWFilterEdit, etc.). This fact makes it harder to implement any new feature to our editing system. Therefore switch to new implementation - define macros to: - dump XML (EDIT_GET_XML) - take an action if XML wasn't changed, usually just vshPrint() (EDIT_NOT_CHANGED) - define new object (EDIT_DEFINE) - the edited XML is in @doc_edited - free object defined by EDIT_DEFINE (EDIT_FREE) and #include "virsh-edit.c" --- cfg.mk | 4 +- po/POTFILES.in | 1 + tools/Makefile.am | 40 +----- tools/virsh-edit.c | 120 ++++++++++++++ tools/virsh.c | 452 +++++++++++++++++++-------------------------------- 5 files changed, 295 insertions(+), 322 deletions(-) create mode 100644 tools/virsh-edit.c diff --git a/cfg.mk b/cfg.mk index 37f51a6..5d38137 100644 --- a/cfg.mk +++ b/cfg.mk @@ -819,9 +819,9 @@ exclude_file_name_regexp--sc_prohibit_xmlURI = ^src/util/viruri\.c$$ exclude_file_name_regexp--sc_prohibit_return_as_function = \.py$$ -exclude_file_name_regexp--sc_require_config_h = ^examples/ +exclude_file_name_regexp--sc_require_config_h = ^(examples/|tools/virsh-edit.c$$) -exclude_file_name_regexp--sc_require_config_h_first = ^examples/ +exclude_file_name_regexp--sc_require_config_h_first = ^(examples/|tools/virsh-edit.c$$) exclude_file_name_regexp--sc_trailing_blank = \ (/qemuhelpdata/|\.(fig|gif|ico|png)$$) diff --git a/po/POTFILES.in b/po/POTFILES.in index e5d6293..2d3dd1c 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -172,6 +172,7 @@ src/xenxs/xen_xm.c tools/console.c tools/libvirt-guests.init.sh tools/virsh.c +tools/virsh-edit.c tools/virt-host-validate-common.c tools/virt-host-validate-lxc.c tools/virt-host-validate-qemu.c diff --git a/tools/Makefile.am b/tools/Makefile.am index c0c907e..7ca5599 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -25,7 +25,8 @@ EXTRA_DIST = \ virt-sanlock-cleanup.in \ virt-sanlock-cleanup.8 \ virsh.pod \ - libvirt-guests.sysconf + libvirt-guests.sysconf \ + virsh-edit.c DISTCLEANFILES = @@ -111,42 +112,7 @@ virsh_CFLAGS = \ $(COVERAGE_CFLAGS) \ $(LIBXML_CFLAGS) \ $(READLINE_CFLAGS) -BUILT_SOURCES = virsh-net-edit.c virsh-pool-edit.c - -virsh-net-edit.c: virsh.c Makefile.am - $(AM_V_GEN)rm -f $@-tmp && \ - echo '/* Automatically generated from: $^ */' > $@-tmp && \ - echo 'static bool' >> $@-tmp && \ - awk '/^cmdEdit/, /^}/' $< \ - | sed -e 's/domain/network/g' \ - -e 's/Domain/Network/g' \ - -e 's/cmdEdit/cmdNetworkEdit/g' \ - -e 's/dom/network/g' \ - -e 's/int flags.*/int flags = 0;/g' \ - >> $@-tmp && \ - chmod a-w $@-tmp && \ - rm -f $@ && \ - mv $@-tmp $@ - -virsh-pool-edit.c: virsh.c Makefile.am - $(AM_V_GEN)rm -f $@-tmp && \ - echo '/* Automatically generated from: $^ */' > $@-tmp && \ - echo 'static bool' >> $@-tmp && \ - awk '/^cmdEdit/, /^}/' $< \ - | sed -e 's/domain/pool/g' \ - -e 's/vshCommandOptDomain/vshCommandOptPool/g' \ - -e 's/Domain %s/Pool %s/g' \ - -e 's/(ctl, cmd, NULL);/(ctl, cmd, "pool", NULL);/' \ - -e 's/Domain/StoragePool/g' \ - -e 's/cmdEdit/cmdPoolEdit/g' \ - -e 's/\(virStoragePoolDefineXML.*\));/\1, 0);/' \ - -e 's/dom/pool/g' \ - -e 's/int flags.*/int flags = 0;/g' \ - >> $@-tmp && \ - chmod a-w $@-tmp && \ - rm -f $@ && \ - mv $@-tmp $@ - +BUILT_SOURCES = if WITH_WIN_ICON virsh_LDADD += virsh_win_icon.$(OBJEXT) diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c new file mode 100644 index 0000000..b50946e --- /dev/null +++ b/tools/virsh-edit.c @@ -0,0 +1,120 @@ +/* + * virsh-edit.c: Implementation of generic virsh *-edit intelligence + * + * Copyright (C) 2012 Red Hat, Inc. + * + * See COPYING.LIB for the License of this software. + * + * Usage: + * Define macros: + * EDIT_GET_XML - expression which produces a pointer to XML string, e.g: + * #define EDIT_GET_XML virDomainGetXMLDesc(dom, flags) + * + * EDIT_NOT_CHANGED - this action is taken if the XML wasn't changed. + * Note, that you don't want to jump to cleanup but edit_cleanp label + * where temporary variables are free()-d and temporary file is deleted: + * #define EDIT_NOT_CHANGED vshPrint(ctl, _("Domain %s XML not changed"), \ + * virDomainGetName(dom)); \ + * ret = true; goto edit_cleanup; + * Note that this is a statement. + * + * EDIT_DEFINE - expression which redefines the object. The edited XML from + * user is in 'doc_edited' variable. Don't overwrite the pointer to the + * object, as we may iterate once more over and therefore the pointer + * would be invalid. Hence assign object to a different variable. + * Moreover, this needs to be an expression where: + * - 0 is taken as error (our virDefine* APIs often return NULL on error) + * - everything else is taken as success + * For example: + * #define EDIT_DEFINE dom_edited = virDomainDefineXML(ctl->conn, doc_edited) + * + * EDIT_FREE - statement which vir*Free()-s object defined by EDIT_DEFINE, e.g: + * #define EDIT_FREE if (dom_edited) virDomainFree(dom_edited); + * + * Michal Privoznik <mprivozn@redhat.com> + */ + +#ifndef EDIT_GET_XML +# error Missing EDIT_GET_XML definition +#endif + +#ifndef EDIT_NOT_CHANGED +# error Missing EDIT_NOT_CHANGED definition +#endif + +#ifndef EDIT_DEFINE +# error Missing EDIT_DEFINE definition +#endif + +#ifndef EDIT_FREE +# error Missing EDIT_FREE definition +#endif + +do { + char *tmp = NULL; + char *doc = NULL; + char *doc_edited = NULL; + char *doc_reread = NULL; + + /* Get the XML configuration of the object. */ + doc = (EDIT_GET_XML); + if (!doc) + goto edit_cleanup; + + /* Create and open the temporary file. */ + tmp = editWriteToTempFile(ctl, doc); + if (!tmp) + goto edit_cleanup; + + /* Start the editor. */ + if (editFile(ctl, tmp) == -1) + goto edit_cleanup; + + /* Read back the edited file. */ + doc_edited = editReadBackFile(ctl, tmp); + if (!doc_edited) + goto edit_cleanup; + + /* Compare original XML with edited. Has it changed at all? */ + if (STREQ(doc, doc_edited)) { + EDIT_NOT_CHANGED + } + + /* Now re-read the object XML. Did someone else change it while + * it was being edited? This also catches problems such as us + * losing a connection or the object going away. + */ + doc_reread = (EDIT_GET_XML); + if (!doc_reread) + goto edit_cleanup; + + if (STRNEQ(doc, doc_reread)) { + vshError(ctl, "%s", _("ERROR: the XML configuration " + "was changed by another user")); + goto edit_cleanup; + } + + /* Everything checks out, so redefine the object. */ + EDIT_FREE + if (!(EDIT_DEFINE)) + goto edit_cleanup; + + break; + +edit_cleanup: + VIR_FREE(doc); + VIR_FREE(doc_edited); + VIR_FREE(doc_reread); + if (tmp) { + unlink (tmp); + VIR_FREE(tmp); + } + goto cleanup; + +} while (0); + + +#undef EDIT_GET_XML +#undef EDIT_NOT_CHANGED +#undef EDIT_DEFINE +#undef EDIT_FREE diff --git a/tools/virsh.c b/tools/virsh.c index 5226bd8..4af5334 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -3422,9 +3422,6 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd) { const char *file = NULL; bool ret = false; - char *tmp = NULL; - char *doc = NULL; - char *doc_edited = NULL; unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; unsigned int define_flags = 0; @@ -3448,51 +3445,21 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd) if (vshCommandOptString(cmd, "file", &file) <= 0) return false; - /* Get the XML configuration of the saved image. */ - doc = virDomainSaveImageGetXMLDesc(ctl->conn, file, getxml_flags); - 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; - - /* Compare original XML with edited. Short-circuit if it did not - * change, and we do not have any flags. */ - if (STREQ(doc, doc_edited) && !define_flags) { - vshPrint(ctl, _("Saved image %s XML configuration not changed.\n"), - file); - ret = true; - goto cleanup; - } - - /* Everything checks out, so redefine the xml. */ - if (virDomainSaveImageDefineXML(ctl->conn, file, doc_edited, - define_flags) < 0) { - vshError(ctl, _("Failed to update %s"), file); - goto cleanup; - } + #define EDIT_GET_XML \ + virDomainSaveImageGetXMLDesc(ctl->conn, file, getxml_flags) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Saved image %s XML configuration " \ + "not changed.\n"), file); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE \ + virDomainSaveImageDefineXML(ctl->conn, file, doc_edited, define_flags) + #define EDIT_FREE /* */ + #include "virsh-edit.c" vshPrint(ctl, _("State file %s edited.\n"), file); ret = true; cleanup: - VIR_FREE(doc); - VIR_FREE(doc_edited); - if (tmp) { - unlink(tmp); - VIR_FREE(tmp); - } return ret; } @@ -8546,84 +8513,42 @@ static const vshCmdOptDef opts_interface_edit[] = { }; static bool -cmdInterfaceEdit (vshControl *ctl, const vshCmd *cmd) +cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd) { bool ret = false; virInterfacePtr iface = NULL; - char *tmp = NULL; - char *doc = NULL; - char *doc_edited = NULL; - char *doc_reread = NULL; + virInterfacePtr iface_edited = NULL; unsigned int flags = VIR_INTERFACE_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; - iface = vshCommandOptInterface (ctl, cmd, NULL); + iface = vshCommandOptInterface(ctl, cmd, NULL); if (iface == NULL) goto cleanup; - /* Get the XML configuration of the interface. */ - doc = virInterfaceGetXMLDesc (iface, flags); - 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; + #define EDIT_GET_XML virInterfaceGetXMLDesc(iface, flags) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Interface %s XML configuration not changed.\n"), \ + virInterfaceGetName(iface)); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE \ + iface_edited = virInterfaceDefineXML(ctl->conn, doc_edited, 0) + #define EDIT_FREE \ + if (iface_edited) \ + virInterfaceFree(iface_edited); + #include "virsh-edit.c" - /* Read back the edited file. */ - doc_edited = editReadBackFile (ctl, tmp); - if (!doc_edited) goto cleanup; - - /* Compare original XML with edited. Has it changed at all? */ - if (STREQ (doc, doc_edited)) { - vshPrint (ctl, _("Interface %s XML configuration not changed.\n"), - virInterfaceGetName (iface)); - ret = true; - goto cleanup; - } - - /* Now re-read the interface XML. Did someone else change it while - * it was being edited? This also catches problems such as us - * losing a connection or the interface going away. - */ - doc_reread = virInterfaceGetXMLDesc (iface, flags); - if (!doc_reread) - goto cleanup; - - if (STRNEQ (doc, doc_reread)) { - vshError(ctl, "%s", - _("ERROR: the XML configuration was changed by another user")); - goto cleanup; - } - - /* Everything checks out, so redefine the interface. */ - virInterfaceFree (iface); - iface = virInterfaceDefineXML(ctl->conn, doc_edited, 0); - if (!iface) - goto cleanup; - - vshPrint (ctl, _("Interface %s XML configuration edited.\n"), - virInterfaceGetName(iface)); + vshPrint(ctl, _("Interface %s XML configuration edited.\n"), + virInterfaceGetName(iface)); ret = true; cleanup: if (iface) - virInterfaceFree (iface); - - VIR_FREE(doc); - VIR_FREE(doc_edited); - VIR_FREE(doc_reread); - - if (tmp) { - unlink (tmp); - VIR_FREE(tmp); - } + virInterfaceFree(iface); + if (iface_edited) + virInterfaceFree(iface_edited); return ret; } @@ -9983,83 +9908,42 @@ static const vshCmdOptDef opts_nwfilter_edit[] = { }; static bool -cmdNWFilterEdit (vshControl *ctl, const vshCmd *cmd) +cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd) { bool ret = false; virNWFilterPtr nwfilter = NULL; - char *tmp = NULL; - char *doc = NULL; - char *doc_edited = NULL; - char *doc_reread = NULL; + virNWFilterPtr nwfilter_edited = NULL; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; - nwfilter = vshCommandOptNWFilter (ctl, cmd, NULL); + nwfilter = vshCommandOptNWFilter(ctl, cmd, NULL); if (nwfilter == NULL) goto cleanup; - /* Get the XML configuration of the interface. */ - doc = virNWFilterGetXMLDesc (nwfilter, 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; - - /* Compare original XML with edited. Has it changed at all? */ - if (STREQ (doc, doc_edited)) { - vshPrint (ctl, _("Network filter %s XML configuration not changed.\n"), - virNWFilterGetName (nwfilter)); - ret = true; - goto cleanup; - } - - /* Now re-read the network filter XML. Did someone else change it while - * it was being edited? This also catches problems such as us - * losing a connection or the interface going away. - */ - doc_reread = virNWFilterGetXMLDesc (nwfilter, 0); - if (!doc_reread) - goto cleanup; - - if (STRNEQ (doc, doc_reread)) { - vshError(ctl, "%s", - _("ERROR: the XML configuration was changed by another user")); - goto cleanup; - } - - /* Everything checks out, so redefine the interface. */ - virNWFilterFree (nwfilter); - nwfilter = virNWFilterDefineXML(ctl->conn, doc_edited); - if (!nwfilter) - goto cleanup; + #define EDIT_GET_XML virNWFilterGetXMLDesc(nwfilter, 0) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Network filter %s XML " \ + "configuration not changed.\n"), \ + virNWFilterGetName(nwfilter)); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE \ + nwfilter_edited = virNWFilterDefineXML(ctl->conn, doc_edited) + #define EDIT_FREE \ + if (nwfilter_edited) \ + virNWFilterFree(nwfilter); + #include "virsh-edit.c" - vshPrint (ctl, _("Network filter %s XML configuration edited.\n"), - virNWFilterGetName(nwfilter)); + vshPrint(ctl, _("Network filter %s XML configuration edited.\n"), + virNWFilterGetName(nwfilter)); ret = true; cleanup: if (nwfilter) - virNWFilterFree (nwfilter); - - VIR_FREE(doc); - VIR_FREE(doc_edited); - VIR_FREE(doc_reread); - - if (tmp) { - unlink (tmp); - VIR_FREE(tmp); - } + virNWFilterFree(nwfilter); + if (nwfilter_edited) + virNWFilterFree(nwfilter_edited); return ret; } @@ -15805,88 +15689,42 @@ static const vshCmdOptDef opts_edit[] = { {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 bool -cmdEdit (vshControl *ctl, const vshCmd *cmd) +cmdEdit(vshControl *ctl, const vshCmd *cmd) { bool ret = false; virDomainPtr dom = NULL; - char *tmp = NULL; - char *doc = NULL; - char *doc_edited = NULL; - char *doc_reread = NULL; + virDomainPtr dom_edited = NULL; unsigned int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; if (!vshConnectionUsability(ctl, ctl->conn)) goto cleanup; - dom = vshCommandOptDomain (ctl, cmd, NULL); + dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; - /* Get the XML configuration of the domain. */ - doc = virDomainGetXMLDesc (dom, flags); - 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; - - /* 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, flags); - if (!doc_reread) - goto cleanup; - - if (STRNEQ (doc, doc_reread)) { - vshError(ctl, - "%s", _("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; + #define EDIT_GET_XML virDomainGetXMLDesc(dom, flags) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Domain %s XML configuration not changed.\n"), \ + virDomainGetName (dom)); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE dom_edited = virDomainDefineXML(ctl->conn, doc_edited) + #define EDIT_FREE \ + if (dom_edited) \ + virDomainFree(dom_edited); + #include "virsh-edit.c" - vshPrint (ctl, _("Domain %s XML configuration edited.\n"), - virDomainGetName(dom)); + vshPrint(ctl, _("Domain %s XML configuration edited.\n"), + virDomainGetName(dom)); ret = true; cleanup: if (dom) - virDomainFree (dom); - - VIR_FREE(doc); - VIR_FREE(doc_edited); - VIR_FREE(doc_reread); - - if (tmp) { - unlink (tmp); - VIR_FREE(tmp); - } + virDomainFree(dom); + if (dom_edited) + virDomainFree(dom_edited); return ret; } @@ -15906,8 +15744,45 @@ static const vshCmdOptDef opts_network_edit[] = { {NULL, 0, 0, NULL} }; -/* This is generated from this file by a sed script in the Makefile. */ -#include "virsh-net-edit.c" +static bool +cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virNetworkPtr network = NULL; + virNetworkPtr network_edited = NULL; + unsigned int flags = 0; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + network = vshCommandOptNetwork(ctl, cmd, NULL); + if (network == NULL) + goto cleanup; + + #define EDIT_GET_XML virNetworkGetXMLDesc(network, flags) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Network %s XML configuration not changed.\n"), \ + virNetworkGetName (network)); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE network_edited = virNetworkDefineXML(ctl->conn, doc_edited) + #define EDIT_FREE \ + if (network_edited) \ + virNetworkFree(network_edited); + #include "virsh-edit.c" + + vshPrint(ctl, _("Network %s XML configuration edited.\n"), + virNetworkGetName(network)); + + ret = true; + + cleanup: + if (network) + virNetworkFree(network); + if (network_edited) + virNetworkFree(network_edited); + + return ret; +} /* * "pool-edit" command @@ -15923,8 +15798,46 @@ static const vshCmdOptDef opts_pool_edit[] = { {NULL, 0, 0, NULL} }; -/* This is generated from this file by a sed script in the Makefile. */ -#include "virsh-pool-edit.c" +static bool +cmdPoolEdit(vshControl *ctl, const vshCmd *cmd) +{ + bool ret = false; + virStoragePoolPtr pool = NULL; + virStoragePoolPtr pool_edited = NULL; + unsigned int flags = 0; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + pool = vshCommandOptPool(ctl, cmd, "pool", NULL); + if (pool == NULL) + goto cleanup; + + #define EDIT_GET_XML virStoragePoolGetXMLDesc(pool, flags) + #define EDIT_NOT_CHANGED \ + vshPrint(ctl, _("Pool %s XML configuration not changed.\n"), \ + virStoragePoolGetName (pool)); \ + ret = true; goto edit_cleanup; + #define EDIT_DEFINE \ + pool_edited = virStoragePoolDefineXML(ctl->conn, doc_edited, 0) + #define EDIT_FREE \ + if (pool_edited) \ + virStoragePoolFree(pool_edited); + #include "virsh-edit.c" + + vshPrint(ctl, _("Pool %s XML configuration edited.\n"), + virStoragePoolGetName(pool)); + + ret = true; + + cleanup: + if (pool) + virStoragePoolFree(pool); + if (pool_edited) + virStoragePoolFree(pool_edited); + + return ret; +} /* * "quit" command @@ -16317,9 +16230,6 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) const char *name; const char *edited_name; bool ret = false; - char *tmp = NULL; - char *doc = NULL; - char *doc_edited = NULL; unsigned int getxml_flags = VIR_DOMAIN_XML_SECURE; unsigned int define_flags = VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE; bool rename_okay = vshCommandOptBool(cmd, "rename"); @@ -16346,45 +16256,25 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) &snapshot, &name) < 0) goto cleanup; - /* Get the XML configuration of the snapshot. */ - doc = virDomainSnapshotGetXMLDesc(snapshot, getxml_flags); - if (!doc) - goto cleanup; - - /* strstr is safe here, since xml came from libvirt API and not user */ - if (strstr(doc, "<state>disk-snapshot</state>")) - define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY; - - /* 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; - - /* Compare original XML with edited. Short-circuit if it did not - * change, and we do not have any flags. */ - if (STREQ(doc, doc_edited) && - !(define_flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) { - vshPrint(ctl, _("Snapshot %s XML configuration not changed.\n"), - name); - ret = true; - goto cleanup; - } - - /* Everything checks out, so redefine the xml. */ - edited = virDomainSnapshotCreateXML(dom, doc_edited, define_flags); - if (!edited) { - vshError(ctl, _("Failed to update %s"), name); - goto cleanup; - } + #define EDIT_GET_XML \ + virDomainSnapshotGetXMLDesc(snapshot, getxml_flags) + #define EDIT_NOT_CHANGED \ + /* Depending on flags, we re-edit even if XML is unchanged. */ \ + if (!(define_flags & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) { \ + vshPrint(ctl, \ + _("Snapshot %s XML configuration not changed.\n"), \ + name); \ + ret = true; \ + goto cleanup; \ + } + #define EDIT_DEFINE \ + (strstr(doc, "<state>disk-snapshot</state>") ? \ + define_flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY : 0), \ + edited = virDomainSnapshotCreateXML(dom, doc_edited, define_flags) + #define EDIT_FREE \ + if (edited) \ + virDomainSnapshotFree(edited); + #include "virsh-edit.c" edited_name = virDomainSnapshotGetName(edited); if (STREQ(name, edited_name)) { @@ -16413,16 +16303,12 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) ret = true; cleanup: - VIR_FREE(doc); - VIR_FREE(doc_edited); - if (tmp) { - unlink(tmp); - VIR_FREE(tmp); - } - if (snapshot) - virDomainSnapshotFree(snapshot); if (edited) virDomainSnapshotFree(edited); + else + vshError(ctl, _("Failed to update %s"), name); + if (snapshot) + virDomainSnapshotFree(snapshot); if (dom) virDomainFree(dom); return ret; -- 1.7.8.5

If users *-edit but make a mistake in XML all changes are permanently lost. However, if virsh is not running within a script we can ask user if he wants to re-edit the file and correct the mistakes. --- tools/console.c | 40 ++++++++++++++++++++++------------- tools/console.h | 2 + tools/virsh-edit.c | 38 ++++++++++++++++++++++++++++++---- tools/virsh.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 20 deletions(-) diff --git a/tools/console.c b/tools/console.c index 34fde05..90e54e3 100644 --- a/tools/console.c +++ b/tools/console.c @@ -298,13 +298,36 @@ vshGetEscapeChar(const char *s) return *s; } +int vshMakeStdinRaw(struct termios *ttyattr, bool report_errors) { + struct termios rawattr; + + if (tcgetattr(STDIN_FILENO, ttyattr) < 0) { + if (report_errors) + VIR_ERROR(_("unable to get tty attributes: %s"), + strerror(errno)); + return -1; + } + + rawattr = *ttyattr; + cfmakeraw(&rawattr); + + if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) { + if (report_errors) + VIR_ERROR(_("unable to set tty attributes: %s"), + strerror(errno)); + return -1; + } + + return 0; +} + int vshRunConsole(virDomainPtr dom, const char *dev_name, const char *escape_seq, unsigned int flags) { int ret = -1; - struct termios ttyattr, rawattr; + struct termios ttyattr; void (*old_sigquit)(int); void (*old_sigterm)(int); void (*old_sigint)(int); @@ -317,21 +340,8 @@ int vshRunConsole(virDomainPtr dom, result in it being echoed back already), and also ensure Ctrl-C, etc is blocked, and misc other bits */ - if (tcgetattr(STDIN_FILENO, &ttyattr) < 0) { - VIR_ERROR(_("unable to get tty attributes: %s"), - strerror(errno)); - return -1; - } - - rawattr = ttyattr; - cfmakeraw(&rawattr); - - if (tcsetattr(STDIN_FILENO, TCSAFLUSH, &rawattr) < 0) { - VIR_ERROR(_("unable to set tty attributes: %s"), - strerror(errno)); + if (vshMakeStdinRaw(&ttyattr, true) < 0) goto resettty; - } - /* Trap all common signals so that we can safely restore the original terminal settings on STDIN before the diff --git a/tools/console.h b/tools/console.h index 2b5440c..1feea9e 100644 --- a/tools/console.h +++ b/tools/console.h @@ -30,6 +30,8 @@ int vshRunConsole(virDomainPtr dom, const char *escape_seq, unsigned int flags); +int vshMakeStdinRaw(struct termios *ttyattr, bool report_errors); + # endif /* !WIN32 */ #endif /* __VIR_CONSOLE_H__ */ diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c index b50946e..ef47f9d 100644 --- a/tools/virsh-edit.c +++ b/tools/virsh-edit.c @@ -55,6 +55,7 @@ do { char *doc = NULL; char *doc_edited = NULL; char *doc_reread = NULL; + char *msg = NULL; /* Get the XML configuration of the object. */ doc = (EDIT_GET_XML); @@ -66,6 +67,7 @@ do { if (!tmp) goto edit_cleanup; +reedit: /* Start the editor. */ if (editFile(ctl, tmp) == -1) goto edit_cleanup; @@ -80,6 +82,9 @@ do { EDIT_NOT_CHANGED } +redefine: + msg = NULL; + /* Now re-read the object XML. Did someone else change it while * it was being edited? This also catches problems such as us * losing a connection or the object going away. @@ -89,15 +94,38 @@ do { goto edit_cleanup; if (STRNEQ(doc, doc_reread)) { - vshError(ctl, "%s", _("ERROR: the XML configuration " - "was changed by another user")); - goto edit_cleanup; + msg = _("The XML configuration was changed by another user."); + VIR_FREE(doc); + doc = doc_reread; + doc_reread = NULL; } /* Everything checks out, so redefine the object. */ EDIT_FREE - if (!(EDIT_DEFINE)) - goto edit_cleanup; + if (!msg && !(EDIT_DEFINE)) { + msg = _("Failed."); + } + + if (msg) { + int c = vshAskReedit(ctl, msg); + switch (c) { + case 'y': + goto reedit; + break; + + case 'f': + goto redefine; + break; + + case 'n': + goto edit_cleanup; + break; + + default: + vshError(ctl, "%s", msg); + break; + } + } break; diff --git a/tools/virsh.c b/tools/virsh.c index 4af5334..574b92f 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -33,6 +33,7 @@ #include <signal.h> #include <poll.h> #include <strings.h> +#include <termios.h> #include <libxml/parser.h> #include <libxml/tree.h> @@ -654,6 +655,62 @@ vshReconnect(vshControl *ctl) ctl->useSnapshotOld = false; } +/** + * vshAskReedit: + * @msg: Question to ask user + * + * Ask user if he wants to return to previously + * edited file. + * + * Returns 'y' if he wants to + * 'f' if he forcibly wants to + * 'n' if he doesn't want to + * -1 on error + * 0 otherwise + */ +static int +vshAskReedit(vshControl *ctl, const char *msg) +{ +#ifndef WIN32 + int c = -1; + struct termios ttyattr; + + if (!isatty(STDIN_FILENO)) + return -1; + + virshReportError(ctl); + + if (vshMakeStdinRaw(&ttyattr, false) < 0) + return -1; + + while (true) { + /* TRANSLATORS: For now, we aren't using LC_MESSAGES, and the user + * choices really are limited to just 'y', 'n', 'f' and '?' */ + vshPrint(ctl, "\r%s %s", msg, _("Try again? [y,n,f,?]:")); + c = c_tolower(getchar()); + + if (c == '?') { + vshPrint(ctl, "\r\n%s", _("y - yes, start editor again\r\n" + "n - no, throw away my changes\r\n" + "f - force, try to redefine again\r\n" + "? - print this help\r\n")); + continue; + } else if (c == 'y' || c == 'n' || c == 'f') { + break; + } + } + + tcsetattr(STDIN_FILENO, TCSAFLUSH, &ttyattr); + + vshPrint(ctl, "\r\n"); + return c; +#else + vshDebug(ctl, VSH_ERR_WARNING, "%s", _("This function is not " + "supported on WIN32 platform")); + return 0; +#endif +} + /* --------------- * Commands * --------------- -- 1.7.8.5