[libvirt] [PATCH] Fix modifying disk devices in qemu driver
When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b8d9c92..55e6314 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4278,12 +4278,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob; if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4307,6 +4308,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0; if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom); -- 1.7.5.1
At 05/11/2011 07:12 PM, Markus Groß Write:
When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 14 ++++++++++---- 1 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b8d9c92..55e6314 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4278,12 +4278,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4307,6 +4308,11 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0;
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) {
We should free dev before calling virDomainDeviceDefParse().
+ dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom);
v2: - free dev before using it again. When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdb3b30..ee06c73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4408,12 +4408,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob; if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4437,6 +4438,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0; if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { + /* If dev exists it was created to modify the domain config. Free it, */ + virDomainDeviceDefFree(dev); + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom); -- 1.7.5.1
At 05/18/2011 03:22 PM, Markus Groß Write:
v2: - free dev before using it again.
When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdb3b30..ee06c73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4408,12 +4408,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4437,6 +4438,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0;
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { + /* If dev exists it was created to modify the domain config. Free it, */ + virDomainDeviceDefFree(dev); + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom);
ACK
Am Mittwoch 18 Mai 2011 09:22:43 schrieb Markus Groß:
v2: - free dev before using it again.
When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdb3b30..ee06c73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4408,12 +4408,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4437,6 +4438,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0;
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { + /* If dev exists it was created to modify the domain config. Free it, */
s/,/./
+ virDomainDeviceDefFree(dev); + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom);
On Wed, May 18, 2011 at 11:34:36AM +0200, Markus Groß wrote:
Am Mittwoch 18 Mai 2011 09:22:43 schrieb Markus Groß:
v2: - free dev before using it again.
When modifying the disk devices of a live domain and the domain configuration, the function qemuDomainAttachDeviceConfig first sets dev->data->disk to NULL. Later qemuDomainAttachDeviceLive accesses dev->data.disk and causes a segfault. --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fdb3b30..ee06c73 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4408,12 +4408,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, "%s", _("cannot modify device on transient domain")); goto endjob; } - dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, - VIR_DOMAIN_XML_INACTIVE); - if (dev == NULL) - goto endjob;
if (flags & VIR_DOMAIN_DEVICE_MODIFY_CONFIG) { + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + /* Make a copy for updated domain. */ vmdef = virDomainObjCopyPersistentDef(driver->caps, vm); if (!vmdef) @@ -4437,6 +4438,13 @@ qemuDomainModifyDeviceFlags(virDomainPtr dom, const char *xml, ret = 0;
if (!ret && (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE)) { + /* If dev exists it was created to modify the domain config. Free it, */
s/,/./
+ virDomainDeviceDefFree(dev); + dev = virDomainDeviceDefParse(driver->caps, vm->def, xml, + VIR_DOMAIN_XML_INACTIVE); + if (dev == NULL) + goto endjob; + switch (action) { case QEMU_DEVICE_ATTACH: ret = qemuDomainAttachDeviceLive(vm, dev, dom);
Okay, pushed with that small fix, thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (3)
-
Daniel Veillard -
Markus Groß -
Wen Congyang