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>
This commit is contained in:
Peter Krempa 2022-03-02 13:47:09 +01:00
parent 420488790e
commit c7dca225e5

View File

@ -8335,7 +8335,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
int state; int state;
int type; int type;
g_autofree char *desc = NULL; g_autofree char *descArg = NULL;
const vshCmdOpt *opt = NULL; const vshCmdOpt *opt = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT; unsigned int flags = VIR_DOMAIN_AFFECT_CURRENT;
@ -8367,14 +8367,17 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
virBufferTrim(&buf, " "); virBufferTrim(&buf, " ");
desc = virBufferContentAndReset(&buf); descArg = virBufferContentAndReset(&buf);
if (edit || desc) { if (edit || descArg) {
if (!desc) { g_autofree char *descDom = NULL;
desc = virshGetDomainDescription(ctl, dom, title, queryflags); g_autofree char *descNew = NULL;
if (!desc)
if (!(descDom = virshGetDomainDescription(ctl, dom, title, queryflags)))
return false; return false;
}
if (!descArg)
descArg = g_strdup(descDom);
if (edit) { if (edit) {
g_autoptr(vshTempFile) tmp = NULL; g_autoptr(vshTempFile) tmp = NULL;
@ -8382,7 +8385,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
char *tmpstr; char *tmpstr;
/* Create and open the temporary file. */ /* Create and open the temporary file. */
if (!(tmp = vshEditWriteToTempFile(ctl, desc))) if (!(tmp = vshEditWriteToTempFile(ctl, descArg)))
return false; return false;
/* Start the editor. */ /* Start the editor. */
@ -8402,7 +8405,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
*tmpstr = '\0'; *tmpstr = '\0';
/* Compare original XML with edited. Has it changed at all? */ /* Compare original XML with edited. Has it changed at all? */
if (STREQ(desc, desc_edited)) { if (STREQ(descDom, desc_edited)) {
if (title) if (title)
vshPrintExtra(ctl, "%s", _("Domain title not changed\n")); vshPrintExtra(ctl, "%s", _("Domain title not changed\n"));
else else
@ -8411,11 +8414,12 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
return true; return true;
} }
VIR_FREE(desc); descNew = g_steal_pointer(&desc_edited);
desc = 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) if (title)
vshError(ctl, "%s", _("Failed to set new domain title")); vshError(ctl, "%s", _("Failed to set new domain title"));
else else
@ -8430,7 +8434,7 @@ cmdDesc(vshControl *ctl, const vshCmd *cmd)
vshPrintExtra(ctl, "%s", _("Domain description updated successfully")); vshPrintExtra(ctl, "%s", _("Domain description updated successfully"));
} else { } else {
desc = virshGetDomainDescription(ctl, dom, title, queryflags); g_autofree char *desc = virshGetDomainDescription(ctl, dom, title, queryflags);
if (!desc) if (!desc)
return false; return false;