[PATCH 0/7] qemu: support updating device's bootindex
Support updating device's(support cdrom, disk and network) bootindex online in virDomainUpdateDeviceFlags. The new bootindex will take effect after guest rebooting. Enable bootindex can be set to -1, it means cancel the device's bootindex. To use this feature, we need to get the device's xml first and modify the boot order in the xml, then use 'virsh update-device <domain> <xml> --flag' to update the bootindex. Note that the flag should be --config or --persistent if the vm is running. Jiang Jiacheng (7): qemu: Introduce qemuDomainChangeBootIndex API qemu: Introduce qemuCheckBootIndex and qemuChangeDiskBootIndex API qemu: Support update disk's bootindex qemu: Support update net's bootindex qemu: Support set bootindex to -1 qemu: Support add bootindex = -1 to boothash qemu: Reserve bootindex = -1 in virDomainDeviceDefCopy src/conf/device_conf.h | 4 +-- src/conf/domain_conf.c | 38 +++++++++++++++----- src/conf/domain_conf.h | 4 +++ src/conf/domain_postparse.c | 8 ++++- src/qemu/qemu_command.c | 38 ++++++++++---------- src/qemu/qemu_conf.c | 69 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 12 +++++++ src/qemu/qemu_domain.c | 3 +- src/qemu/qemu_driver.c | 32 +++++++++++++++++ src/qemu/qemu_hotplug.c | 17 ++++++--- src/qemu/qemu_monitor.c | 12 +++++++ src/qemu/qemu_monitor.h | 6 ++++ src/qemu/qemu_monitor_json.c | 22 ++++++++++++ src/qemu/qemu_monitor_json.h | 6 ++++ src/qemu/qemu_process.c | 8 ++--- src/qemu/qemu_validate.c | 6 ++-- 16 files changed, 242 insertions(+), 43 deletions(-) -- 2.33.0
Introduce qemuDomainChangeBootIndex api to support update device's bootindex. These function will be used in following patches to support change device's (support cdrom, disk and net) bootindex with virsh command like 'virsh update-device <domain> <file> [--flag]'. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/qemu/qemu_conf.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_conf.h | 5 +++++ src/qemu/qemu_monitor.c | 12 ++++++++++++ src/qemu/qemu_monitor.h | 6 ++++++ src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 ++++++ 6 files changed, 75 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3b75cdeb95..b653f44f25 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1571,3 +1571,27 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver, *memPath = g_strdup_printf("%s/%s", domainPath, alias); return 0; } + +int +qemuDomainChangeBootIndex(virDomainObj *vm, + virDomainDeviceInfo *devInfo, + int newBootIndex) +{ + int ret = -1; + qemuDomainObjPrivate *priv = vm->privateData; + + if (!devInfo->alias) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("cannot change boot index: device alias not found")); + return -1; + } + + VIR_DEBUG("Change dev: %s boot index from %d to %d", devInfo->alias, + devInfo->bootIndex, newBootIndex); + + qemuDomainObjEnterMonitor(vm); + ret = qemuMonitorSetBootIndex(priv->mon, devInfo->alias, newBootIndex); + qemuDomainObjExitMonitor(vm); + + return ret; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c40c452f58..4e2cd84fe5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -359,3 +359,8 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver, const virDomainDef *def, const char *alias, char **memPath); + +int qemuDomainChangeBootIndex(virDomainObj *vm, + virDomainDeviceInfo *devInfo, + int newBootIndex) + ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c2808c75a3..1b120d766d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4417,3 +4417,15 @@ qemuMonitorExtractQueryStats(virJSONValue *info) return g_steal_pointer(&hash_table); } + +int +qemuMonitorSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) +{ + VIR_DEBUG("name=%s, bootIndex=%d", name, bootIndex); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSetBootIndex(mon, name, bootIndex); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 63269e15bc..f27431a3f2 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1529,3 +1529,9 @@ qemuMonitorQueryStats(qemuMonitor *mon, GHashTable * qemuMonitorExtractQueryStats(virJSONValue *info); + +int +qemuMonitorSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 70fba50e6c..5cc1f2665a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8665,3 +8665,25 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, return virJSONValueObjectStealArray(reply, "return"); } + +int +qemuMonitorJSONSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-set", "s:path", name, + "s:property", "bootindex", + "i:value", bootIndex, NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index a53e6423df..8283c9b29a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -818,3 +818,9 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, qemuMonitorQueryStatsTargetType target, char **vcpus, GPtrArray *providers); + +int +qemuMonitorJSONSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -- 2.33.0
On Tue, Oct 11, 2022 at 21:38:27 +0800, Jiang Jiacheng wrote:
Introduce qemuDomainChangeBootIndex api to support update device's bootindex. These function will be used in following patches to support change device's (support cdrom, disk and net) bootindex with virsh command like 'virsh update-device <domain> <file> [--flag]'.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/qemu/qemu_conf.c | 24 ++++++++++++++++++++++++ src/qemu/qemu_conf.h | 5 +++++ src/qemu/qemu_monitor.c | 12 ++++++++++++ src/qemu/qemu_monitor.h | 6 ++++++ src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 6 ++++++ 6 files changed, 75 insertions(+)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 3b75cdeb95..b653f44f25 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1571,3 +1571,27 @@ qemuGetMemoryBackingPath(virQEMUDriver *driver, *memPath = g_strdup_printf("%s/%s", domainPath, alias); return 0; } +
Please document the expectations of the API ... e.g. the values that the 'newBootIndex' variable takes.
+int +qemuDomainChangeBootIndex(virDomainObj *vm, + virDomainDeviceInfo *devInfo, + int newBootIndex) +{ + int ret = -1; + qemuDomainObjPrivate *priv = vm->privateData; + + if (!devInfo->alias) { + virReportError(VIR_ERR_OPERATION_FAILED, "%s", + _("cannot change boot index: device alias not found")); + return -1; + } + + VIR_DEBUG("Change dev: %s boot index from %d to %d", devInfo->alias, + devInfo->bootIndex, newBootIndex); + + qemuDomainObjEnterMonitor(vm); + ret = qemuMonitorSetBootIndex(priv->mon, devInfo->alias, newBootIndex);
So here you pass alias as qom-path for qom-set. I'm not sure what our policy on using the shortcut path is though as all code using qom-set/qom-get always uses the full absolute path.
+ qemuDomainObjExitMonitor(vm); + + return ret; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index c40c452f58..4e2cd84fe5 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -359,3 +359,8 @@ int qemuGetMemoryBackingPath(virQEMUDriver *driver, const virDomainDef *def, const char *alias, char **memPath); + +int qemuDomainChangeBootIndex(virDomainObj *vm, + virDomainDeviceInfo *devInfo, + int newBootIndex) + ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c2808c75a3..1b120d766d 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4417,3 +4417,15 @@ qemuMonitorExtractQueryStats(virJSONValue *info)
return g_steal_pointer(&hash_table); } + +int +qemuMonitorSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) +{ + VIR_DEBUG("name=%s, bootIndex=%d", name, bootIndex); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSetBootIndex(mon, name, bootIndex); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 63269e15bc..f27431a3f2 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1529,3 +1529,9 @@ qemuMonitorQueryStats(qemuMonitor *mon,
GHashTable * qemuMonitorExtractQueryStats(virJSONValue *info); + +int +qemuMonitorSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 70fba50e6c..5cc1f2665a 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8665,3 +8665,25 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon,
return virJSONValueObjectStealArray(reply, "return"); } + +int +qemuMonitorJSONSetBootIndex(qemuMonitor *mon, + const char *name, + int bootIndex) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("qom-set", "s:path", name, + "s:property", "bootindex",
Note that you use 'name' as parameter, but in fact it's a qom path. Please rename it appropriately to avoid confusion.
+ "i:value", bootIndex, NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + return 0; +}
Introduce qemuCheckBootIndex to check the new bootindex and is it nessary to update the bootindex. Introduce qemuChangeDiskBootIndex to support update disk's bootindex according to different disks' type. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/qemu/qemu_conf.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 7 +++++++ 2 files changed, 52 insertions(+) diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index b653f44f25..3255632254 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1595,3 +1595,48 @@ qemuDomainChangeBootIndex(virDomainObj *vm, return ret; } + +int +qemuCheckBootIndex(const int orig_bootindex, + const int new_bootindex) +{ + if ((new_bootindex == -1) && ((orig_bootindex == 0) || (orig_bootindex == -1))) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, + _("this device does not set boot index, cannot cancel it.")); + return -1; + } + + /* if the new bootindex is different from the old bootindex and not + * equal to 0, we will update the old bootindex. + */ + if (new_bootindex && (orig_bootindex != new_bootindex)) { + return 1; + } + + return 0; +} + +int +qemuChangeDiskBootIndex(virDomainObj *vm, + virDomainDiskDef *orig_disk, + virDomainDiskDef *disk) +{ + switch (disk->device) { + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_DISK: + case VIR_DOMAIN_DISK_DEVICE_LUN: + if (qemuDomainChangeBootIndex(vm, &orig_disk->info, + disk->info.bootIndex) < 0) + return -1; + + orig_disk->info.bootIndex = disk->info.bootIndex; + break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("The boot index of disk bus '%s' cannot be updated."), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + return 0; +} diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index 4e2cd84fe5..43197afb7d 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -364,3 +364,10 @@ int qemuDomainChangeBootIndex(virDomainObj *vm, virDomainDeviceInfo *devInfo, int newBootIndex) ATTRIBUTE_NONNULL(2); + +int qemuCheckBootIndex(const int orig_bootindex, + const int new_bootindex); + +int qemuChangeDiskBootIndex(virDomainObj *vm, + virDomainDiskDef *orig_disk, + virDomainDiskDef *disk); -- 2.33.0
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@huawei.com> --- 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_domain.c b/src/qemu/qemu_domain.c index fe3ce023a4..f7ee7b7844 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8121,7 +8121,8 @@ qemuDomainDiskChangeSupported(virDomainDiskDef *disk, /* device alias is checked already in virDomainDefCompatibleDevice */ - CHECK_EQ(info.bootIndex, "boot order", true); + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) + CHECK_EQ(info.bootIndex, "boot order", true); CHECK_EQ(rawio, "rawio", true); CHECK_EQ(sgio, "sgio", true); CHECK_EQ(discard, "discard", true); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 707f4cc1bb..38cc5d0229 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7027,6 +7027,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, @@ -7044,6 +7045,10 @@ qemuDomainChangeDiskLive(virDomainObj *vm, if (!qemuDomainDiskChangeSupported(disk, orig_disk)) return -1; + if ((needChangeIndex = qemuCheckBootIndex(orig_disk->info.bootIndex, + disk->info.bootIndex)) < 0) + return -1; + if (!virStorageSourceIsSameLocation(disk->src, orig_disk->src)) { /* Disk source can be changed only for removable devices */ if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && @@ -7067,6 +7072,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; @@ -7762,6 +7772,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(newDisk->info.bootIndex, + vmdef->disks[pos]->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; + } + } + virDomainDiskDefFree(vmdef->disks[pos]); vmdef->disks[pos] = newDisk; dev->data.disk = NULL; -- 2.33.0
Support to update the net'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. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/qemu/qemu_driver.c | 4 ++++ src/qemu/qemu_hotplug.c | 17 ++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 38cc5d0229..7d32b3bc50 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7827,6 +7827,10 @@ qemuDomainUpdateDeviceConfig(virDomainDef *vmdef, false) < 0) return -1; + if (qemuCheckBootIndex(net->info.bootIndex, + vmdef->nets[pos]->info.bootIndex) < 0) + return -1; + if (virDomainNetUpdate(vmdef, pos, net)) return -1; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 9b508dc8f0..370a272715 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3469,6 +3469,7 @@ qemuDomainChangeNet(virQEMUDriver *driver, bool needCoalesceChange = false; bool needVlanUpdate = false; bool needIsolatedPortChange = false; + int needChangeIndex = 0; int ret = -1; int changeidx = -1; g_autoptr(virConnect) conn = NULL; @@ -3634,11 +3635,8 @@ qemuDomainChangeNet(virQEMUDriver *driver, goto cleanup; } - if (newdev->info.bootIndex == 0) - newdev->info.bootIndex = olddev->info.bootIndex; - if (olddev->info.bootIndex != newdev->info.bootIndex) { - virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", - _("cannot modify network device boot index setting")); + if ((needChangeIndex = qemuCheckBootIndex(olddev->info.bootIndex, + newdev->info.bootIndex)) < 0) { goto cleanup; } @@ -3919,6 +3917,15 @@ qemuDomainChangeNet(virQEMUDriver *driver, needReplaceDevDef = true; } + if (needChangeIndex) { + if (qemuDomainChangeBootIndex(vm, &olddev->info, newdev->info.bootIndex) < 0) + goto cleanup; + /* we successfully switched to the new boot index, and we've + * determined that the rest of newdev is equivalent to olddev, + * so move newdev into place */ + needReplaceDevDef = true; + } + if (needReplaceDevDef) { /* the changes above warrant replacing olddev with newdev in * the domain's nets list. -- 2.33.0
Enable bootindex can be set to -1, it means cancel the device's bootindex. Change bootindex's type from unsigned int to int and modify other related codes concered with type. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/conf/device_conf.h | 4 ++-- src/conf/domain_conf.c | 15 ++++++++++----- src/conf/domain_postparse.c | 2 +- src/qemu/qemu_command.c | 38 ++++++++++++++++++------------------- src/qemu/qemu_process.c | 8 ++++---- src/qemu/qemu_validate.c | 6 +++--- 6 files changed, 39 insertions(+), 34 deletions(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index f2907dc596..a9df2e6e5d 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -139,11 +139,11 @@ struct _virDomainDeviceInfo { char *romfile; /* bootIndex is only used for disk, network interface, hostdev * and redirdev devices */ - unsigned int bootIndex; + int bootIndex; /* 'effectiveBootIndex' is same as 'bootIndex' (if provided in the XML) but * not formatted back. This allows HV drivers to update it if <os><boot .. * is present. */ - unsigned int effectiveBootIndex; + int effectiveBootIndex; /* Valid for any PCI device. Can be used for NIC to get * stable numbering in Linux */ unsigned int acpiIndex; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c904d9c63d..fe7e5f9116 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5093,7 +5093,7 @@ virDomainDeviceInfoFormat(virBuffer *buf, g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf); if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) { - virBufferAsprintf(buf, "<boot order='%u'", info->bootIndex); + virBufferAsprintf(buf, "<boot order='%d'", info->bootIndex); if (info->loadparm) virBufferAsprintf(buf, " loadparm='%s'", info->loadparm); @@ -5254,10 +5254,15 @@ virDomainDeviceBootParseXML(xmlNodePtr node, { g_autofree char *loadparm = NULL; - if (virXMLPropUInt(node, "order", 10, - VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, - &info->bootIndex) < 0) + if (virXMLPropInt(node, "order", 10, + VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO, + &info->bootIndex, -1) < 0) return -1; + if (info->bootIndex == 0 || info->bootIndex < -1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("incorrect boot order '%d', expecting positive integer or -1"), + info->bootIndex); + } info->effectiveBootIndex = info->bootIndex; @@ -27652,7 +27657,7 @@ virDomainDeviceInfoCheckBootIndex(virDomainDef *def G_GNUC_UNUSED, if (info->bootIndex == data->newInfo->bootIndex) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("boot order %u is already used by another device"), + _("boot order %d is already used by another device"), data->newInfo->bootIndex); return -1; } diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c index df59de2d0d..9810de7437 100644 --- a/src/conf/domain_postparse.c +++ b/src/conf/domain_postparse.c @@ -1041,7 +1041,7 @@ virDomainDefCollectBootOrder(virDomainDef *def G_GNUC_UNUSED, */ return 0; } - order = g_strdup_printf("%u", info->bootIndex); + order = g_strdup_printf("%d", info->bootIndex); if (virHashLookup(bootHash, order)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a31b8ee438..e124800bcc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1765,7 +1765,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, virTristateSwitch shareRW = VIR_TRISTATE_SWITCH_ABSENT; g_autofree char *chardev = NULL; g_autofree char *drive = NULL; - unsigned int bootindex = 0; + int bootindex = 0; unsigned int logical_block_size = 0; unsigned int physical_block_size = 0; g_autoptr(virJSONValue) wwn = NULL; @@ -1951,7 +1951,7 @@ qemuBuildDiskDeviceProps(const virDomainDef *def, "S:drive", drive, "S:chardev", chardev, "s:id", disk->info.alias, - "p:bootindex", bootindex, + "z:bootindex", bootindex, "p:logical_block_size", logical_block_size, "p:physical_block_size", physical_block_size, "A:wwn", &wwn, @@ -2019,25 +2019,25 @@ qemuCommandAddExtDevice(virCommand *cmd, static void qemuBuildFloppyCommandLineControllerOptionsImplicit(virCommand *cmd, - unsigned int bootindexA, - unsigned int bootindexB) + int bootindexA, + int bootindexB) { if (bootindexA > 0) { virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.bootindexA=%u", bootindexA); + virCommandAddArgFormat(cmd, "isa-fdc.bootindexA=%d", bootindexA); } if (bootindexB > 0) { virCommandAddArg(cmd, "-global"); - virCommandAddArgFormat(cmd, "isa-fdc.bootindexB=%u", bootindexB); + virCommandAddArgFormat(cmd, "isa-fdc.bootindexB=%d", bootindexB); } } static int qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, - unsigned int bootindexA, - unsigned int bootindexB, + int bootindexA, + int bootindexB, const virDomainDef *def, virQEMUCaps *qemuCaps) { @@ -2045,8 +2045,8 @@ qemuBuildFloppyCommandLineControllerOptionsExplicit(virCommand *cmd, if (virJSONValueObjectAdd(&props, "s:driver", "isa-fdc", - "p:bootindexA", bootindexA, - "p:bootindexB", bootindexB, + "z:bootindexA", bootindexA, + "z:bootindexB", bootindexB, NULL) < 0) return -1; @@ -2062,8 +2062,8 @@ qemuBuildFloppyCommandLineControllerOptions(virCommand *cmd, const virDomainDef *def, virQEMUCaps *qemuCaps) { - unsigned int bootindexA = 0; - unsigned int bootindexB = 0; + int bootindexA = 0; + int bootindexB = 0; bool hasfloppy = false; size_t i; @@ -2286,7 +2286,7 @@ qemuBuildVHostUserFsDevProps(virDomainFSDef *fs, "s:chardev", chardev_alias, "P:queue-size", fs->queue_size, "s:tag", fs->dst, - "p:bootindex", fs->info.bootIndex, + "z:bootindex", fs->info.bootIndex, NULL) < 0) return NULL; @@ -3762,7 +3762,7 @@ qemuBuildNicDevProps(virDomainDef *def, "s:netdev", netdev, "s:id", net->info.alias, "s:mac", macaddr, - "p:bootindex", net->info.effectiveBootIndex, + "z:bootindex", net->info.effectiveBootIndex, NULL) < 0) return NULL; @@ -4576,7 +4576,7 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def, "s:driver", "vfio-pci", "s:host", host, "s:id", dev->info->alias, - "p:bootindex", dev->info->effectiveBootIndex, + "z:bootindex", dev->info->effectiveBootIndex, "S:failover_pair_id", failover_pair_id, NULL) < 0) return NULL; @@ -4643,7 +4643,7 @@ qemuBuildUSBHostdevDevProps(const virDomainDef *def, "p:hostbus", hostbus, "p:hostaddr", hostaddr, "s:id", dev->info->alias, - "p:bootindex", dev->info->bootIndex, + "z:bootindex", dev->info->bootIndex, "T:guest-reset", guestReset, "T:guest-resets-all", guestResetsAll, NULL) < 0) @@ -4733,7 +4733,7 @@ qemuBuildSCSIHostdevDevProps(const virDomainDef *def, "s:driver", "scsi-generic", "s:drive", backendAlias, "s:id", dev->info->alias, - "p:bootindex", dev->info->bootIndex, + "z:bootindex", dev->info->bootIndex, NULL) < 0) return NULL; @@ -4831,7 +4831,7 @@ qemuBuildHostdevMediatedDevProps(const virDomainDef *def, "s:sysfsdev", mdevPath, "S:display", qemuOnOffAuto(mdevsrc->display), "B:ramfb", ramfb, - "p:bootindex", dev->info->bootIndex, + "z:bootindex", dev->info->bootIndex, NULL) < 0) return NULL; @@ -9022,7 +9022,7 @@ qemuBuildRedirdevDevProps(const virDomainDef *def, "s:chardev", chardev, "s:id", dev->info.alias, "S:filter", filter, - "p:bootindex", dev->info.bootIndex, + "z:bootindex", dev->info.bootIndex, NULL) < 0) return NULL; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 32f03ff79a..327307de9c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6264,10 +6264,10 @@ static void qemuProcessPrepareDeviceBootorder(virDomainDef *def) { size_t i; - unsigned int bootCD = 0; - unsigned int bootFloppy = 0; - unsigned int bootDisk = 0; - unsigned int bootNetwork = 0; + int bootCD = 0; + int bootFloppy = 0; + int bootDisk = 0; + int bootNetwork = 0; if (def->os.nBootDevs == 0) return; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 6403266559..c3ecd03dfb 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2520,7 +2520,7 @@ qemuValidateDomainMdevDefVFIOPCI(const virDomainHostdevDef *hostdev, } /* VFIO-PCI does not support boot */ - if (hostdev->info->bootIndex) { + if (hostdev->info->bootIndex > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("booting from assigned devices is not " "supported by mediated devices of " @@ -2576,7 +2576,7 @@ qemuValidateDomainMdevDefVFIOAP(const virDomainHostdevDef *hostdev, } /* VFIO-AP does not support boot */ - if (hostdev->info->bootIndex) { + if (hostdev->info->bootIndex > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("booting from assigned devices is not " "supported by mediated devices of " @@ -2687,7 +2687,7 @@ qemuValidateDomainDeviceDefHostdev(const virDomainHostdevDef *hostdev, break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: - if (hostdev->info->bootIndex) { + if (hostdev->info->bootIndex > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("booting from assigned devices is not " "supported by vhost SCSI devices")); -- 2.33.0
On Tue, Oct 11, 2022 at 21:38:31 +0800, Jiang Jiacheng wrote:
Enable bootindex can be set to -1, it means cancel the device's bootindex. Change bootindex's type from unsigned int to int and modify other related codes concered with type.
Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/conf/device_conf.h | 4 ++-- src/conf/domain_conf.c | 15 ++++++++++----- src/conf/domain_postparse.c | 2 +- src/qemu/qemu_command.c | 38 ++++++++++++++++++------------------- src/qemu/qemu_process.c | 8 ++++---- src/qemu/qemu_validate.c | 6 +++--- 6 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index f2907dc596..a9df2e6e5d 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -139,11 +139,11 @@ struct _virDomainDeviceInfo { char *romfile; /* bootIndex is only used for disk, network interface, hostdev * and redirdev devices */ - unsigned int bootIndex; + int bootIndex;
Here you should describe also what the negative value means.
/* 'effectiveBootIndex' is same as 'bootIndex' (if provided in the XML) but * not formatted back. This allows HV drivers to update it if <os><boot .. * is present. */ - unsigned int effectiveBootIndex; + int effectiveBootIndex;
IMO it makes no sense to change the effective boot index value and type as those will be only positive.
/* Valid for any PCI device. Can be used for NIC to get * stable numbering in Linux */ unsigned int acpiIndex; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c904d9c63d..fe7e5f9116 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5093,7 +5093,7 @@ virDomainDeviceInfoFormat(virBuffer *buf, g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) { - virBufferAsprintf(buf, "<boot order='%u'", info->bootIndex); + virBufferAsprintf(buf, "<boot order='%d'", info->bootIndex);
In subsequent patch 7/7 you have a special copy function and state: Since we don't format bootindex's xml when it is -1, we will lost bootindex So either that is wrong or you do in fact format it here. In general I don't think that allowing '-1' for any other case than the changing of bootindex should be allowed. In fact I find it questionable to even allow -1 for it as it can cause problems in corner cases. In fact I'd probably prefer if -1 is not considered at all. Internally we use 0 as no boot index. You can add another bool field 'bootIndexSpecified' and set it to true if user provided the field in XML (virXMLProp*) has different return value if the element was specified. The new field will be used in the specific case when you want to know whether you have to clear the boot index. That way we won't try to overload (and potentially break) the meaning of the actual boot index value. [...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 32f03ff79a..327307de9c 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6264,10 +6264,10 @@ static void qemuProcessPrepareDeviceBootorder(virDomainDef *def) { size_t i; - unsigned int bootCD = 0; - unsigned int bootFloppy = 0; - unsigned int bootDisk = 0; - unsigned int bootNetwork = 0; + int bootCD = 0; + int bootFloppy = 0; + int bootDisk = 0; + int bootNetwork = 0;
This change isn't needed. We'll only ever assign positive values here.
if (def->os.nBootDevs == 0) return;
Support add bootindex = -1 to boothash and return 0 if duplicated bootindex = -1 is set. It is nessary to add bootindex = -1 into boothash, otherwise libvirt will auto set boot dev, which will cause bootindex unchangable. Meanwhile, bootindex = -1 means disable bootindex, so it should be allowed to set duplicated bootindex = -1. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/conf/domain_postparse.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c index 9810de7437..d264a2ffee 100644 --- a/src/conf/domain_postparse.c +++ b/src/conf/domain_postparse.c @@ -1044,6 +1044,12 @@ virDomainDefCollectBootOrder(virDomainDef *def G_GNUC_UNUSED, order = g_strdup_printf("%d", info->bootIndex); if (virHashLookup(bootHash, order)) { + /* -1 means disable bootorder so it allowed to + * make more than one device use -1 + */ + if (info->bootIndex == -1) { + return 0; + } virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("boot order '%s' used for more than one device"), order); -- 2.33.0
Since we don't format bootindex's xml when it is -1, we will lost bootindex if it is set to -1. So we should reserve bootindex = -1 in virDomainDeviceDefCopy to save the message that we cancel a bootindex set. Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com> --- src/conf/domain_conf.c | 23 ++++++++++++++++++++--- src/conf/domain_conf.h | 4 ++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fe7e5f9116..d35fd995e4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28651,6 +28651,7 @@ virDomainDeviceDefCopy(virDomainDeviceDef *src, virDomainXMLOption *xmlopt, void *parseOpaque) { + virDomainDeviceDef* ret; g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; int flags = VIR_DOMAIN_DEF_FORMAT_INACTIVE | VIR_DOMAIN_DEF_FORMAT_SECURE; int rc = -1; @@ -28746,9 +28747,13 @@ virDomainDeviceDefCopy(virDomainDeviceDef *src, return NULL; xmlStr = virBufferContentAndReset(&buf); - return virDomainDeviceDefParse(xmlStr, def, xmlopt, parseOpaque, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); + ret = virDomainDeviceDefParse(xmlStr, def, xmlopt, parseOpaque, + VIR_DOMAIN_DEF_PARSE_INACTIVE | + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); + if (ret) + virDomainDeviceDefCopyBootIndex(ret, src); + + return ret; } @@ -30669,3 +30674,15 @@ virDomainDefHasSpiceGraphics(const virDomainDef *def) return false; } + +void +virDomainDeviceDefCopyBootIndex(virDomainDeviceDef *dest, virDomainDeviceDef *src) +{ + if (src->type == VIR_DOMAIN_DEVICE_DISK && + src->data.disk->info.bootIndex == -1) + dest->data.disk->info.bootIndex = src->data.disk->info.bootIndex; + + if (src->type == VIR_DOMAIN_DEVICE_NET && + src->data.net->info.bootIndex == -1) + dest->data.net->info.bootIndex = src->data.net->info.bootIndex; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 2b1497d78d..8a152df692 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -4330,3 +4330,7 @@ virDomainObjGetMessages(virDomainObj *vm, bool virDomainDefHasSpiceGraphics(const virDomainDef *def); + +void +virDomainDeviceDefCopyBootIndex(virDomainDeviceDef *dest, + virDomainDeviceDef *src); -- 2.33.0
On Tue, Oct 11, 2022 at 21:38:26 +0800, Jiang Jiacheng wrote:
Support updating device's(support cdrom, disk and network) bootindex online in virDomainUpdateDeviceFlags. The new bootindex will take effect after guest rebooting. Enable bootindex can be set to -1, it means cancel the device's bootindex.
To use this feature, we need to get the device's xml first and modify the boot order in the xml, then use 'virsh update-device <domain> <xml> --flag' to update the bootindex. Note that the flag should be --config or --persistent if the vm is running.
Your series doesn't pass our test suite, namely there are trailing spaces and missing '%s' in an error message. Make sure you run the test suite including syntax-check before your next submission: https://www.libvirt.org/hacking.html#preparing-patches
participants (2)
-
Jiang Jiacheng -
Peter Krempa