Historically the use of the '-desc' multiple argument parameter was not forbidden toghether with '-edit', but use of both together has some unexpected behaviour. Specifically the editor is filled with the contents passed via '-desc' but if the user doesn't change the text in any way virsh will claim that the description was not chaged even if it differs from the currently set description. Similarly, when the user would edit the description provided via 'desc' so that it's identical with the one configured for the domain, virsh would claim that it was updated: # virsh desc cd No description for domain: cd # EDITOR=true virsh desc cd --edit "test desc" Domain description not changed After the fix: # virsh desc cd No description for domain: cd # EDITOR=true virsh desc cd --edit "test desc" Domain description updated successfully # EDITOR=true virsh desc cd --edit "test desc" Domain description not changed Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx> --- tools/virsh-domain.c | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 89ad45dbf0..d5fd8be7c3 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8335,7 +8335,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) int state; int type; - g_autofree char *desc = NULL; + g_autofree char *descArg = NULL; const vshCmdOpt *opt = NULL; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; @@ -8367,14 +8367,17 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) virBufferTrim(&buf, " "); - desc = virBufferContentAndReset(&buf); + descArg = virBufferContentAndReset(&buf); - if (edit || desc) { - if (!desc) { - desc = virshGetDomainDescription(ctl, dom, title, queryflags); - if (!desc) - return false; - } + if (edit || descArg) { + g_autofree char *descDom = NULL; + g_autofree char *descNew = NULL; + + if (!(descDom = virshGetDomainDescription(ctl, dom, title, queryflags))) + return false; + + if (!descArg) + descArg = g_strdup(descDom); if (edit) { g_autoptr(vshTempFile) tmp = NULL; @@ -8382,7 +8385,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) char *tmpstr; /* Create and open the temporary file. */ - if (!(tmp = vshEditWriteToTempFile(ctl, desc))) + if (!(tmp = vshEditWriteToTempFile(ctl, descArg))) return false; /* Start the editor. */ @@ -8402,7 +8405,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) *tmpstr = '\0'; /* Compare original XML with edited. Has it changed at all? */ - if (STREQ(desc, desc_edited)) { + if (STREQ(descDom, desc_edited)) { if (title) vshPrintExtra(ctl, "%s", _("Domain title not changed\n")); else @@ -8411,11 +8414,12 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) return true; } - VIR_FREE(desc); - desc = g_steal_pointer(&desc_edited); + descNew = g_steal_pointer(&desc_edited); + } else { + descNew = g_steal_pointer(&descArg); } - if (virDomainSetMetadata(dom, type, desc, NULL, NULL, flags) < 0) { + if (virDomainSetMetadata(dom, type, descNew, NULL, NULL, flags) < 0) { if (title) vshError(ctl, "%s", _("Failed to set new domain title")); else @@ -8430,7 +8434,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd) vshPrintExtra(ctl, "%s", _("Domain description updated successfully")); } else { - desc = virshGetDomainDescription(ctl, dom, title, queryflags); + g_autofree char *desc = virshGetDomainDescription(ctl, dom, title, queryflags); if (!desc) return false; -- 2.35.1