> -----Original Message----- > From: Eric Blake [mailto:eblake@xxxxxxxxxx] > Sent: Friday, October 18, 2013 5:45 AM > To: Chen Hanxiao; libvir-list@xxxxxxxxxx > Subject: Re: [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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list