On Sun, Nov 08, 2015 at 02:53:49AM +0300, rodinasophie@gmail.com wrote:

From: Sofia Rodina <rodinasophie@gmail.com>

Wow, this is here for a long time. I'll just point out few things I've seen since there was not ping or pressure for this. So at least to let others know how to proceed if anyone is interested.

--- tools/Makefile.am | 1 + tools/virsh-domain.c | 187 ++++++++++++++++++++++++++++++++++++------------ tools/virsh-edit.c | 79 ++++++-------------- tools/virsh-edit.h | 42 +++++++++++ tools/virsh-interface.c | 64 +++++++++++++---- tools/virsh-network.c | 62 ++++++++++++---- tools/virsh-nwfilter.c | 64 +++++++++++++---- tools/virsh-pool.c | 64 +++++++++++++---- tools/virsh-snapshot.c | 76 ++++++++++++++------ 9 files changed, 464 insertions(+), 175 deletions(-) create mode 100644 tools/virsh-edit.h

diff --git a/tools/Makefile.am b/tools/Makefile.am index d5638d9..88bb1b2 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -205,6 +205,7 @@ virsh_SOURCES = \ virsh-domain.c virsh-domain.h \ virsh-domain-monitor.c virsh-domain-monitor.h \ virsh-host.c virsh-host.h \ + virsh-edit.c virsh-edit.h \ virsh-interface.c virsh-interface.h \ virsh-network.c virsh-network.h \ virsh-nodedev.c virsh-nodedev.h \ diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 12e85e3..0beaa62 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -56,6 +56,7 @@ #include "virtime.h" #include "virtypedparam.h" #include "virxml.h" +#include "virsh-edit.h"

/* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -4622,6 +4623,44 @@ static const vshCmdOptDef opts_save_image_edit[] = { {.name = NULL} };

+typedef struct virshEditDomainUniversalStruct

I would prefer shorter name, we have too many long ones already. I think something like virshEditData could work as well. Plus we usually do: struct _virshEditData { stuff }; typedef struct _virshEditData virshEditData; typedef virshEditData *virshEditDataPtr;

+{ + vshControl *ctl; + virshControlPtr priv; + const char *file; + virDomainPtr dom; + unsigned int getxml_flags; + bool *ret; + unsigned int *define_flags; + const char *key; + virDomainPtr *dom_edited;

Do we really need all those? For example I see you only setting the "ret" variable, but not using it anywhere.

+} virshEditDomainUniversalStruct; + +static char * +virshEditDomainSaveImageGetXML(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct *)args; + return virDomainSaveImageGetXMLDesc(str->priv->conn, str->file, str->getxml_flags); +} + +static void +virshEditDomainSaveImageNotChanged(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + vshPrint(str->ctl, _("Saved image %s XML configuration not changed.\n"), str->file); + *(str->ret) = true; +} +

This function ^^ could be generalized so that it's not redefined so many times, I guess. But that's not needed.

+static bool +virshEditDomainSaveIageDefine(void *args, char *doc_edited, char *doc)

s/Iage/Image/

+{ + if (!doc) + return false; + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + return (virDomainSaveImageDefineXML(str->priv->conn, str->file, + doc_edited, *(str->define_flags)) == 0); +} + static bool cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd) { @@ -4647,20 +4686,22 @@ cmdSaveImageEdit(vshControl *ctl, const vshCmd *cmd)

if (vshCommandOptStringReq(ctl, cmd, "file", &file) < 0) return false; - -#define EDIT_GET_XML \ - virDomainSaveImageGetXMLDesc(priv->conn, file, getxml_flags) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Saved image %s XML configuration " \ - "not changed.\n"), file); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (virDomainSaveImageDefineXML(priv->conn, file, doc_edited, define_flags) == 0) -#include "virsh-edit.c" - + {

This doesn't need to be in a block. It would be more readable outside of it, anyway.

+ virshEditDomainUniversalStruct editDomainArgs = { + NULL, priv, file, NULL, getxml_flags, + &ret, &define_flags, NULL, NULL

If you use C99 style definition, you don't need to skip places with NULLs and everyone knows what is being set.

+ }; + virshEditControl editCtl = { + &virshEditDomainSaveImageGetXML, + &virshEditDomainSaveImageNotChanged, + &virshEditDomainSaveIageDefine, + NULL, + &editDomainArgs, + }; + + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } vshPrint(ctl, _("State file %s edited.\n"), file); ret = true;

@@ -8145,6 +8186,30 @@ virshDomainGetEditMetadata(vshControl *ctl, return ret; }

+static char * +virshEditDomainMetadataGetXML(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)(args); + return virshDomainGetEditMetadata(str->ctl, str->dom, str->file, str->getxml_flags); +} + +static void +virshEditMetadataNotChanged(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + vshPrint(str->ctl, "%s", _("Metadata not changed")); + *(str->ret) = true; +} + +static bool +virshEditMetadataDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + return (virDomainSetMetadata(str->dom, VIR_DOMAIN_METADATA_ELEMENT, doc_edited, + str->key, str->file, *(str->define_flags)) == 0); +}

static bool cmdMetadata(vshControl *ctl, const vshCmd *cmd) @@ -8196,20 +8261,19 @@ cmdMetadata(vshControl *ctl, const vshCmd *cmd) else vshPrint("%s\n", _("Metadata modified")); } else if (edit) { -#define EDIT_GET_XML \ - virshDomainGetEditMetadata(ctl, dom, uri, flags) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, "%s", _("Metadata not changed")); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) - -#define EDIT_DEFINE \ - (virDomainSetMetadata(dom, VIR_DOMAIN_METADATA_ELEMENT, doc_edited, \ - key, uri, flags) == 0) -#include "virsh-edit.c" - + virshEditDomainUniversalStruct editMetadataArgs = { + ctl, NULL, uri, dom, + flags, &ret, &flags, key, NULL + }; + virshEditControl editCtl = { + &virshEditDomainMetadataGetXML, + &virshEditMetadataNotChanged, + &virshEditMetadataDefine, + NULL, + &editMetadataArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; vshPrint("%s\n", _("Metadata modified")); } else { char *data; @@ -11764,6 +11828,40 @@ static const vshCmdOptDef opts_edit[] = { {.name = NULL} };

+static char * +virshEditDomainGetXML(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + return virDomainGetXMLDesc(str->dom, str->getxml_flags); +} + +static void +virshEditDomainNotChanged(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + vshPrint(str->ctl, _("Domain %s XML configuration not changed.\n"), + virDomainGetName(str->dom)); + *(str->ret) = true; +} + +static bool +virshEditDomainDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + return + (*(str->dom_edited) = virshDomainDefine(str->priv->conn, doc_edited, + *(str->define_flags))); +} + +static void +virshEditDomainRelax(void *args) +{ + virshEditDomainUniversalStruct *str = (virshEditDomainUniversalStruct*)args; + *(str->define_flags) &= ~VIR_DOMAIN_DEFINE_VALIDATE; +} + static bool cmdEdit(vshControl *ctl, const vshCmd *cmd) { @@ -11780,24 +11878,21 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd)

if (vshCommandOptBool(cmd, "skip-validate")) define_flags &= ~VIR_DOMAIN_DEFINE_VALIDATE; - -#define EDIT_GET_XML virDomainGetXMLDesc(dom, query_flags) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Domain %s XML configuration not changed.\n"), \ - virDomainGetName(dom)); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (dom_edited = virshDomainDefine(priv->conn, doc_edited, define_flags)) -#define EDIT_RELAX \ - do { \ - define_flags &= ~VIR_DOMAIN_DEFINE_VALIDATE; \ - } while (0); - -#include "virsh-edit.c" -#undef EDIT_RELAX + { + virshEditDomainUniversalStruct editDomainArgs = { + ctl, priv, NULL, dom, + query_flags, &ret, &define_flags, NULL, &dom_edited + }; + virshEditControl editCtl = { + &virshEditDomainGetXML, + &virshEditDomainNotChanged, + &virshEditDomainDefine, + &virshEditDomainRelax, + &editDomainArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + }

vshPrint(ctl, _("Domain %s XML configuration edited.\n"), virDomainGetName(dom_edited)); diff --git a/tools/virsh-edit.c b/tools/virsh-edit.c index 1b39cb7..fdc2842 100644 --- a/tools/virsh-edit.c +++ b/tools/virsh-edit.c @@ -1,7 +1,7 @@ /* - * virsh-edit.c: Implementation of generic virsh *-edit intelligence + * virsh-edit.c: Main function to edit in virsh * - * Copyright (C) 2012, 2015 Red Hat, Inc. + * Copyright (C) 2005, 2007-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -17,45 +17,17 @@ * License along with this library. If not, see * <http://www.gnu.org/licenses/>. * - * 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_cleanup 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)) - * - * Michal Privoznik <mprivozn@redhat.com>

You could've left the original author in here...

*/

-#ifndef EDIT_GET_XML -# error Missing EDIT_GET_XML definition -#endif +#include <config.h> +#include "internal.h"

-#ifndef EDIT_NOT_CHANGED -# error Missing EDIT_NOT_CHANGED definition -#endif +#include "virsh-edit.h" +#include "viralloc.h"

-#ifndef EDIT_DEFINE -# error Missing EDIT_DEFINE definition -#endif - -do { +bool +virshEdit(vshControl *ctl, virshEditControl *editCtl) +{ char *tmp = NULL; char *doc = NULL; char *doc_edited = NULL; @@ -65,7 +37,7 @@ do { bool relax_avail = false;

/* Get the XML configuration of the object. */ - doc = (EDIT_GET_XML); + doc = (*editCtl->virshEditGetXML)(editCtl->virshEditArgs); if (!doc) goto edit_cleanup;

@@ -75,10 +47,8 @@ do { goto edit_cleanup;

reedit: - -#ifdef EDIT_RELAX - relax_avail = true; -#endif + if (editCtl->virshEditRelax) + relax_avail = true;

/* Start the editor. */ if (vshEditFile(ctl, tmp) == -1) @@ -91,8 +61,10 @@ do { goto edit_cleanup;

/* Compare original XML with edited. Has it changed at all? */ - if (STREQ(doc, doc_edited)) - EDIT_NOT_CHANGED; + if (STREQ(doc, doc_edited)) { + (*editCtl->virshEditNotChanged)(editCtl->virshEditArgs); + goto edit_cleanup; + }

redefine: msg = NULL; @@ -102,7 +74,7 @@ do { * losing a connection or the object going away. */ VIR_FREE(doc_reread); - doc_reread = (EDIT_GET_XML); + doc_reread = (*editCtl->virshEditGetXML)(editCtl->virshEditArgs); if (!doc_reread) goto edit_cleanup;

@@ -114,7 +86,7 @@ do { }

/* Everything checks out, so redefine the object. */ - if (!msg && !(EDIT_DEFINE)) + if (!msg && !((*editCtl->virshEditDefine)(editCtl->virshEditArgs, doc_edited, doc))) msg = _("Failed.");

if (msg) { @@ -132,16 +104,14 @@ do { goto edit_cleanup; break;

-#ifdef EDIT_RELAX case 'i': if (relax_avail) { - EDIT_RELAX; + (*editCtl->virshEditRelax)(editCtl->virshEditArgs); relax_avail = false; goto redefine; break; } /* fall-through */ -#endif

default: vshError(ctl, "%s", msg); @@ -161,11 +131,6 @@ do { }

if (!edit_success) - goto cleanup; - -} while (0); - - -#undef EDIT_GET_XML -#undef EDIT_NOT_CHANGED -#undef EDIT_DEFINE + return false; + return true;

return edit_success;

+} diff --git a/tools/virsh-edit.h b/tools/virsh-edit.h new file mode 100644 index 0000000..9de3456 --- /dev/null +++ b/tools/virsh-edit.h @@ -0,0 +1,42 @@ +/* + * virsh-edit.h: Definition of generic virsh *-edit interface + * + * Copyright (C) 2012, 2015 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#ifndef __VIRSH_EDIT_H_ +#define __VIRSH_EDIT_H_ + +#include "vsh.h" + +typedef char *(virshEditGetXMLCallback)(void *args); +typedef void (virshEditNotChangedCallback)(void *args); +typedef bool (virshEditDefineCallback)(void *args, char *doc_edited, char *doc); +typedef void (virshEditRelaxCallback)(void *args); + +typedef struct virshEditControl { + virshEditGetXMLCallback *virshEditGetXML; + virshEditNotChangedCallback *virshEditNotChanged; + virshEditDefineCallback *virshEditDefine; + virshEditRelaxCallback *virshEditRelax; + void *virshEditArgs;

We usually call this "opaque" as it transfers something it cannot see through. Also "virshEditArgs" sounds like it carries parameter to virshEdit function which is not true.

+} virshEditControl; + +bool virshEdit(vshControl *ctl, virshEditControl *editCtl); + +#endif // __VIRSH_EDIT_H_

Nit picks: - /* */ instead of // - if you start with double underscore, end with double underscore as well

diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index b69c685..28b15c4 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -39,6 +39,7 @@ #include "virutil.h" #include "virxml.h" #include "virstring.h" +#include "virsh-edit.h"

virInterfacePtr virshCommandOptInterfaceBy(vshControl *ctl, const vshCmd *cmd, @@ -108,6 +109,42 @@ static const vshCmdOptDef opts_interface_edit[] = { {.name = NULL} };

+typedef struct virshEditInterfaceStruct +{ + vshControl *ctl; + virshControlPtr priv; + virInterfacePtr iface; + virInterfacePtr *iface_edited; + unsigned int flags; + bool *ret; +} virshEditInterfaceStruct; + +static char * +virshEditInterfaceGetXML(void *args) +{ + virshEditInterfaceStruct *str = (virshEditInterfaceStruct*)args; + return virInterfaceGetXMLDesc(str->iface, str->flags); +} + +static void +virshEditInterfaceNotChanged(void *args) +{ + virshEditInterfaceStruct *str = (virshEditInterfaceStruct*)args; + vshPrint(str->ctl, _("Interface %s XML configuration not changed.\n"), + virInterfaceGetName(str->iface)); + *(str->ret) = true; +} + +static bool +virshEditInterfaceDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditInterfaceStruct *str = (virshEditInterfaceStruct*)args;

We don't do c99 initializations in the middle of the function.

+ return (*(str->iface_edited) = virInterfaceDefineXML(str->priv->conn, + doc_edited, 0)); +} + static bool cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd) { @@ -120,19 +157,20 @@ cmdInterfaceEdit(vshControl *ctl, const vshCmd *cmd) iface = virshCommandOptInterface(ctl, cmd, NULL); if (iface == NULL) goto cleanup; - -#define EDIT_GET_XML virInterfaceGetXMLDesc(iface, flags) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Interface %s XML configuration not changed.\n"), \ - virInterfaceGetName(iface)); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (iface_edited = virInterfaceDefineXML(priv->conn, doc_edited, 0)) -#include "virsh-edit.c" - + { + virshEditInterfaceStruct editInterfaceArgs = { + ctl, priv, iface, &iface_edited, flags, &ret + }; + virshEditControl editCtl = { + &virshEditInterfaceGetXML, + &virshEditInterfaceNotChanged, + &virshEditInterfaceDefine, + NULL, + &editInterfaceArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } vshPrint(ctl, _("Interface %s XML configuration edited.\n"), virInterfaceGetName(iface_edited));

diff --git a/tools/virsh-network.c b/tools/virsh-network.c index a0f7707..0118adc 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -32,6 +32,7 @@ #include "virfile.h" #include "virstring.h" #include "conf/network_conf.h" +#include "virsh-edit.h"

virNetworkPtr virshCommandOptNetworkBy(vshControl *ctl, const vshCmd *cmd, @@ -1121,6 +1122,41 @@ static char *virshNetworkGetXMLDesc(virNetworkPtr network) return doc; }

+typedef struct virshEditNetworkStruct +{ + vshControl *ctl; + virNetworkPtr network; + virNetworkPtr *network_edited; + virshControlPtr priv; + bool *ret; +} virshEditNetworkStruct; + +static char * +virshEditNetworkGetXML(void *args) +{ + virshEditNetworkStruct *str = (virshEditNetworkStruct*)args; + return virshNetworkGetXMLDesc(str->network); +} + +static void +virshEditNetworkNotChanged(void *args) +{ + virshEditNetworkStruct *str = (virshEditNetworkStruct*)args; + vshPrint(str->ctl, _("Network %s XML configuration not changed.\n"), + virNetworkGetName(str->network)); + *(str->ret) = true; +} + +static bool +virshEditNetworkDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditNetworkStruct *str = (virshEditNetworkStruct*)args; + *(str->network_edited) = virNetworkDefineXML(str->priv->conn, doc_edited); + return *(str->network_edited); +} + static bool cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) { @@ -1133,18 +1169,20 @@ cmdNetworkEdit(vshControl *ctl, const vshCmd *cmd) if (network == NULL) goto cleanup;

-#define EDIT_GET_XML virshNetworkGetXMLDesc(network) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Network %s XML configuration not changed.\n"), \ - virNetworkGetName(network)); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (network_edited = virNetworkDefineXML(priv->conn, doc_edited)) -#include "virsh-edit.c" - + { + virshEditNetworkStruct editNetworkArgs = { + ctl, network, &network_edited, priv, &ret + }; + virshEditControl editCtl = { + &virshEditNetworkGetXML, + &virshEditNetworkNotChanged, + &virshEditNetworkDefine, + NULL, + &editNetworkArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } vshPrint(ctl, _("Network %s XML configuration edited.\n"), virNetworkGetName(network_edited));

diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index 7732da8..36677ca 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -31,6 +31,7 @@ #include "viralloc.h" #include "virfile.h" #include "virutil.h" +#include "virsh-edit.h"

virNWFilterPtr virshCommandOptNWFilterBy(vshControl *ctl, const vshCmd *cmd, @@ -404,6 +405,41 @@ static const vshCmdOptDef opts_nwfilter_edit[] = { {.name = NULL} };

+typedef struct virshEditNWFilterStruct +{ + vshControl *ctl; + virNWFilterPtr nwfilter; + virNWFilterPtr *nwfilter_edited; + virshControlPtr priv; + bool *ret; +} virshEditNWFilterStruct; + +static char * +virshEditNWFilterGetXML(void *args) +{ + virshEditNWFilterStruct *str = (virshEditNWFilterStruct*)args; + return virNWFilterGetXMLDesc(str->nwfilter, 0); +} + +static void +virshEditNWFilterNotChanged(void *args) +{ + virshEditNWFilterStruct *str = (virshEditNWFilterStruct*)args; + vshPrint(str->ctl, _("Network filter %s XML configuration not changed.\n"), + virNWFilterGetName(str->nwfilter)); + *(str->ret) = true; +} + +static bool +virshEditNWFilterDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditNWFilterStruct *str = (virshEditNWFilterStruct*)args; + *(str->nwfilter_edited) = virNWFilterDefineXML(str->priv->conn, doc_edited); + return *(str->nwfilter_edited); +} + static bool cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd) { @@ -415,20 +451,20 @@ cmdNWFilterEdit(vshControl *ctl, const vshCmd *cmd) nwfilter = virshCommandOptNWFilter(ctl, cmd, NULL); if (nwfilter == NULL) goto cleanup; - -#define EDIT_GET_XML virNWFilterGetXMLDesc(nwfilter, 0) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Network filter %s XML " \ - "configuration not changed.\n"), \ - virNWFilterGetName(nwfilter)); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (nwfilter_edited = virNWFilterDefineXML(priv->conn, doc_edited)) -#include "virsh-edit.c" - + { + virshEditNWFilterStruct editNWFilterArgs = { + ctl, nwfilter, &nwfilter_edited, priv, &ret + }; + virshEditControl editCtl = { + &virshEditNWFilterGetXML, + &virshEditNWFilterNotChanged, + &virshEditNWFilterDefine, + NULL, + &editNWFilterArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } vshPrint(ctl, _("Network filter %s XML configuration edited.\n"), virNWFilterGetName(nwfilter_edited));

diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index cf5a8f3..740a130 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -32,6 +32,7 @@ #include "virfile.h" #include "conf/storage_conf.h" #include "virstring.h" +#include "virsh-edit.h"

virStoragePoolPtr virshCommandOptPoolBy(vshControl *ctl, const vshCmd *cmd, const char *optname, @@ -1791,6 +1792,42 @@ static const vshCmdOptDef opts_pool_edit[] = { {.name = NULL} };

+typedef struct virshEditStoragePoolStruct +{ + vshControl *ctl; + virStoragePoolPtr pool; + virStoragePoolPtr *pool_edited; + virshControlPtr priv; + bool *ret; + unsigned int flags; +} virshEditStoragePoolStruct; + +static char * +virshEditStoragePoolGetXML(void *args) +{ + virshEditStoragePoolStruct *str = (virshEditStoragePoolStruct*)args; + return virStoragePoolGetXMLDesc(str->pool, str->flags); +} + +static void +virshEditStoragePoolNotChanged(void *args) +{ + virshEditStoragePoolStruct *str = (virshEditStoragePoolStruct*)args; + vshPrint(str->ctl, _("Pool %s XML configuration not changed.\n"), + virStoragePoolGetName(str->pool)); + *(str->ret) = true; +} + +static bool +virshEditStoragePoolDefine(void *args, char *doc_edited, char *doc) +{ + if (!doc) + return false; + virshEditStoragePoolStruct *str = (virshEditStoragePoolStruct*)args; + return (*(str->pool_edited) = + virStoragePoolDefineXML(str->priv->conn, doc_edited, 0)); +} + static bool cmdPoolEdit(vshControl *ctl, const vshCmd *cmd) { @@ -1816,19 +1853,20 @@ cmdPoolEdit(vshControl *ctl, const vshCmd *cmd) } else { VIR_FREE(tmp_desc); } - -#define EDIT_GET_XML virStoragePoolGetXMLDesc(pool, flags) -#define EDIT_NOT_CHANGED \ - do { \ - vshPrint(ctl, _("Pool %s XML configuration not changed.\n"), \ - virStoragePoolGetName(pool)); \ - ret = true; \ - goto edit_cleanup; \ - } while (0) -#define EDIT_DEFINE \ - (pool_edited = virStoragePoolDefineXML(priv->conn, doc_edited, 0)) -#include "virsh-edit.c" - + { + virshEditStoragePoolStruct editStoragePoolArgs = { + ctl, pool, &pool_edited, priv, &ret, flags + }; + virshEditControl editCtl = { + &virshEditStoragePoolGetXML, + &virshEditStoragePoolNotChanged, + &virshEditStoragePoolDefine, + NULL, + &editStoragePoolArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } vshPrint(ctl, _("Pool %s XML configuration edited.\n"), virStoragePoolGetName(pool_edited));

diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c index 3ab2104..908dcfd 100644 --- a/tools/virsh-snapshot.c +++ b/tools/virsh-snapshot.c @@ -41,6 +41,7 @@ #include "virstring.h" #include "virxml.h" #include "conf/snapshot_conf.h" +#include "virsh-edit.h"

/* Helper for snapshot-create and snapshot-create-as */ static bool @@ -548,6 +549,47 @@ static const vshCmdOptDef opts_snapshot_edit[] = { {.name = NULL} };

+typedef struct virshEditSnapshotStruct +{ + vshControl *ctl; + virDomainSnapshotPtr snapshot; + virDomainSnapshotPtr *edited; + virDomainPtr dom; + bool *ret; + unsigned int getxml_flags; + unsigned int *define_flags; + const char *name; +} virshEditSnapshotStruct; + +static char * +virshEditSnapshotGetXML(void *args) +{ + virshEditSnapshotStruct *str = (virshEditSnapshotStruct*)args; + return virDomainSnapshotGetXMLDesc(str->snapshot, str->getxml_flags); +} + +static void +virshEditSnapshotNotChanged(void *args) +{ + virshEditSnapshotStruct *str = (virshEditSnapshotStruct*)args; + /* Depending on flags, we re-edit even if XML is unchanged. */ + if (!(*(str->define_flags) & VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT)) { + vshPrint(str->ctl, _("Snapshot %s XML configuration not changed.\n"), + str->name); + *(str->ret) = true; + } +} + +static bool +virshEditSnapshotDefine(void *args, char *doc_edited, char *doc) +{ + virshEditSnapshotStruct *str = (virshEditSnapshotStruct*)args; + (strstr(doc, "<state>disk-snapshot</state>") ? + *(str->define_flags) |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY : 0);

This doesn't have to be one-liner (like it had to be before), this is not very readable.

+ return (*(str->edited) = virDomainSnapshotCreateXML(str->dom, doc_edited, + *(str->define_flags))); +} + static bool cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) { @@ -574,26 +616,20 @@ cmdSnapshotEdit(vshControl *ctl, const vshCmd *cmd) if (virshLookupSnapshot(ctl, cmd, "snapshotname", false, dom, &snapshot, &name) < 0) goto cleanup; - -#define EDIT_GET_XML \ - virDomainSnapshotGetXMLDesc(snapshot, getxml_flags) -#define EDIT_NOT_CHANGED \ - do { \ - /* 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 edit_cleanup; \ - } \ - } while (0) -#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) -#include "virsh-edit.c" - + { + virshEditSnapshotStruct editSnapshotArgs = { + ctl, snapshot, &edited, dom, &ret, getxml_flags, &define_flags, name + }; + virshEditControl editCtl = { + &virshEditSnapshotGetXML, + &virshEditSnapshotNotChanged, + &virshEditSnapshotDefine, + NULL, + &editSnapshotArgs, + }; + if (!virshEdit(ctl, &editCtl)) + goto cleanup; + } edited_name = virDomainSnapshotGetName(edited); if (STREQ(name, edited_name)) { vshPrint(ctl, _("Snapshot %s edited.\n"), name); -- 2.1.4

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list