From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35892) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4TBW-0007Kp-WF for qemu-devel@nongnu.org; Tue, 08 Jul 2014 07:03:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X4TBO-0006sP-0C for qemu-devel@nongnu.org; Tue, 08 Jul 2014 07:03:42 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:44478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X4TBN-0006ru-8r for qemu-devel@nongnu.org; Tue, 08 Jul 2014 07:03:33 -0400 Message-ID: <53BBCFCA.3090606@huawei.com> Date: Tue, 8 Jul 2014 19:02:34 +0800 From: ChenLiang MIME-Version: 1.0 References: <1404724261-9412-1-git-send-email-arei.gonglei@huawei.com> <1404724261-9412-2-git-send-email-arei.gonglei@huawei.com> <20140708083302.GB3962@z.redhat.com> In-Reply-To: <20140708083302.GB3962@z.redhat.com> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 1/5] bootindex: add *_boot_device_path function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: peter.maydell@linaro.org, weidong.huang@huawei.com, mst@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org, agraf@suse.de, kraxel@redhat.com, dmitry@daynix.com, armbru@redhat.com, arei.gonglei@huawei.com, lersek@redhat.com, marcel.a@redhat.com, somlo@cmu.edu, luonengjun@huawei.com, peter.huangpeng@huawei.com, alex.williamson@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, rth@twiddle.net, kwolf@redhat.com, peter.crosthwaite@xilinx.com, imammedo@redhat.com, afaerber@suse.de 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 >> >> 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 >> Signed-off-by: Gonglei >> --- >> 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 >> >