Re: [libvirt] [PATCH] virsh: fix snapshot-create with no xmlfile

On 03/11/13 13:40, Ján Tomko wrote: > Properly check the return value of vshCommandOptStringReq for xmlfile: > * error out on incorrect input (--xmlfile '') > * use default XML <domainsnapshot/> with no --xmlfile specified > > (Broken by commit b2e8585) > > Bug: https://bugzilla.redhat.com/show_bug.cgi?id=919826 > --- > tools/virsh-snapshot.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tools/virsh-snapshot.c b/tools/virsh-snapshot.c > index ed41014..d994fd9 100644 > --- a/tools/virsh-snapshot.c > +++ b/tools/virsh-snapshot.c > @@ -211,7 +211,9 @@ cmdSnapshotCreate(vshControl *ctl, const vshCmd *cmd) > if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) > goto cleanup; > > - if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) { > + if (vshCommandOptStringReq(ctl, cmd, "xmlfile", &from) < 0) > + goto cleanup; > + if (!from) { > buffer = vshStrdup(ctl, "<domainsnapshot/>"); This fallback code is really weird. We should have rejected missing XML name by default and encourage to use "snapshot-create-as". > } else { > if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { > ACK to this patch, though. We probably have to support that code forever now :(. Peter