[libvirt] [PATCH 0/6] SCSI-Disk-Hotremove
[PATCH 0/6] SCSI-Disk-Hotremove This series adds support for hot-removing SCSI disks when qemu supports the drive/device model. It also contains fixes for SCSI controller hotremoval and the build process. Cheers, Wolfgang --- Siemens AG, Corporate Technology, CT T TC 4 Corporate Competence Centre Embedded Linux Makefile.am | 7 +-- qemu/qemu_driver.c | 97 +++++++++++++++++++++++++++++++++++++++++++---- qemu/qemu_monitor.c | 13 ++++++ qemu/qemu_monitor.h | 3 + qemu/qemu_monitor_json.c | 24 +++++++++++ qemu/qemu_monitor_json.h | 3 + qemu/qemu_monitor_text.c | 40 +++++++++++++++++++ qemu/qemu_monitor_text.h | 3 + 8 files changed, 180 insertions(+), 10 deletions(-)
--- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e4b493..1586e35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5475,7 +5475,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, return cont; } - /* No SCSI controller present, for back compatability we + /* No SCSI controller present, for backward compatibility we * now hotplug a controller */ if (VIR_ALLOC(cont) < 0) { virReportOOMError(); -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:14PM +0100, Wolfgang Mauerer wrote:
--- src/qemu/qemu_driver.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1e4b493..1586e35 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5475,7 +5475,7 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, return cont; }
- /* No SCSI controller present, for back compatability we + /* No SCSI controller present, for backward compatibility we * now hotplug a controller */ if (VIR_ALLOC(cont) < 0) { virReportOOMError();
Okay, applied and pushed ! 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/
... not just when using the remote driver: libvirt.c always depends on it. This unbreaks builds with --without-remote Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/Makefile.am | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 2051e5d..8f0fd30 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -334,8 +334,11 @@ libvirt_la_LIBADD += libvirt_driver.la libvirt_driver_la_SOURCES = $(DRIVER_SOURCES) libvirt_driver_la_CFLAGS = $(NUMACTL_CFLAGS) \ + $(GNUTLS_CFLAGS) \ -I@top_srcdir@/src/conf -libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) +libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) \ + $(GNUTLS_LIBS) + USED_SYM_FILES = libvirt_private.syms @@ -362,11 +365,9 @@ noinst_LTLIBRARIES += libvirt_driver_remote.la libvirt_la_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ - $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ -I@top_srcdir@/src/conf libvirt_driver_remote_la_LDFLAGS = \ - $(GNUTLS_LIBS) \ $(SASL_LIBS) if WITH_DRIVER_MODULES libvirt_driver_remote_la_LDFLAGS += -module -avoid-version -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:15PM +0100, Wolfgang Mauerer wrote:
... not just when using the remote driver: libvirt.c always depends on it. This unbreaks builds with --without-remote
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/Makefile.am | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
Hmm, I think we also need to make the TLS threads code in libvirt.c conditional on WITH_REMOTE, since we don't want that compiled at all if the remote driver isn't present
diff --git a/src/Makefile.am b/src/Makefile.am index 2051e5d..8f0fd30 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -334,8 +334,11 @@ libvirt_la_LIBADD += libvirt_driver.la libvirt_driver_la_SOURCES = $(DRIVER_SOURCES)
libvirt_driver_la_CFLAGS = $(NUMACTL_CFLAGS) \ + $(GNUTLS_CFLAGS) \ -I@top_srcdir@/src/conf -libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) +libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) \ + $(GNUTLS_LIBS) +
USED_SYM_FILES = libvirt_private.syms
@@ -362,11 +365,9 @@ noinst_LTLIBRARIES += libvirt_driver_remote.la libvirt_la_LIBADD += libvirt_driver_remote.la endif libvirt_driver_remote_la_CFLAGS = \ - $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ -I@top_srcdir@/src/conf libvirt_driver_remote_la_LDFLAGS = \ - $(GNUTLS_LIBS) \ $(SASL_LIBS) if WITH_DRIVER_MODULES libvirt_driver_remote_la_LDFLAGS += -module -avoid-version
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 26, 2010 at 02:09:15PM +0100, Wolfgang Mauerer wrote:
... not just when using the remote driver: libvirt.c always depends on it. This unbreaks builds with --without-remote
Hum, there is something weird here, I did configure without remote and built fine without that patch configure: Remote: no [...] configure: gnutls: -lgnutls -lgcrypt [...] virsh links fine even though it doesn't explicitely reference GNUTLS_LIBS in the linking section, so it seems everything needed is provided at the libvirt.so level (I also looked for DT_NEEDED of libvirt.so and this doesn't include gcrypt so I'm a bit puzzled). What are the configure/autogen flags you used to get to the problem ? 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/
We must not delete the data part when the operation succeeds because it is required later on. The correct pattern to handle the parsed representation of the device information on success is dev->data.controller = NULL; virDomainDeviceDefFree(dev);, which leaves the structure pointed at by data in memory. Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1586e35..5394ff5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6111,6 +6111,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom, if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { ret = qemudDomainAttachPciControllerDevice(driver, vm, dev->data.controller, qemuCmdFlags); + if (ret == 0) + dev->data.controller = NULL; } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotplugged."), -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:16PM +0100, Wolfgang Mauerer wrote:
We must not delete the data part when the operation succeeds because it is required later on. The correct pattern to handle the parsed representation of the device information on success is dev->data.controller = NULL; virDomainDeviceDefFree(dev);, which leaves the structure pointed at by data in memory.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1586e35..5394ff5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6111,6 +6111,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom, if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { ret = qemudDomainAttachPciControllerDevice(driver, vm, dev->data.controller, qemuCmdFlags); + if (ret == 0) + dev->data.controller = NULL; } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotplugged."), --
ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 26, 2010 at 02:09:16PM +0100, Wolfgang Mauerer wrote:
We must not delete the data part when the operation succeeds because it is required later on. The correct pattern to handle the parsed representation of the device information on success is dev->data.controller = NULL; virDomainDeviceDefFree(dev);, which leaves the structure pointed at by data in memory.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1586e35..5394ff5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6111,6 +6111,8 @@ static int qemudDomainAttachDevice(virDomainPtr dom, if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { ret = qemudDomainAttachPciControllerDevice(driver, vm, dev->data.controller, qemuCmdFlags); + if (ret == 0) + dev->data.controller = NULL; } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotplugged."),
Okay, ACK, obviously a bug fix, so pushed, 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/
When a controller is not present in the system anymore, the PCI address must be deleted from libvirt's hashtable because it can be re-used for other purposes. Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5394ff5..8960ef8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, VIR_FREE(vm->def->controllers); vm->def->ncontrollers = 0; } + + if (qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) { + VIR_WARN0("Unable to release PCI address on controller"); + } + virDomainControllerDefFree(detach); ret = 0; -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:17PM +0100, Wolfgang Mauerer wrote:
When a controller is not present in the system anymore, the PCI address must be deleted from libvirt's hashtable because it can be re-used for other purposes.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5394ff5..8960ef8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, VIR_FREE(vm->def->controllers); vm->def->ncontrollers = 0; } + + if (qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) { + VIR_WARN0("Unable to release PCI address on controller"); + } + virDomainControllerDefFree(detach);
ret = 0;
ACK, I have a feeling we leak the address in other hotunplug code too Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 26, 2010 at 02:09:17PM +0100, Wolfgang Mauerer wrote:
When a controller is not present in the system anymore, the PCI address must be deleted from libvirt's hashtable because it can be re-used for other purposes.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5394ff5..8960ef8 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, VIR_FREE(vm->def->controllers); vm->def->ncontrollers = 0; } + + if (qemuDomainPCIAddressReleaseAddr(priv->pciaddrs, &detach->info) < 0) { + VIR_WARN0("Unable to release PCI address on controller"); + } + virDomainControllerDefFree(detach);
ret = 0;
Okay, ACK, also a bug fix, so pushed, 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/
...when the underlying qemu supports the drive/device model and the controller has been added this way. Signed-off-by: Wolfgang Mauerer <woilfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ src/qemu/qemu_monitor_text.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 +++ 7 files changed, 111 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8960ef8..d683b1c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6230,7 +6230,8 @@ cleanup: static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDeviceDefPtr dev, + unsigned long long qemuCmdFlags) { int i, ret = -1; virDomainControllerDefPtr detach = NULL; @@ -6259,11 +6260,23 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; } + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuAssignDeviceControllerAlias(detach) < 0) + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } + } else { + if (qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } } qemuDomainObjExitMonitorWithDriver(driver, vm); @@ -6513,6 +6526,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; + unsigned long long qemuCmdFlags; virDomainDeviceDefPtr dev = NULL; int ret = -1; @@ -6540,6 +6554,10 @@ static int qemudDomainDetachDevice(virDomainPtr dom, if (dev == NULL) goto endjob; + if (qemudExtractVersionInfo(vm->def->emulator, + NULL, + &qemuCmdFlags) < 0) + goto endjob; if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && @@ -6549,7 +6567,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemudDomainDetachPciControllerDevice(driver, vm, dev); + ret = qemudDomainDetachPciControllerDevice(driver, vm, dev, + qemuCmdFlags); } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotunplugged."), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b88532c..a4d2b89 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1318,6 +1318,19 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; } +int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + DEBUG("mon=%p, fd=%d device(del)=%s", mon, mon->fd, devicestr); + int ret; + + if (mon->json) + ret = qemuMonitorJSONDelDevice(mon, devicestr); + else + ret = qemuMonitorTextDelDevice(mon, devicestr); + return ret; +} + int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0ac3957..3e55236 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -290,6 +290,9 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr); +int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7b45594..3a94dd0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1835,6 +1835,30 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, } +int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("device_del", + "s:config", devicestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c6a6d51..70a8dae 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -161,6 +161,9 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr); +int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr); diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 62ffcc6..610bb1f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2053,6 +2053,46 @@ error: #undef SKIP_TO +int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + + if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "device_del %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot detach %s device"), devicestr); + goto cleanup; + } + + if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("detaching %s device failed: %s"), devicestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} + + int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 1937e99..9dcb0c2 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -163,6 +163,9 @@ int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr); +int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr); -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:18PM +0100, Wolfgang Mauerer wrote:
...when the underlying qemu supports the drive/device model and the controller has been added this way.
Signed-off-by: Wolfgang Mauerer <woilfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ src/qemu/qemu_monitor_text.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 +++ 7 files changed, 111 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8960ef8..d683b1c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6230,7 +6230,8 @@ cleanup:
static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDeviceDefPtr dev, + unsigned long long qemuCmdFlags) { int i, ret = -1; virDomainControllerDefPtr detach = NULL; @@ -6259,11 +6260,23 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuAssignDeviceControllerAlias(detach) < 0) + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } + } else { + if (qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } } qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -6513,6 +6526,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; + unsigned long long qemuCmdFlags; virDomainDeviceDefPtr dev = NULL; int ret = -1;
@@ -6540,6 +6554,10 @@ static int qemudDomainDetachDevice(virDomainPtr dom, if (dev == NULL) goto endjob;
+ if (qemudExtractVersionInfo(vm->def->emulator, + NULL, + &qemuCmdFlags) < 0) + goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && @@ -6549,7 +6567,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemudDomainDetachPciControllerDevice(driver, vm, dev); + ret = qemudDomainDetachPciControllerDevice(driver, vm, dev, + qemuCmdFlags); } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotunplugged."), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b88532c..a4d2b89 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1318,6 +1318,19 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; }
+int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + DEBUG("mon=%p, fd=%d device(del)=%s", mon, mon->fd, devicestr); + int ret; + + if (mon->json) + ret = qemuMonitorJSONDelDevice(mon, devicestr); + else + ret = qemuMonitorTextDelDevice(mon, devicestr); + return ret; +} +
int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0ac3957..3e55236 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -290,6 +290,9 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7b45594..3a94dd0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1835,6 +1835,30 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, }
+int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("device_del", + "s:config", devicestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c6a6d51..70a8dae 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -161,6 +161,9 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr);
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 62ffcc6..610bb1f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2053,6 +2053,46 @@ error: #undef SKIP_TO
+int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + + if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "device_del %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot detach %s device"), devicestr); + goto cleanup; + } + + if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("detaching %s device failed: %s"), devicestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} + + int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 1937e99..9dcb0c2 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -163,6 +163,9 @@ int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr);
--
ACK, I'd been rather lazy and not converted any of the hot-unplug code over to use device_del yet, since it wasn't a show-stopper at the time Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Fri, Feb 26, 2010 at 02:09:18PM +0100, Wolfgang Mauerer wrote:
...when the underlying qemu supports the drive/device model and the controller has been added this way.
Signed-off-by: Wolfgang Mauerer <woilfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 31 +++++++++++++++++++++++++------ src/qemu/qemu_monitor.c | 13 +++++++++++++ src/qemu/qemu_monitor.h | 3 +++ src/qemu/qemu_monitor_json.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 3 +++ src/qemu/qemu_monitor_text.c | 40 ++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_text.h | 3 +++ 7 files changed, 111 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8960ef8..d683b1c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6230,7 +6230,8 @@ cleanup:
static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, virDomainObjPtr vm, - virDomainDeviceDefPtr dev) + virDomainDeviceDefPtr dev, + unsigned long long qemuCmdFlags) { int i, ret = -1; virDomainControllerDefPtr detach = NULL; @@ -6259,11 +6260,23 @@ static int qemudDomainDetachPciControllerDevice(struct qemud_driver *driver, goto cleanup; }
+ if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuAssignDeviceControllerAlias(detach) < 0) + goto cleanup; + } + qemuDomainObjEnterMonitorWithDriver(driver, vm); - if (qemuMonitorRemovePCIDevice(priv->mon, - &detach->info.addr.pci) < 0) { - qemuDomainObjExitMonitor(vm); - goto cleanup; + if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) { + if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } + } else { + if (qemuMonitorRemovePCIDevice(priv->mon, + &detach->info.addr.pci) < 0) { + qemuDomainObjExitMonitor(vm); + goto cleanup; + } } qemuDomainObjExitMonitorWithDriver(driver, vm);
@@ -6513,6 +6526,7 @@ static int qemudDomainDetachDevice(virDomainPtr dom, const char *xml) { struct qemud_driver *driver = dom->conn->privateData; virDomainObjPtr vm; + unsigned long long qemuCmdFlags; virDomainDeviceDefPtr dev = NULL; int ret = -1;
@@ -6540,6 +6554,10 @@ static int qemudDomainDetachDevice(virDomainPtr dom, if (dev == NULL) goto endjob;
+ if (qemudExtractVersionInfo(vm->def->emulator, + NULL, + &qemuCmdFlags) < 0) + goto endjob;
if (dev->type == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && @@ -6549,7 +6567,8 @@ static int qemudDomainDetachDevice(virDomainPtr dom, ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { if (dev->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { - ret = qemudDomainDetachPciControllerDevice(driver, vm, dev); + ret = qemudDomainDetachPciControllerDevice(driver, vm, dev, + qemuCmdFlags); } else { qemuReportError(VIR_ERR_NO_SUPPORT, _("disk controller bus '%s' cannot be hotunplugged."), diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b88532c..a4d2b89 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1318,6 +1318,19 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, return ret; }
+int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + DEBUG("mon=%p, fd=%d device(del)=%s", mon, mon->fd, devicestr); + int ret; + + if (mon->json) + ret = qemuMonitorJSONDelDevice(mon, devicestr); + else + ret = qemuMonitorTextDelDevice(mon, devicestr); + return ret; +} +
int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr) diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 0ac3957..3e55236 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -290,6 +290,9 @@ int qemuMonitorGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorAddDrive(qemuMonitorPtr mon, const char *drivestr);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 7b45594..3a94dd0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -1835,6 +1835,30 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon ATTRIBUTE_UNUSED, }
+int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + int ret; + virJSONValuePtr cmd; + virJSONValuePtr reply = NULL; + + cmd = qemuMonitorJSONMakeCommand("device_del", + "s:config", devicestr, + NULL); + if (!cmd) + return -1; + + ret = qemuMonitorJSONCommand(mon, cmd, &reply); + + if (ret == 0) + ret = qemuMonitorJSONCheckError(cmd, reply); + + virJSONValueFree(cmd); + virJSONValueFree(reply); + return ret; +} + + int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c6a6d51..70a8dae 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -161,6 +161,9 @@ int qemuMonitorJSONGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorJSONAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorJSONDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorJSONAddDrive(qemuMonitorPtr mon, const char *drivestr);
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 62ffcc6..610bb1f 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -2053,6 +2053,46 @@ error: #undef SKIP_TO
+int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr) +{ + char *cmd = NULL; + char *reply = NULL; + char *safedev; + int ret = -1; + + if (!(safedev = qemuMonitorEscapeArg(devicestr))) { + virReportOOMError(); + goto cleanup; + } + + if (virAsprintf(&cmd, "device_del %s", safedev) < 0) { + virReportOOMError(); + goto cleanup; + } + + if (qemuMonitorCommand(mon, cmd, &reply) < 0) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("cannot detach %s device"), devicestr); + goto cleanup; + } + + if (STRNEQ(reply, "")) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("detaching %s device failed: %s"), devicestr, reply); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(cmd); + VIR_FREE(reply); + VIR_FREE(safedev); + return ret; +} + + int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr) { diff --git a/src/qemu/qemu_monitor_text.h b/src/qemu/qemu_monitor_text.h index 1937e99..9dcb0c2 100644 --- a/src/qemu/qemu_monitor_text.h +++ b/src/qemu/qemu_monitor_text.h @@ -163,6 +163,9 @@ int qemuMonitorTextGetAllPCIAddresses(qemuMonitorPtr mon, int qemuMonitorTextAddDevice(qemuMonitorPtr mon, const char *devicestr);
+int qemuMonitorTextDelDevice(qemuMonitorPtr mon, + const char *devicestr); + int qemuMonitorTextAddDrive(qemuMonitorPtr mon, const char *drivestr);
Looks fine. On one hand it's an improvement so it might have been better to delay pushing it, but since we do device add, it's more sound to have a symetric approach, so I made an exception an pushed that one too, 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/
Recent qemu versions allow us to add disks dynamically into the system via the drive_add/device_add mechanism. Removing them is now just a matter of using device_del, and this patch adds the required bits and pieces to libvirt. Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d683b1c..ecbb23d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5494,6 +5494,60 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, } +static int qemudDomainDetachSCSIDisk(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned int qemuCmdFlags) +{ + int i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDiskDefPtr detach = NULL; + + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + return -1; + } + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { + detach = vm->def->disks[i]; + break; + } + } + + if (!detach) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + return -1; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* Note: drive_del does not exist, but device_del will + automatically erase the associated drive as well */ + if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + return -1; + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (vm->def->ndisks > 1) { + memmove(vm->def->disks + i, + vm->def->disks + i + 1, + sizeof(*vm->def->disks) * + (vm->def->ndisks - (i + 1))); + vm->def->ndisks--; + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(vm->def->disks); + vm->def->ndisks = 0; + } + virDomainDiskDefFree(detach); + + return 0; +} + + static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -6563,6 +6617,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { ret = qemudDomainDetachPciDiskDevice(driver, vm, dev); + } else if(dev->type == VIR_DOMAIN_DEVICE_DISK && + dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + ret = qemudDomainDetachSCSIDisk(driver, vm, dev, qemuCmdFlags); } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) { -- 1.6.4
On Fri, Feb 26, 2010 at 02:09:19PM +0100, Wolfgang Mauerer wrote:
Recent qemu versions allow us to add disks dynamically into the system via the drive_add/device_add mechanism. Removing them is now just a matter of using device_del, and this patch adds the required bits and pieces to libvirt.
There's one minor issue here, in that this removes the guest device, but does not remove the host side QEMU block driver state. We're still waiting for the 'drive_del' command to be implemented todo the latter bit.
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d683b1c..ecbb23d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5494,6 +5494,60 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, }
+static int qemudDomainDetachSCSIDisk(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned int qemuCmdFlags) +{ + int i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDiskDefPtr detach = NULL; + + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + return -1; + } + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { + detach = vm->def->disks[i]; + break; + } + } + + if (!detach) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + return -1; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* Note: drive_del does not exist, but device_del will + automatically erase the associated drive as well */
Are you sure about that ? I was under the impression that device_del would leave the drive dangling unused.
+ if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + return -1; + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (vm->def->ndisks > 1) { + memmove(vm->def->disks + i, + vm->def->disks + i + 1, + sizeof(*vm->def->disks) * + (vm->def->ndisks - (i + 1))); + vm->def->ndisks--; + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(vm->def->disks); + vm->def->ndisks = 0; + } + virDomainDiskDefFree(detach); + + return 0; +} + + static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -6563,6 +6617,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { ret = qemudDomainDetachPciDiskDevice(driver, vm, dev); + } else if(dev->type == VIR_DOMAIN_DEVICE_DISK && + dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + ret = qemudDomainDetachSCSIDisk(driver, vm, dev, qemuCmdFlags); } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
On Fri, Feb 26, 2010 at 02:09:19PM +0100, Wolfgang Mauerer wrote:
Recent qemu versions allow us to add disks dynamically into the system via the drive_add/device_add mechanism. Removing them is now just a matter of using device_del, and this patch adds the required bits and pieces to libvirt.
There's one minor issue here, in that this removes the guest device, but does not remove the host side QEMU block driver state. We're still waiting for the 'drive_del' command to be implemented todo the latter bit.
I haven't checked with the qemu source code (Gerd is CC'ed), but the drive associated with a device seems to disappear when the device is removed - the drive ID can be reused with a different backing file after removing the device, and I suppose that's sufficient for libvirt (or did I miss a use case?): (Add controller, drive and device) (qemu) device_add lsi,id=controller (qemu) drive_add 0 if=none,file=test.img,id=drive0 OK (qemu) info block ide1-cd0: type=cdrom removable=1 locked=0 [not inserted] floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] drive0: type=hd removable=0 file=test.img ro=0 drv=raw encrypted=0 (qemu) device_add driver=scsi-disk,drive=drive0,bus=controller.0,id=device0 ########### (Remove the device -> the associated drive disappears) (qemu) device_del device0 (qemu) info block ide1-cd0: type=cdrom removable=1 locked=0 [not inserted] floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] ########### (Add a new drive with the same ID as before, but different backing file) (qemu) drive_add 0 if=none,file=test2.img,id=drive0 OK (qemu) device_add driver=scsi-disk,drive=drive0,bus=controller.0,id=device0 (qemu) info block ide1-cd0: type=cdrom removable=1 locked=0 [not inserted] floppy0: type=floppy removable=1 locked=0 [not inserted] sd0: type=floppy removable=1 locked=0 [not inserted] drive0: type=hd removable=0 file=test2.img ro=0 drv=raw encrypted=0 Gerd: Are you intending to add the drive_del feature, or is the approach outlined above sufficient for drive hotplug/remove? Respectively can there be any problems if we remove a device associated with a disk and then re-create a drive/device pair with the same IDs as before, but with a different configuration? Best regards, Wolfgang
Signed-off-by: Wolfgang Mauerer <wolfgang.mauerer@siemens.com> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> --- src/qemu/qemu_driver.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 57 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d683b1c..ecbb23d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -5494,6 +5494,60 @@ qemuDomainFindOrCreateSCSIDiskController(struct qemud_driver *driver, }
+static int qemudDomainDetachSCSIDisk(struct qemud_driver *driver, + virDomainObjPtr vm, + virDomainDeviceDefPtr dev, + unsigned int qemuCmdFlags) +{ + int i; + qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDiskDefPtr detach = NULL; + + if (!(qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE)) { + return -1; + } + + for (i = 0 ; i < vm->def->ndisks ; i++) { + if (STREQ(vm->def->disks[i]->dst, dev->data.disk->dst)) { + detach = vm->def->disks[i]; + break; + } + } + + if (!detach) { + qemuReportError(VIR_ERR_OPERATION_FAILED, + _("disk %s not found"), dev->data.disk->dst); + return -1; + } + + qemuDomainObjEnterMonitorWithDriver(driver, vm); + /* Note: drive_del does not exist, but device_del will + automatically erase the associated drive as well */
Are you sure about that ? I was under the impression that device_del would leave the drive dangling unused.
+ if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) { + qemuDomainObjExitMonitor(vm); + return -1; + } + qemuDomainObjExitMonitorWithDriver(driver, vm); + + if (vm->def->ndisks > 1) { + memmove(vm->def->disks + i, + vm->def->disks + i + 1, + sizeof(*vm->def->disks) * + (vm->def->ndisks - (i + 1))); + vm->def->ndisks--; + if (VIR_REALLOC_N(vm->def->disks, vm->def->ndisks) < 0) { + /* ignore, harmless */ + } + } else { + VIR_FREE(vm->def->disks); + vm->def->ndisks = 0; + } + virDomainDiskDefFree(detach); + + return 0; +} + + static int qemudDomainAttachSCSIDisk(struct qemud_driver *driver, virDomainObjPtr vm, virDomainDiskDefPtr disk, @@ -6563,6 +6617,9 @@ static int qemudDomainDetachDevice(virDomainPtr dom, dev->data.disk->device == VIR_DOMAIN_DISK_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { ret = qemudDomainDetachPciDiskDevice(driver, vm, dev); + } else if(dev->type == VIR_DOMAIN_DEVICE_DISK && + dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_SCSI) { + ret = qemudDomainDetachSCSIDisk(driver, vm, dev, qemuCmdFlags); } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { ret = qemudDomainDetachNetDevice(driver, vm, dev); } else if (dev->type == VIR_DOMAIN_DEVICE_CONTROLLER) {
Regards, Daniel
On 03/01/10 15:29, Wolfgang Mauerer wrote:
Gerd: Are you intending to add the drive_del feature, or is the approach outlined above sufficient for drive hotplug/remove?
I'm busy with other tasks right now.
Respectively can there be any problems if we remove a device associated with a disk and then re-create a drive/device pair with the same IDs as before, but with a different configuration?
There are no problems with that, when it is gone from 'info block' all traces of the drive are gone and creating another one with the same name is not a problem. For symmetry reasons it would be nice to have a drive_del command though and have device_del not implicitly zap the drive. The current scheme also fails to handle some corner cases like device_add failing (you are left with a drive you can't remove easily). cheers, Gerd
On Mon, Mar 01, 2010 at 03:40:14PM +0100, Gerd Hoffmann wrote:
On 03/01/10 15:29, Wolfgang Mauerer wrote:
Gerd: Are you intending to add the drive_del feature, or is the approach outlined above sufficient for drive hotplug/remove?
I'm busy with other tasks right now.
Respectively can there be any problems if we remove a device associated with a disk and then re-create a drive/device pair with the same IDs as before, but with a different configuration?
There are no problems with that, when it is gone from 'info block' all traces of the drive are gone and creating another one with the same name is not a problem.
Great, this is sufficient for the hot-unplug code to work.
For symmetry reasons it would be nice to have a drive_del command though and have device_del not implicitly zap the drive. The current scheme also fails to handle some corner cases like device_add failing (you are left with a drive you can't remove easily).
Yep, that's a minor edge case we can worry about later Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Gerd Hoffmann -
Wolfgang Mauerer