[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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.

Attachment: signature.asc
Description: Digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]