[libvirt] [PATCH 5/8] qemu: support attachment of disk device with boot index
John Ferlan
jferlan at redhat.com
Tue Feb 3 01:41:13 UTC 2015
On 01/05/2015 02:29 AM, Wang Rui wrote:
> When we attach a disk to a running VM with boot index, we can get a
> successful result. But in fact the boot index won't take effect. QEMU
> supported to set device's boot index online recently(since QEMU 2.2.0).
>
> After this patch, the boot index will take effect after
> virDomainAttachDevice(Flags) API returning success. If new disk is
> attached successfully but boot index is set failed, we'll remove the
> new disk to restore.
>
> Signed-off-by: Wang Rui <moon.wangrui at huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> ---
> src/qemu/qemu_hotplug.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 88 insertions(+)
>
If I read the tea leaves correctly - running this on < qemu 2.2 will
cause a failure since the ChangeBootIndex isn't supported there.
I don't get a sense why just failing the boot index setting should cause
the disk to not be attached. How do you differentiate the failure is
not supported vs. not present?
Seems to me there should be a way to qom-get and do some "pre-checks"
before adding, then trying to rip it out which is just ripe for all
sorts of errors. Taking that route perhaps means the code movement patch
is unnecessary and the device removal is unnecessary.
I didn't dive deep into the following code as I really think the qom-get
is something that'll obsolete certain parts of it.
John
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 2f84949..5eacfce 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -2999,7 +2999,12 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> {
> virDomainDiskDefPtr disk = dev->data.disk;
> virDomainDiskDefPtr orig_disk = NULL;
> + virDomainDeviceDefPtr new_dev_copy = NULL;
> + virDomainDeviceDefPtr old_dev_copy = NULL;
> + virDomainDiskDefPtr tmp = NULL;
> + virCapsPtr caps = NULL;
> int ret = -1;
> + int removed = 0;
> const char *driverName = virDomainDiskGetDriver(disk);
> const char *src = virDomainDiskGetSource(disk);
>
> @@ -3022,6 +3027,13 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
> goto end;
>
> + if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
> + goto end;
> +
> + if (!(new_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
> + caps, driver->xmlopt)))
> + goto end;
> +
> switch (disk->device) {
> case VIR_DOMAIN_DISK_DEVICE_CDROM:
> case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> @@ -3036,12 +3048,33 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> goto end;
> }
>
> + tmp = dev->data.disk;
> + dev->data.disk = orig_disk;
> + /* save a copy of old device to restore */
> + if (!(old_dev_copy = virDomainDeviceDefCopy(dev, vm->def,
> + caps, driver->xmlopt))) {
> + dev->data.disk = tmp;
> + goto end;
> + }
> + dev->data.disk = tmp;
> +
> if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
> disk->src, false) < 0)
> goto end;
>
> disk->src = NULL;
> ret = 0;
> +
> + tmp = new_dev_copy->data.disk;
> + if (orig_disk->info.bootIndex != tmp->info.bootIndex) {
> + /* If boot index is to be changed to 0, we can pass "value":-1 to
> + qmp command("qom-set") to cancel boot index. */
> + if (qemuDomainChangeBootIndex(driver, vm, &orig_disk->info,
> + tmp->info.bootIndex ?
> + tmp->info.bootIndex : -1) < 0)
> + goto try_remove;
> + orig_disk->info.bootIndex = tmp->info.bootIndex;
> + }
> break;
>
> case VIR_DOMAIN_DISK_DEVICE_DISK:
> @@ -3063,6 +3096,15 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> _("disk bus '%s' cannot be hotplugged."),
> virDomainDiskBusTypeToString(disk->bus));
> }
> +
> + tmp = new_dev_copy->data.disk;
> + if (!ret && tmp->info.bootIndex > 0) {
> + if (qemuDomainChangeBootIndex(driver, vm, &disk->info,
> + tmp->info.bootIndex) < 0)
> + goto try_remove;
> + disk->info.bootIndex = tmp->info.bootIndex;
> + }
> +
> break;
> default:
> virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
> @@ -3074,7 +3116,53 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
> end:
> if (ret != 0)
> ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
> + virObjectUnref(caps);
> + virDomainDeviceDefFree(new_dev_copy);
> + virDomainDeviceDefFree(old_dev_copy);
> + if (removed) {
> + dev->data.disk = NULL;
> + ret = -1;
> + }
> return ret;
> +
> + try_remove:
> + if (!virDomainObjIsActive(vm)) {
> + removed = 1;
> + goto end;
> + }
> +
> + switch (new_dev_copy->data.disk->device) {
> + case VIR_DOMAIN_DISK_DEVICE_CDROM:
> + case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
> + if (qemuAddSharedDevice(driver, old_dev_copy, vm->def->name) < 0)
> + break;
> +
> + tmp = old_dev_copy->data.disk;
> + if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, tmp->src, false) < 0) {
> + VIR_WARN("Unable to recover original media with target '%s' and path '%s'",
> + tmp->dst, tmp->src->path);
> + ignore_value(qemuRemoveSharedDevice(driver, old_dev_copy, vm->def->name));
> + } else {
> + tmp->src = NULL;
> + }
> + break;
> + case VIR_DOMAIN_DISK_DEVICE_DISK:
> + case VIR_DOMAIN_DISK_DEVICE_LUN:
> + if (disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) {
> + if (qemuDomainDetachVirtioDiskDevice(driver, vm, disk) != 0)
> + VIR_WARN("Unable to detach new disk with bus '%s' and target '%s'",
> + virDomainDiskBusTypeToString(disk->bus), disk->dst);
> + }
> + else if (disk->bus == VIR_DOMAIN_DISK_BUS_SCSI ||
> + disk->bus == VIR_DOMAIN_DISK_BUS_USB)
> + if (qemuDomainDetachDiskDevice(driver, vm, disk) != 0)
> + VIR_WARN("Unable to detach new disk with bus '%s' and target '%s'",
> + virDomainDiskBusTypeToString(disk->bus), disk->dst);
> + ret = -1;
> + break;
> + }
> + removed = 1;
> + goto end;
> }
>
> static int
>
More information about the libvir-list
mailing list