6:49 a.m.
If the snapshot format type string was NULL, the JSON framework created
an invalid JSON string.
---
The other option would be to fix qemuMonitorJSONMakeCommandRaw that string arguments with
a NULL
argument would suppress outputing the complete option, but I'm afraid of breaking
something.
Background:
http://www.redhat.com/archives/libvir-list/2012-March/msg01198.html
src/qemu/qemu_monitor_json.c | 27 +++++++++++++++++++--------
1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eeeb6a6..6fce58a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3112,14 +3112,25 @@ qemuMonitorJSONDiskSnapshot(qemuMonitorPtr mon, virJSONValuePtr
actions,
virJSONValuePtr cmd;
virJSONValuePtr reply = NULL;
- cmd = qemuMonitorJSONMakeCommandRaw(actions != NULL,
- "blockdev-snapshot-sync",
- "s:device", device,
- "s:snapshot-file", file,
- "s:format", format,
- reuse ? "s:mode" : NULL,
- reuse ? "existing" : NULL,
- NULL);
+ if (format) {
+ cmd = qemuMonitorJSONMakeCommandRaw(actions != NULL,
+ "blockdev-snapshot-sync",
+ "s:device", device,
+ "s:snapshot-file", file,
+ "s:format", format,
+ reuse ? "s:mode" : NULL,
+ reuse ? "existing" : NULL,
+ NULL);
+ } else {
+ cmd = qemuMonitorJSONMakeCommandRaw(actions != NULL,
+ "blockdev-snapshot-sync",
+ "s:device", device,
+ "s:snapshot-file", file,
+ reuse ? "s:mode" : NULL,
+ reuse ? "existing" : NULL,
+ NULL);
+ }
+
if (!cmd)
return -1;
--
1.7.3.4
7:18 a.m.
On 03/27/2012 05:49 AM, Peter Krempa wrote:
If the snapshot format type string was NULL, the JSON framework
created
an invalid JSON string.
---
The other option would be to fix qemuMonitorJSONMakeCommandRaw that string arguments with
a NULL
argument would suppress outputing the complete option, but I'm afraid of breaking
something.
Background:
http://www.redhat.com/archives/libvir-list/2012-March/msg01198.html
Thanks for tracking this down.
Actually, I'd rather fix qemu_driver.c to guarantee that format is
always non-NULL (omitting the format argument means that qemu either
probes the file or hard-codes a default, and that carries risk, since in
the past, we've had CVEs where autoprobing of a raw file can mistakenly
result in treating the file as qcow2 and cause SELinux labeling of
unintended files).
I'll propose a counter-proposal patch later this morning. Let's wait
until we have both patches to compare before deciding which one to push.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:37 a.m.
New subject: [libvirt] [PATCH] snapshot: don't pass NULL to QMP command creation
Commit d42a2ff caused a regression in creating a disk-only snapshot
of a qcow2 disk; by passing the wrong variable to the monitor call,
libvirt ended up creating JSON that looked like "format":null instead
of the intended "format":"qcow2".
To make it easier to diagnose this in the future, make JSON creation
error out if "s:arg" is paired with NULL (it is still possible to
use "n:arg" in the rare cases where qemu will accept a null).
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Pass correct value.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommandRaw):
Improve error message.
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_monitor_json.c | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ea772cf..1f4dfe6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9894,7 +9894,7 @@ qemuDomainSnapshotCreateSingleDiskActive(struct qemud_driver
*driver,
/* create the actual snapshot */
ret = qemuMonitorDiskSnapshot(priv->mon, actions, device, source,
- driverType, reuse);
+ snap->driverType, reuse);
virDomainAuditDisk(vm, disk->src, source, "snapshot", ret >= 0);
if (ret < 0)
goto cleanup;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index eeeb6a6..8c028b9 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -417,6 +417,12 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char *cmdname, ...)
switch (type) {
case 's': {
char *val = va_arg(args, char *);
+ if (!val) {
+ qemuReportError(VIR_ERR_INTERNAL_ERROR,
+ _("argument key '%s' must not have null
value"),
+ key);
+ goto error;
+ }
ret = virJSONValueObjectAppendString(jargs, key, val);
} break;
case 'i': {
--
1.7.7.6
10:33 a.m.
New subject: [libvirt] [PATCH] snapshot: don't pass NULL to QMP command creation
Dňa 27.3.2012 16:37, Eric Blake wrote / napísal(a):
Commit d42a2ff caused a regression in creating a disk-only snapshot
of a qcow2 disk; by passing the wrong variable to the monitor call,
libvirt ended up creating JSON that looked like "format":null instead
of the intended "format":"qcow2".
To make it easier to diagnose this in the future, make JSON creation
error out if "s:arg" is paired with NULL (it is still possible to
use "n:arg" in the rare cases where qemu will accept a null).
* src/qemu/qemu_driver.c
(qemuDomainSnapshotCreateSingleDiskActive): Pass correct value.
* src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommandRaw):
Improve error message.
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_monitor_json.c | 6 ++++++
2 files changed, 7 insertions(+), 1 deletions(-)
Definitely a better solution. ACK
Peter
10:43 a.m.
New subject: [libvirt] [PATCH] snapshot: don't pass NULL to QMP command creation
On 03/27/2012 09:33 AM, Peter Krempa wrote:
Dňa 27.3.2012 16:37, Eric Blake wrote / napísal(a):
> Commit d42a2ff caused a regression in creating a disk-only snapshot
> of a qcow2 disk; by passing the wrong variable to the monitor call,
> libvirt ended up creating JSON that looked like "format":null instead
> of the intended "format":"qcow2".
>
> To make it easier to diagnose this in the future, make JSON creation
> error out if "s:arg" is paired with NULL (it is still possible to
> use "n:arg" in the rare cases where qemu will accept a null).
>
> * src/qemu/qemu_driver.c
> (qemuDomainSnapshotCreateSingleDiskActive): Pass correct value.
> * src/qemu/qemu_monitor_json.c (qemuMonitorJSONMakeCommandRaw):
> Improve error message.
> ---
> src/qemu/qemu_driver.c | 2 +-
> src/qemu/qemu_monitor_json.c | 6 ++++++
> 2 files changed, 7 insertions(+), 1 deletions(-)
Definitely a better solution. ACK
Thanks; pushed.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4626
days inactive
4626
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Peter Krempa