8:40 p.m.
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
8:48 p.m.
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
9:07 p.m.
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
11:50 p.m.
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)
>
> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=919826
> + 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
12:49 p.m.
On Mon, Mar 11, 2013 at 7:48 AM, Peter Krempa <pkrempa(a)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
4248
days inactive
4251
days old
4 comments
4 participants
participants (4)
-
Doug Goldstein -
Eric Blake -
Ján Tomko -
Peter Krempa