On 2022/11/22 23:06, Peter Krempa wrote:
> On Thu, Nov 17, 2022 at 10:05:29 +0800, Jiang Jiacheng wrote:
>> 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@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_conf.c | 67 ++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_conf.h | 7 +++++
>
> qemu_conf.* seems to be inappropriate file for these helpers. qemu_conf
> is a module which deals mostly with the configuration of the qemu driver
> itself and host-side specifics.
>
> qemu_domain is usually a better fit.
>
Thank for your suggestion, I will move it to qemu_domain in my next patch.
>> 2 files changed, 74 insertions(+)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index 0071a95cb6..9a7992db01 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -1681,3 +1681,70 @@ qemuDomainChangeBootIndex(virDomainObj *vm,
>>
>> return ret;
>> }
>> +
>> +/**
>> + * qemuCheckBootIndex:
>> + * @devInfo: origin device info
>> + * @new_bootindex: new bootIndex
>> + *
>> + * check whether the device's bootIndex could be changed or neet to
>> + * be changed
>> + *
>> + * Returns: 1 on need to change
>> + * 0 on don't need to change
>> + * -1 on could not change with an error
>> + */
>> +int
>> +qemuCheckBootIndex(virDomainDeviceInfo *devInfo,
>> + const int new_bootindex)
>> +{
>> + if (!devInfo->bootIndexSpecified) {
>> + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>> + _("this device does not set boot index, cannot change it."));
>> + return -1;
>
> This check doesn't seem to make much sense. What if you want to add boot
> index?>
The bootindex changable devices is fixed when start the domain, so it is
not supported to add a boot index, this check is used to prevent them.
However, it do have problems as you mentioned in next patch, the logical
here should be optimized.
>> + }
>> +
>> + /* if the new bootindex is different from the old bootindex,
>> + * we will update the bootindex. */
>> + if (devInfo->bootIndex != new_bootindex) {
>> + return 1;
>> + }
>> +
>> + return 0;
>> +}
>
