[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 :(.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]