Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function

[ChenLiang <chenliang88@huawei.com>]

On 2014/7/8 16:33, Amos Kong wrote:

> On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gonglei@huawei.com wrote:
>> From: Chenliang <chenliang88@huawei.com>
>>
>> Add del_boot_device_path and modify_boot_device_path. Device should
>> be removed from boot device list  by del_boot_device_path when device
>> hotplug. modify_boot_device_path is used to modify deviceboot order.
> 
> s/hotplug/is unhotplugged/
> 
> same issue in commitlog of patch 3/5
>  
>> Signed-off-by: Chenliang <chenliang88@huawei.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>>  include/sysemu/sysemu.h |  4 ++++
>>  vl.c                    | 55 +++++++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
>> index 285c45b..38ef1cd 100644
>> --- a/include/sysemu/sysemu.h
>> +++ b/include/sysemu/sysemu.h
>> @@ -204,6 +204,10 @@ void usb_info(Monitor *mon, const QDict *qdict);
>>  
>>  void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>>                            const char *suffix);
>> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                          const char *suffix);
>> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                             const char *suffix);
>>  char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
>>  
>>  DeviceState *get_boot_device(uint32_t position);
>> diff --git a/vl.c b/vl.c
>> index a1686ef..6264e11 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -1247,6 +1247,61 @@ void add_boot_device_path(int32_t bootindex, DeviceState *dev,
>>      QTAILQ_INSERT_TAIL(&fw_boot_order, node, link);
>>  }
>>  
>> +static bool is_same_fw_dev_path(DeviceState *src, DeviceState *target)
>> +{
>> +    bool ret = false;
>> +    char *devpath_src = qdev_get_fw_dev_path(src);
>> +    char *devpath_target = qdev_get_fw_dev_path(target);
>> +
>> +    if (!strcmp(devpath_src, devpath_target)) {
>> +        ret = true;
>> +    }
>> +
>> +    g_free(devpath_src);
>> +    g_free(devpath_target);
>> +    return ret;
>> +}
>> +
>> +void del_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                          const char *suffix)
>> +{
>> +    FWBootEntry *i;
>> +
>> +    assert(dev != NULL);
>> +
> 
> assert(booindex >= 0 || suffix != NULL);
> 
>> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
>> +        if (is_same_fw_dev_path(i->dev, dev)) {
> 
>                if (!suffix) {
>                    break;
>                }
> 
>> +            if (suffix && i->suffix && strcmp(i->suffix, suffix)) {
>> +                continue;
>> +            }
> 
> If suffix is NULL, then all the entries will be removed?


yes, it will be if caller don't give suffix.

> 
>> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
>> +            g_free(i->suffix);
>> +            g_free(i);
>> +            break;
>> +        }
>> +    }
>> +
>> +    if (bootindex == -1) {
> 
>    if (bootindex < 0) {


acked

> 
>> +        return;
>> +    }
>> +
>> +    QTAILQ_FOREACH(i, &fw_boot_order, link) {
>> +        if (i->bootindex == bootindex) {
>> +            QTAILQ_REMOVE(&fw_boot_order, i, link);
>> +            g_free(i->suffix);
>> +            g_free(i);
>> +            break;
>> +        }
>> +    }
>> +}
>> +
>> +void modify_boot_device_path(int32_t bootindex, DeviceState *dev,
>> +                             const char *suffix)
>> +{
>> +    del_boot_device_path(bootindex, dev, suffix);
>> +    add_boot_device_path(bootindex, dev, suffix);
> 
> Why do you directly modify existed entry?


Sometimes, in old boot device list:

device_id      bootindex
net0           1
net1           2
net2           3

we want to make vm reboot from net2, we can do it like this:

modify_boot_device_path(bootindex=1, DeviceState=net2, suffix=NULL), the new boot device list will like this:

device_id     bootindex
net2          1
net1          2


Best regards
Chenliang

> 
>> +}
>> +
>>  DeviceState *get_boot_device(uint32_t position)
>>  {
>>      uint32_t counter = 0;
>> -- 
>> 1.7.12.4
>>
>