Support add bootindex = 0 to boothash and return 0 if duplicated bootindex = 0 is set.
It is nessary to add bootindex = 0 into boothash, otherwise libvirt will auto set boot dev,
which will cause bootindex unchangable. Meanwhile, bootindex = 0 means disable bootindex,
so it should be allowed to set duplicated bootindex = 0.

Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
---
 src/conf/domain_postparse.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_postparse.c b/src/conf/domain_postparse.c
index 9a3e8f494c..2ba3186561 100644
--- a/src/conf/domain_postparse.c
+++ b/src/conf/domain_postparse.c
@@ -1031,7 +1031,7 @@ virDomainDefCollectBootOrder(virDomainDef *def G_GNUC_UNUSED,
     GHashTable *bootHash = data;
     g_autofree char *order = NULL;
 
-    if (info->bootIndex == 0)
+    if (info->bootIndex == 0 && !info->bootIndexSpecified)
         return 0;
 
     if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV &&
@@ -1045,6 +1045,12 @@ virDomainDefCollectBootOrder(virDomainDef *def G_GNUC_UNUSED,
     order = g_strdup_printf("%u", info->bootIndex);
 
     if (virHashLookup(bootHash, order)) {
+        /* 0 in the bootHash means cancel the bootIndex specified in
+         * XML, so it allowed to make more than one device use 0 with
+         * its bootIndexSpecified = true.
+         */
+        if (info->bootIndex == 0)
+            return 0;
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("boot order '%s' used for more than one device"),
                        order);
-- 
2.33.0

On Thu, Nov 17, 2022 at 10:05:32 +0800, Jiang Jiacheng wrote:
> Support add bootindex = 0 to boothash and return 0 if duplicated bootindex = 0 is set.
> It is nessary to add bootindex = 0 into boothash, otherwise libvirt will auto set boot dev,
> which will cause bootindex unchangable. Meanwhile, bootindex = 0 means disable bootindex,
> so it should be allowed to set duplicated bootindex = 0.

Libvirt auto-sets boot order when there is none specified. With this
patch you could un-select all bootable devices and end up with an
un-bootable VM. Do you really want to allow that?

Also doing this will require that we hohour the explicit bootindex setting
of 0 in the XML separate from when it's unpopulated. Thus this requires
that the XML indeed records that an explicit value of 0 was provided by
the user.

Thus you must improve patch 2 to carry the appropriate tests and
documentation if you want to go this route.

> Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
> ---
>  src/conf/domain_postparse.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

On 2022/11/22 23:52, Peter Krempa wrote:
> On Thu, Nov 17, 2022 at 10:05:32 +0800, Jiang Jiacheng wrote:
>> Support add bootindex = 0 to boothash and return 0 if duplicated bootindex = 0 is set.
>> It is nessary to add bootindex = 0 into boothash, otherwise libvirt will auto set boot dev,
>> which will cause bootindex unchangable. Meanwhile, bootindex = 0 means disable bootindex,
>> so it should be allowed to set duplicated bootindex = 0.
> 
> Libvirt auto-sets boot order when there is none specified. With this
> patch you could un-select all bootable devices and end up with an
> un-bootable VM. Do you really want to allow that?
> 
> Also doing this will require that we hohour the explicit bootindex setting
> of 0 in the XML separate from when it's unpopulated. Thus this requires
> that the XML indeed records that an explicit value of 0 was provided by
> the user.
> 
> Thus you must improve patch 2 to carry the appropriate tests and
> documentation if you want to go this route.
> 
Yeah, this what I mean in patch2. Qemu will boot in an default sequence
'disk->cdrom->net' in those devices whose bootindex is set to explicit
0. I will improve all those patches in next version.

>> Signed-off-by: Jiang Jiacheng <jiangjiacheng@huawei.com>
>> ---
>>  src/conf/domain_postparse.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
>