| Message ID | 1406191142-16556-2-git-send-email-arei.gonglei@huawei.com |
|---|---|
| State | New |
| Headers | show |
On Do, 2014-07-24 at 16:38 +0800, arei.gonglei@huawei.com wrote: > From: Gonglei <arei.gonglei@huawei.com> > > When we want to change one device's bootindex, we should do three > things. On the on hand, remove the device from global fw_boot_order list, > regardless attaching suffix or not delete. On the other hand, delete > original object of the assigned bootindex. Finally add the new device's > bootindex into the global fw_boot_order list. Hmm. I think we should simply lookup the device and modify the bootindex, leaving the entry as-is otherwise. In case the new bootindex is already used by another device just throw an error. > +void del_boot_device_path(int32_t bootindex, DeviceState *dev, > + const char *suffix); Should be del_boot_device_path(DeviceState *dev) and simply delete all entries belonging to the device. Patch #3 can be much simpler then as we can call the function from generic device cleanup code. > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev, > + const char *suffix); No need for suffix here if we just update the existing entry. cheers, Gerd
Hi, Gerd > -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Thursday, July 24, 2014 6:09 PM > To: Gonglei (Arei) > Cc: qemu-devel@nongnu.org; afaerber@suse.de; agraf@suse.de; > stefanha@redhat.com; akong@redhat.com; aik@ozlabs.ru; > alex.williamson@redhat.com; armbru@redhat.com; eblake@redhat.com; > kwolf@redhat.com; peter.maydell@linaro.org; lcapitulino@redhat.com; > mst@redhat.com; pbonzini@redhat.com; lersek@redhat.com; > imammedo@redhat.com; dmitry@daynix.com; marcel.a@redhat.com; > peter.crosthwaite@xilinx.com; rth@twiddle.net; somlo@cmu.edu; > Huangweidong (C); Luonengjun; Huangpeng (Peter); chenliang (T) > Subject: Re: [PATCH 1/6] bootindex: add {del,modify}_boot_device_path > function > > On Do, 2014-07-24 at 16:38 +0800, arei.gonglei@huawei.com wrote: > > From: Gonglei <arei.gonglei@huawei.com> > > > > When we want to change one device's bootindex, we should do three > > things. On the on hand, remove the device from global fw_boot_order list, > > regardless attaching suffix or not delete. On the other hand, delete > > original object of the assigned bootindex. Finally add the new device's > > bootindex into the global fw_boot_order list. > > Hmm. I think we should simply lookup the device and modify the > bootindex, leaving the entry as-is otherwise. In case the new bootindex > is already used by another device just throw an error. > If we just throw an error but not change the bootindex is already used, we cannot achieve our purpose. For example, we configure a hard disk, which bootindex=1, a nic which bootindex=2. If we want to boot the guest from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the bootindex=1 always be used. > > +void del_boot_device_path(int32_t bootindex, DeviceState *dev, > > + const char *suffix); > > Should be del_boot_device_path(DeviceState *dev) and simply delete all > entries belonging to the device. Patch #3 can be much simpler then as > we can call the function from generic device cleanup code. > Because the IDE device may configure two kind of disk, HD and CDROM, we have to distinguish them by suffix. > > +void modify_boot_device_path(int32_t bootindex, DeviceState *dev, > > + const char *suffix); > > No need for suffix here if we just update the existing entry. > > cheers, > Gerd > Best regards, -Gonglei
Hi, > > Hmm. I think we should simply lookup the device and modify the > > bootindex, leaving the entry as-is otherwise. In case the new bootindex > > is already used by another device just throw an error. > > > If we just throw an error but not change the bootindex is already used, > we cannot achieve our purpose. For example, we configure a hard disk, > which bootindex=1, a nic which bootindex=2. If we want to boot the guest > from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the > bootindex=1 always be used. No. The devices are simply sorted by bootindex. You don't have to use '1'. And you can have holes in your numbering and use --for example -- 3+5. So you can start qemu with hd=2,cdrom=3,nic=4, then set nic=1 or cdrom=1 for a guest install, change it back when done. > > Should be del_boot_device_path(DeviceState *dev) and simply delete all > > entries belonging to the device. Patch #3 can be much simpler then as > > we can call the function from generic device cleanup code. > > > Because the IDE device may configure two kind of disk, HD and CDROM, we > have to distinguish them by suffix. Yes, the suffix indicated whenever the device is a disk or cdrom. But you'll never have both cdrom+disk paths attached to a single device. Therefore the device is enough to identify the bootpath entry, you don't need the suffix for that. cheers, Gerd
> -----Original Message----- > From: Gerd Hoffmann [mailto:kraxel@redhat.com] > Sent: Thursday, July 24, 2014 9:25 PM > Subject: Re: [PATCH 1/6] bootindex: add {del,modify}_boot_device_path > function > > Hi, > > > > Hmm. I think we should simply lookup the device and modify the > > > bootindex, leaving the entry as-is otherwise. In case the new bootindex > > > is already used by another device just throw an error. > > > > > If we just throw an error but not change the bootindex is already used, > > we cannot achieve our purpose. For example, we configure a hard disk, > > which bootindex=1, a nic which bootindex=2. If we want to boot the guest > > from nic firstly, we should must set the nic's bootindex to 1. AFAICT, the > > bootindex=1 always be used. > > No. The devices are simply sorted by bootindex. You don't have to use > '1'. And you can have holes in your numbering and use --for example -- > 3+5. > Yes, you are right. I make a mistake because libvirt will default assign the first disk as bootindex=1. > So you can start qemu with hd=2,cdrom=3,nic=4, then set nic=1 or cdrom=1 > for a guest install, change it back when done. > Yes. > > > Should be del_boot_device_path(DeviceState *dev) and simply delete all > > > entries belonging to the device. Patch #3 can be much simpler then as > > > we can call the function from generic device cleanup code. > > > > > Because the IDE device may configure two kind of disk, HD and CDROM, we > > have to distinguish them by suffix. > > Yes, the suffix indicated whenever the device is a disk or cdrom. But > you'll never have both cdrom+disk paths attached to a single device. > Therefore the device is enough to identify the bootpath entry, you don't > need the suffix for that. > Agreed. Thank you so much, Gerd. A new version will be posted later. Best regards, -Gonglei
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index d8539fd..e368627 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -209,6 +209,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 fe451aa..e304a93 100644 --- a/vl.c +++ b/vl.c @@ -1248,6 +1248,63 @@ 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(bootindex >= 0 || suffix != NULL); + + QTAILQ_FOREACH(i, &fw_boot_order, link) { + if (is_same_fw_dev_path(i->dev, dev)) { + if (suffix && i->suffix && strcmp(i->suffix, suffix)) { + continue; + } + /* if suffix is NULL, remove all entries of the assigend dev */ + QTAILQ_REMOVE(&fw_boot_order, i, link); + g_free(i->suffix); + g_free(i); + break; + } + } + + if (bootindex < 0) { + return; + } + /* find the object of assigned bootindex, and then remove it */ + 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); +} + DeviceState *get_boot_device(uint32_t position) { uint32_t counter = 0;