8:29 a.m.
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(a)siemens.com>
> Signed-off-by: Jan Kiszka <jan.kiszka(a)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