[libvirt] [PATCH] virsh: fix snapshot-create with no xmlfile
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/>"); } else { if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) { -- 1.7.12.4
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
On 03/11/13 13:48, Peter Krempa wrote:
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
Thanks, pushed now. Jan
On 03/11/2013 06:48 AM, Peter Krempa wrote:
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)
+ 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".
Except that 'snapshot-create' was invented several releases prior to 'snapshot-create-as', back when snapshots were simple enough that creating without an xml file was common enough to do without needing a second command. We have to keep the older behavior for clients written prior to when snapshot-create-as was introduced. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Mon, Mar 11, 2013 at 7:48 AM, Peter Krempa <pkrempa@redhat.com> wrote:
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
Pushed to v1.0.3-maint Thanks. -- Doug Goldstein
participants (4)
-
Doug Goldstein -
Eric Blake -
Ján Tomko -
Peter Krempa