[libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for attach-disk command

[Eric Blake]

On 10/16/2013 10:05 PM, Chen Hanxiao wrote:
> From: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> 
> '--print-xml' option is very useful for doing some test.
> But we had to specify a real domain for it.
> This patch could enable us to specify a fake domain
> when using --print-xml option.
> 
> Signed-off-by: Chen Hanxiao <chenhanxiao at cn.fujitsu.com>
> ---
>  tools/virsh-domain.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 

Just code motion - makes sense to delay probing for the domain until you
know you need it.

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 2aed9f9..565966d 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -528,13 +528,6 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>      if (live)
>          flags |= VIR_DOMAIN_AFFECT_LIVE;
>  
> -    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> -        return false;
> -
> -    if (persistent &&
> -        virDomainIsActive(dom) == 1)
> -        flags |= VIR_DOMAIN_AFFECT_LIVE;
> -
>      if (vshCommandOptStringReq(ctl, cmd, "source", &source) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "target", &target) < 0 ||
>          vshCommandOptStringReq(ctl, cmd, "driver", &driver) < 0 ||
> @@ -672,6 +665,13 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
>          goto cleanup;

Ouch - you can now go to cleanup while dom is still NULL.

>      }
>  
> +    if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
> +        return false;

and this is wrong - if you get here, you've allocated resources that
need the cleanup label.

> +
> +    if (persistent &&
> +        virDomainIsActive(dom) == 1)
> +        flags |= VIR_DOMAIN_AFFECT_LIVE;
> +
>      if (flags)
>          ret = virDomainAttachDeviceFlags(dom, xml, flags);
>      else
> 

ACK with this squashed in, and pushed.

diff --git i/tools/virsh-domain.c w/tools/virsh-domain.c
index 45c4823..b75f331 100644
--- i/tools/virsh-domain.c
+++ w/tools/virsh-domain.c
@@ -666,7 +666,7 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)
     }

     if (!(dom = vshCommandOptDomain(ctl, cmd, NULL)))
-        return false;
+        goto cleanup;

     if (persistent &&
         virDomainIsActive(dom) == 1)
@@ -686,7 +686,8 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd)

  cleanup:
     VIR_FREE(xml);
-    virDomainFree(dom);
+    if (dom)
+        virDomainFree(dom);
     virBufferFreeAndReset(&buf);
     return functionReturn;
 }



-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20131017/c369234b/attachment-0001.sig>