Re: [PATCH] virsh: Teach attach-interface to --print-xml

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

 



On Mon, Sep 07, 2015 at 12:16:23PM +0200, Michal Privoznik wrote:
We have the same argument to many other commands that produce an
XML based on what user typed. But unfortunately attach-interface
was missing it. Maybe nobody hasn't needed it yet. Well, I did

"nobody needed it" or maybe "nobody had needed it" if connected with
the previous sentence, but definitely not "nobody hasn't needed it
yet" :)

just now.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
tools/virsh-domain.c | 20 ++++++++++++++++----
tools/virsh.pod      |  4 ++++
2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index b029b65..516a51e 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -862,6 +862,10 @@ static const vshCmdOptDef opts_attach_interface[] = {
     .type = VSH_OT_BOOL,
     .help = N_("affect current domain")
    },
+    {.name = "print-xml",
+     .type = VSH_OT_BOOL,
+     .help = N_("print XML document rather than attach the interface")
+    },
    {.name = NULL}
};

@@ -938,9 +942,6 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
    if (live)
        flags |= VIR_DOMAIN_AFFECT_LIVE;

-    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
-        return false;
-
    if (persistent &&
        virDomainIsActive(dom) == 1)

You're referencing the domain here, that's not cool when @dom is
uninitialized.  Either move all the logic after the printing or keep
it here making it working only in proper cases (which is probably no
what you want).

Otherwise looks fine.

        flags |= VIR_DOMAIN_AFFECT_LIVE;
@@ -1051,6 +1052,7 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
        virBufferAddLit(&buf, "</bandwidth>\n");
    }

+    virBufferAdjustIndent(&buf, -2);
    virBufferAddLit(&buf, "</interface>\n");

    if (virBufferError(&buf)) {
@@ -1060,6 +1062,15 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)

    xml = virBufferContentAndReset(&buf);

+    if (vshCommandOptBool(cmd, "print-xml")) {
+        vshPrint(ctl, "%s", xml);
+        functionReturn = true;
+        goto cleanup;
+    }
+
+    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
+        goto cleanup;
+
    if (flags || current)
        ret = virDomainAttachDeviceFlags(dom, xml, flags);
    else
@@ -1075,7 +1086,8 @@ cmdAttachInterface(vshControl *ctl, const vshCmd *cmd)
    }

 cleanup:
-    virDomainFree(dom);
+    if (dom)
+        virDomainFree(dom);
    virBufferFreeAndReset(&buf);
    return functionReturn;
}
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 83c445d3..0212e7a 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -2507,6 +2507,7 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>.
[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]]
[I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>]
[I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>]
+[I<--print-xml>]

Attach a new network interface to the domain.  I<type> can be
I<network> to indicate connection via a libvirt virtual network, or
@@ -2536,6 +2537,9 @@ kilobytes in a single burst at I<peak> speed as described in the
Network XML documentation at
L<http://libvirt.org/formatnetwork.html#elementQoS>.

+If I<--print-xml> is specified, then the XML of the interface that would be
+attached is printed instead.
+
If I<--live> is specified, affect a running domain.
If I<--config> is specified, affect the next startup of a persistent domain.
If I<--current> is specified, affect the current domain state.
--
2.4.6

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]