On 2022/11/22 23:40, Peter Krempa wrote:
> On Thu, Nov 17, 2022 at 10:05:31 +0800, Jiang Jiacheng wrote:
>> 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@xxxxxxxxxx>
>> ---
>> 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 65abc04998..863b779514 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -7568,6 +7568,10 @@ qemuDomainUpdateDeviceConfig(virDomainDef *vmdef,
>> false) < 0)
>> return -1;
>>
>> + if (qemuCheckBootIndex(&vmdef->nets[pos]->info,
>> + net->info.bootIndex) < 0)
>> + return -1;
>
> Same as with disks. This check makes no sense for the *Config variant.
>
>> +
>> if (virDomainNetUpdate(vmdef, pos, net))
>> return -1;
>>
>> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>> index da92ced2f4..5d1913d0c7 100644
>> --- a/src/qemu/qemu_hotplug.c
>> +++ b/src/qemu/qemu_hotplug.c
>> @@ -3468,6 +3468,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;
>> @@ -3633,11 +3634,8 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>> goto cleanup;
>> }
>>
>> - if (newdev->info.bootIndex == 0)
>> - newdev->info.bootIndex = olddev->info.bootIndex;
>
> You are deleting too much of the logic here, this code specifically
> ensures that if the new device's boot index is not specified it adopts
> the old one, thus this needs to be changed suich that it's still adopted
> in cases when an explicit new boot index was not specified.
>
I use 'boot order = 0' or new bootindex was not specified to cancel the
bootindex setting. It seems not specified bootindex means inherit the
old bootindex? But I can't find similiar code with disk, is this only
used for net?
>> - 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,
>> + newdev->info.bootIndex)) < 0) {
>
> This once again (due to the weird check) breaks update of network
> devices which don't have bootindex enabled.
>
See above.
>> goto cleanup;
>> }
>>
>
