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@xxxxxxxxxxx>
>> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
>> ---
>> 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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list