[libvirt] [PATCH] qemu_monitor: Don't output snapshot format argument if type is NULL
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
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
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
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
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@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Peter Krempa