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

[Amos Kong]

On Mon, Jul 07, 2014 at 05:10:57PM +0800, address@hidden wrote:
> From: Chenliang <address@hidden>
> 
> 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 <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> ---
>  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?

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

       if (bootindex < 0) {

> +        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?

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

-- 
                        Amos.