[libvirt] [PATCH 0/2] fix two issues while doing virsh attach-disk
First issue was caused by virsh generating invalid XML to attach the disk, and the second issue is a off-by-one error that crashes the daemon with 50-90% probability while doing virsh attach-disk dom /dev/zero vdx. Peter Krempa (2): virsh: Don't generate invalid XML in attach-disk command qemu: Fix off-by-one error while unescaping monitor strings src/qemu/qemu_monitor.c | 11 +++-------- tools/virsh.c | 16 ++++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-) -- 1.7.8.6
The attach-disk command used with parameter --cache created an invalid XML snippet as the beginning of the <driver> element was not printed when used solely with --cache and no other attribute to driver. --- tools/virsh.c | 16 ++++++++-------- 1 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 98305c0..1862500 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -14765,18 +14765,18 @@ cmdAttachDisk(vshControl *ctl, const vshCmd *cmd) virBufferAddLit(&buf, " rawio='yes'"); virBufferAddLit(&buf, ">\n"); - if (driver || subdriver) + if (driver || subdriver || cache) { virBufferAsprintf(&buf, " <driver"); - if (driver) - virBufferAsprintf(&buf, " name='%s'", driver); - if (subdriver) - virBufferAsprintf(&buf, " type='%s'", subdriver); - if (cache) - virBufferAsprintf(&buf, " cache='%s'", cache); + if (driver) + virBufferAsprintf(&buf, " name='%s'", driver); + if (subdriver) + virBufferAsprintf(&buf, " type='%s'", subdriver); + if (cache) + virBufferAsprintf(&buf, " cache='%s'", cache); - if (driver || subdriver || cache) virBufferAddLit(&buf, "/>\n"); + } if (source) virBufferAsprintf(&buf, " <source %s='%s'/>\n", -- 1.7.8.6
While unescaping the commands the commands passed through to the monitor function qemuMonitorUnescapeArg() initialized lenght of the input string to strlen()+1 which is fine for alloc but not for iteration of the string. This patch fixes the off-by-one error and drops the pointless check for a single trailing slash that is automaticaly handled by the default branch of switch. --- src/qemu/qemu_monitor.c | 11 +++-------- 1 files changed, 3 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 7084c68..007e7b9 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -161,20 +161,15 @@ char *qemuMonitorUnescapeArg(const char *in) { int i, j; char *out; - int len = strlen(in) + 1; + int len = strlen(in); char next; - if (VIR_ALLOC_N(out, len) < 0) + if (VIR_ALLOC_N(out, len + 1) < 0) return NULL; for (i = j = 0; i < len; ++i) { next = in[i]; if (in[i] == '\\') { - if (len < i + 1) { - /* trailing backslash shouldn't be possible */ - VIR_FREE(out); - return NULL; - } ++i; switch(in[i]) { case 'r': @@ -188,7 +183,7 @@ char *qemuMonitorUnescapeArg(const char *in) next = in[i]; break; default: - /* invalid input */ + /* invalid input (including trailing '\' at end of in) */ VIR_FREE(out); return NULL; } -- 1.7.8.6
On 06/14/2012 02:53 AM, Peter Krempa wrote:
First issue was caused by virsh generating invalid XML to attach the disk, and the second issue is a off-by-one error that crashes the daemon with 50-90% probability while doing virsh attach-disk dom /dev/zero vdx.
Peter Krempa (2): virsh: Don't generate invalid XML in attach-disk command qemu: Fix off-by-one error while unescaping monitor strings
src/qemu/qemu_monitor.c | 11 +++-------- tools/virsh.c | 16 ++++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-)
ACK series. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 06/14/12 13:14, Eric Blake wrote:
On 06/14/2012 02:53 AM, Peter Krempa wrote:
First issue was caused by virsh generating invalid XML to attach the disk, and the second issue is a off-by-one error that crashes the daemon with 50-90% probability while doing virsh attach-disk dom /dev/zero vdx.
Peter Krempa (2): virsh: Don't generate invalid XML in attach-disk command qemu: Fix off-by-one error while unescaping monitor strings
src/qemu/qemu_monitor.c | 11 +++-------- tools/virsh.c | 16 ++++++++-------- 2 files changed, 11 insertions(+), 16 deletions(-)
ACK series.
Thanks; pushed. Peter
participants (2)
-
Eric Blake -
Peter Krempa