> -----Original Message----- > From: Amos Kong [mailto:ak...@redhat.com] > Sent: Tuesday, July 08, 2014 10:55 PM > To: Gonglei (Arei) > Cc: chenliang (T); qemu-devel@nongnu.org; afaer...@suse.de; > ag...@suse.de; stefa...@redhat.com; a...@ozlabs.ru; > alex.william...@redhat.com; arm...@redhat.com; ebl...@redhat.com; > kw...@redhat.com; peter.mayd...@linaro.org; lcapitul...@redhat.com; > m...@redhat.com; pbonz...@redhat.com; ler...@redhat.com; > kra...@redhat.com; imamm...@redhat.com; dmi...@daynix.com; > marce...@redhat.com; peter.crosthwa...@xilinx.com; r...@twiddle.net; > so...@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter) > Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function > > On Tue, Jul 08, 2014 at 01:22:53PM +0000, Gonglei (Arei) wrote: > > > -----Original Message----- > > > From: chenliang (T) > > > Sent: Tuesday, July 08, 2014 7:03 PM > > > To: Amos Kong > > > Cc: Gonglei (Arei); qemu-devel@nongnu.org; afaer...@suse.de; > > > ag...@suse.de; stefa...@redhat.com; a...@ozlabs.ru; > > > alex.william...@redhat.com; arm...@redhat.com; ebl...@redhat.com; > > > kw...@redhat.com; peter.mayd...@linaro.org; lcapitul...@redhat.com; > > > m...@redhat.com; pbonz...@redhat.com; ler...@redhat.com; > > > kra...@redhat.com; imamm...@redhat.com; dmi...@daynix.com; > > > marce...@redhat.com; peter.crosthwa...@xilinx.com; r...@twiddle.net; > > > so...@cmu.edu; Huangweidong (C); Luonengjun; Huangpeng (Peter) > > > Subject: Re: [RFC PATCH 1/5] bootindex: add *_boot_device_path function > > > > > > On 2014/7/8 16:33, Amos Kong wrote: > > > > > > > On Mon, Jul 07, 2014 at 05:10:57PM +0800, arei.gong...@huawei.com > > > wrote: > > > >> From: Chenliang <chenlian...@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 > > > > > > > > Yep, thanks! > > > > > >> Signed-off-by: Chenliang <chenlian...@huawei.com> > > > >> Signed-off-by: Gonglei <arei.gong...@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); > > > > > > > > suffix call be NULL. > > > > > >> + QTAILQ_FOREACH(i, &fw_boot_order, link) { > > > >> + if (is_same_fw_dev_path(i->dev, dev)) { > > > > > > > > if (!suffix) { > > > > break; > > > > } > > If suffix is NULL, at least we should do nothing in the loop. > > > > > > > > >> + if (suffix && i->suffix && strcmp(i->suffix, suffix)) { > > > >> + continue; > > > >> + } > > How about this one? > > if (!suffix) { > break; > } else (i->suffix && strcmp(i->suffix, suffix)) { > continue; > } > > Agreed.
It's more better, which will reduce useless loop, thanks! > > > > > > > > If suffix is NULL, then all the entries will be removed? > > > > If suffix is NULL, the entry will be checked by the bootindex as below > > QTAILQ_FOREACH loop. If suffix and bootindex are all not match, > > no entry will not be deleted from the global fw_boot_order list. > > This is why I added "assert(booindex >= 0 || suffix != NULL);" on > above. > > ^^^^ OK. You are right. Thanks, Amos. Best regards, -Gonglei
