]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: don't munge user input during block commit
authorEric Blake <eblake@redhat.com>
Thu, 6 Mar 2014 23:33:16 +0000 (16:33 -0700)
committerEric Blake <eblake@redhat.com>
Tue, 11 Mar 2014 23:53:19 +0000 (17:53 -0600)
While investigating https://bugzilla.redhat.com/show_bug.cgi?id=1061827
I noticed that we pass user input unscathed for block-pull, but
always pass a canonical absolute name through for block-commit.
[Note that we probably _ought_ to validate that the user's request
for block-pull actually matches the backing chain, the way we already
do for block-commit - but that's a separate issue.  Further note that
the ability to pass user input through unscathed allows backdoors
such as specifying a backing image that is a network URI such as
a gluster disk, instead of forcing things to the local file system;
which is an area still under active investigation on whether libvirt
needs to behave differently for network disks.]

Since qemu may write the name that the user passed in as the backing
file, a user may have a reason to want a relative file name passed
through to qemu, and always munging things to absolute prevents that.

Put another way, if you have the backing chain:

[A] <- [B(back=./A)] <- [C(back=./B)]

and commit B into A (virsh blockcommit $dom vda --base A --top B),
the metadata of C will have to be re-written. But should it be
rewritten as [C(back=./A)] or as [C(back=/path/to/A)]?  Still up in
the air is whether qemu's decision should be based on whether B
and/or C had relative paths, or on whether the --base and/or
--top arguments to the command were relative paths; but if we always
pass a canonical name, we've prevented the spelling of the command
arguments from being part of the hueristics that qemu uses.

I also audited the code, and verified that we never call
qemuMonitorBlockCommit() with a NULL base, either before or after
the change to qemu_driver.c.

* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Preserve user's
spelling, since absolute vs. relative matters to qemu.
* src/qemu/qemu_monitor.h (qemuMonitorBlockCommit): Base is never
null.
* src/qemu/qemu_monitor.c (qemuMonitorBlockCommit): Likewise.
* src/qemu/qemu_monitor_json.h (qemuMonitorJSONBlockCommit):
Likewise.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONBlockCommit):
Likewise.

Signed-off-by: Eric Blake <eblake@redhat.com>
src/qemu/qemu_driver.c
src/qemu/qemu_monitor.c
src/qemu/qemu_monitor.h
src/qemu/qemu_monitor_json.c
src/qemu/qemu_monitor_json.h

index aa7b129b3bbc5bd4cda920ea17b28a37f80134b1..b167949d9ecb4aab29a594897d78def288dd1d5a 100644 (file)
@@ -15437,10 +15437,15 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
                                            VIR_DISK_CHAIN_READ_WRITE) < 0))
         goto endjob;
 
-    /* Start the commit operation.  */
+    /* Start the commit operation.  Pass the user's original spelling,
+     * if any, through to qemu, since qemu may behave differently
+     * depending on whether the input was specified as relative or
+     * absolute (that is, our absolute top_canon may do the wrong
+     * thing if the user specified a relative name). */
     qemuDomainObjEnterMonitor(driver, vm);
-    ret = qemuMonitorBlockCommit(priv->mon, device, top_canon, base_canon,
-                                 bandwidth);
+    ret = qemuMonitorBlockCommit(priv->mon, device,
+                                 top ? top : top_canon,
+                                 base ? base : base_canon, bandwidth);
     qemuDomainObjExitMonitor(driver, vm);
 
 endjob:
index a2769dbe2b2d1adf6f99bb68181a74bf3c6f7753..e4707b7c05d4e29f92230fcf2fb6c7649e6cd0d1 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * qemu_monitor.c: interaction with QEMU monitor console
  *
- * Copyright (C) 2006-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -3188,7 +3188,7 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
     unsigned long long speed;
 
     VIR_DEBUG("mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld",
-              mon, device, NULLSTR(top), NULLSTR(base), bandwidth);
+              mon, device, top, base, bandwidth);
 
     /* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
      * limited to LLONG_MAX also for unsigned values */
index eabf000969f1aedb6ded5c2c60cdf0d739516d4b..7bb465bea9fdec436b6277ed63a807f7af0ac447 100644 (file)
@@ -647,7 +647,8 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
                            const char *top,
                            const char *base,
                            unsigned long bandwidth)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+    ATTRIBUTE_NONNULL(4);
 
 int qemuMonitorArbitraryCommand(qemuMonitorPtr mon,
                                 const char *cmd,
index 7d6b88b3261f6b21ce973c02bc4355de3d9d7e9e..dbc04f68d2eea83cf589ed136d83f3531042a62c 100644 (file)
@@ -3307,7 +3307,7 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
                                      "s:device", device,
                                      "U:speed", speed,
                                      "s:top", top,
-                                     base ? "s:base" : NULL, base,
+                                     "s:base", base,
                                      NULL);
     if (!cmd)
         return -1;
index a93c51eeb0135b47db4dffc1a4ccb1b3b6792f1f..ef71588269b06853b2d4959ac49d438c1a927eac 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * qemu_monitor_json.h: interaction with QEMU monitor console
  *
- * Copyright (C) 2006-2009, 2011-2013 Red Hat, Inc.
+ * Copyright (C) 2006-2009, 2011-2014 Red Hat, Inc.
  * Copyright (C) 2006 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -258,7 +258,8 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
                                const char *top,
                                const char *base,
                                unsigned long long bandwidth)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+    ATTRIBUTE_NONNULL(4);
 
 int qemuMonitorJSONArbitraryCommand(qemuMonitorPtr mon,
                                     const char *cmd_str,