[libvirt] [PATCH v2] virsh: add [--domain DOMAIN] option to domxml-to-native DOMAIN COMMAND

[Martin Kletzander]

On Mon, May 29, 2017 at 02:08:53PM -0400, Daniel Liu wrote:
>The option allows someone to run domain-to-native on already existing
> domain without the need of supplying their XML.  It is basically
> wrapper around 'virsh dumpxml  | virsh domxml-to-native /dev/stdin'.
>Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=835476
>---
> tools/virsh-domain.c | 52 ++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 40 insertions(+), 12 deletions(-)
>

Just few things I forgot to sent, so they remained in my drafts folder.

>diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>index ccb514ef9..929f9c896 100644
>--- a/tools/virsh-domain.c
>+++ b/tools/virsh-domain.c
>@@ -9859,30 +9863,54 @@ static const vshCmdOptDef opts_domxmltonative[] = {
> static bool
> cmdDomXMLToNative(vshControl *ctl, const vshCmd *cmd)
> {
>-    bool ret = true;
>+    bool ret = false;
>     const char *format = NULL;
>-    const char *xmlFile = NULL;
>-    char *configData;
>-    char *xmlData;
>+    const char *domain = NULL;
>+    const char *xml = NULL;
>+    char *xmlData = NULL;
>+    char *configData = NULL;
>     unsigned int flags = 0;
>     virshControlPtr priv = ctl->privData;
>+    virDomainPtr dom = NULL;
>
>     if (vshCommandOptStringReq(ctl, cmd, "format", &format) < 0 ||
>-        vshCommandOptStringReq(ctl, cmd, "xml", &xmlFile) < 0)
>+        vshCommandOptStringReq(ctl, cmd, "xml", &xml) < 0 ||
>+        vshCommandOptStringReq(ctl, cmd, "domain", &domain) < 0)

You don't have to do this, you don't use the domain name anywhere, just
the information whether it's present or not.

>         return false;
>
>-    if (virFileReadAll(xmlFile, VSH_MAX_XML_FILE, &xmlData) < 0)
>+    VSH_EXCLUSIVE_OPTIONS_VAR(domain, xml);
>+
>+    if (domain) {
>+        if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>+            return false;


And instead of this, you can just do:

if (vshCommandOptBool("domain") &&
    (!(dom = virshCommandOptDomain(ctl, cmd, NULL))))
    return false;

>+    }
>+
>+    if (dom) {
>+        xmlData = virDomainGetXMLDesc(dom, flags);

xmlData can be NULL after this in case of an error.

>+    } else if (xml) {
>+        if (virFileReadAll(xml, VSH_MAX_XML_FILE, &xmlData) < 0)
>+            goto cleanup;
>+    }
>+
>+    if (!xmlData) {

So this could just be else {} of the last condition and if xmlData is
NULL after calling virDomainGetXMLDesc(), there should be different
error message.

Other things were pointed out already, I guess.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20170531/ab9f3d84/attachment-0001.sig>