Re: [libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for attach-disk command
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: Friday, October 18, 2013 5:45 AM
To: Chen Hanxiao; libvir-list(a)redhat.com
Subject: Re: [libvirt] [PATCH]virsh: improve usability of '--print-xml' flag for
attach-disk command
Just code motion - makes sense to delay probing for the domain until you
know you need it.
Wow
> 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.
Thanks for your kindly help.
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