Re: [PATCH 4/7] qemu: Support update disk's bootindex

On Thu, Nov 17, 2022 at 10:05:30 +0800, Jiang Jiacheng wrote: > Support to update the disk's bootindex using 'virsh update-device'. > Using flag --config or --persistent to change the boot index and the > change will be affected after reboot. With --persistent, we can get > the result of change immediently, but it still takes effect after reboot. > Currently, support update bootindex of disk type as cdrom or disk. > > Signed-off-by: Jiang Jiacheng <jiangjiacheng(a)huawei.com&gt; > --- > src/qemu/qemu_domain.c | 3 ++- > src/qemu/qemu_driver.c | 28 ++++++++++++++++++++++++++++ > 2 files changed, 30 insertions(+), 1 deletion(-) [...] > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index ff5a743716..65abc04998 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -6767,6 +6767,7 @@ qemuDomainChangeDiskLive(virDomainObj *vm, > virDomainDiskDef *orig_disk = NULL; > virDomainStartupPolicy origStartupPolicy; > virDomainDeviceDef oldDev = { .type = dev->type }; > + int needChangeIndex = 0; > > if (!(orig_disk = virDomainDiskByTarget(vm->def, disk->dst))) { > virReportError(VIR_ERR_INTERNAL_ERROR, > @@ -6784,6 +6785,10 @@ qemuDomainChangeDiskLive(virDomainObj *vm, > if (!qemuDomainDiskChangeSupported(disk, orig_disk)) > return -1; > > + if ((needChangeIndex = qemuCheckBootIndex(&orig_disk->info, > + disk->info.bootIndex)) < 0) > + return -1; Due to the weird check I complained about in previous patch, this actively breaks media change (and other parameter change) for disks which don't have bootindex.

> + > if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { > /* Disk source can be changed only for removable devices */ > if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && > @@ -6807,6 +6812,11 @@ qemuDomainChangeDiskLive(virDomainObj *vm, > dev->data.disk->src = NULL; > } > > + if (needChangeIndex) { > + if (qemuChangeDiskBootIndex(vm, orig_disk, disk) < 0) > + return -1; > + } > + > /* in case when we aren't updating disk source we update startup policy here */ > orig_disk->startupPolicy = dev->data.disk->startupPolicy; > orig_disk->snapshot = dev->data.disk->snapshot; > @@ -7503,6 +7513,24 @@ qemuDomainUpdateDeviceConfig(virDomainDef *vmdef, > false) < 0) > return -1; > > + switch (vmdef->disks[pos]->device) { > + case VIR_DOMAIN_DISK_DEVICE_CDROM: > + case VIR_DOMAIN_DISK_DEVICE_DISK: > + case VIR_DOMAIN_DISK_DEVICE_LUN: > + if (qemuCheckBootIndex(&vmdef->disks[pos]->info, > + newDisk->info.bootIndex) < 0) > + return -1; > + break; > + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: > + case VIR_DOMAIN_DISK_DEVICE_LAST: > + if ((newDisk->info.bootIndex) && > + (vmdef->disks[pos]->info.bootIndex != newDisk->info.bootIndex)) { > + virReportError(VIR_ERR_INVALID_ARG, "%s", > + _("this disk doesn't support update")); > + return -1; > + } > + } This also doesn't make sense. You don't have the same issue updating the bootindex of a floppy. Similarly, the qemuCheckBootIndex check is also not applicable as the next boot config willr esult in a new boot where you can change anything.