Re: [PATCH 2/7] qemu: Add bootIndexSpecified and support set bootIndex to '0'
On Thu, Nov 17, 2022 at 10:05:28 +0800, Jiang Jiacheng wrote:
Add a bool bootIndexSpecified into _virDomainDeviceInfo, which means
whether
the bootindex could be update or not. BootIndexSpecified will be set to
true if bootindex is set in XML.
Support set bootindex to 0, which means cancel the previous bootindex setting,
and format boot order = '0' into XML when its bootIndexSpecified is true to
keep its bootindex changeble.
Signed-off-by: Jiang Jiacheng <jiangjiacheng(a)huawei.com>
---
src/conf/device_conf.h | 3 +++
src/conf/domain_conf.c | 9 ++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
You'll need to note in the documentation the special 0 value and how it
behaves when updating a device.
In case where you disagree with my comment below that the bootindex
should not be in the output XML you'll also need to add XML2XML test
cases.
diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
index f2907dc596..5259e25c10 100644
--- a/src/conf/device_conf.h
+++ b/src/conf/device_conf.h
@@ -144,6 +144,9 @@ struct _virDomainDeviceInfo {
* not formatted back. This allows HV drivers to update it if <os><boot ..
* is present. */
unsigned int effectiveBootIndex;
+ /* bootIndexSpecified is set to true when device's bootIndex is provided in
+ * the XML. This allows changing bootIndex online of some devices. */
+ bool bootIndexSpecified;
/* 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 3790121cf7..dcd9696a93 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5142,7 +5142,10 @@ virDomainDeviceInfoFormat(virBuffer *buf,
g_auto(virBuffer) attrBuf = VIR_BUFFER_INITIALIZER;
g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
- if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && info->bootIndex) {
+ if ((flags & VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT) && (info->bootIndex ||
info->bootIndexSpecified)) {
+ /* format the boot order = 0 in XML when its bootIndexSpecified is true,
+ * which means the boot order could be changed by virsh update-device.
+ */
I'm not persuaded that the logic here needs changing. If the boot index
is 0 and the device is not bootable we should avoid the element. This
includes cases where the device was updated in order co clear the boot
index.
virBufferAsprintf(buf, "<boot
order='%u'", info->bootIndex);
if (info->loadparm)
@@ -5304,12 +5307,12 @@ virDomainDeviceBootParseXML(xmlNodePtr node,
{
g_autofree char *loadparm = NULL;
- if (virXMLPropUInt(node, "order", 10,
- VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO,
+ if (virXMLPropUInt(node, "order", 10, VIR_XML_PROP_REQUIRED,
&info->bootIndex) < 0)
return -1;
info->effectiveBootIndex = info->bootIndex;
+ info->bootIndexSpecified = true;
loadparm = virXMLPropString(node, "loadparm");
if (loadparm) {
--
2.33.0