virsh: cmdDesc: Fix logic when '-edit' is used along with 'desc' argument

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@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 89ad45dbf040c949d70699992454eea4a2602623..d5fd8be7c368e83a3bd3211062645c12ba397bf4 100644 (file)
--- 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;