Hi, > > > > > > > + del_boot_device_path(dev); > > > > > > You can call this from device_finalize() instead of placing it into each > > > individual device. > > > > > Maybe put this call in device_finalize is not a good idea. > > I have three reasons: > > > > 1. the device's some memory have been freed before call device_finalize, > > such as device->id. It is too later. > > I don't think you even need id. See my reply to v4 2/8. > > But you have a point about being too late: some devices call > add_boot_device_path() on realize, so those would need to revert the > operation on unrealize; others do it on init, so they need to do it on > finalize. > > On either case, I believe an extra check inside device_finalize() > wouldn't hurt, even if it becomes redundant on some devices. > > OK. And I copy your review from v3 2/7, as follows:
> > What if the device doesn't have any ID set? I don't see anything on > add_boot_device_path() ensuring that dev->id is always set. > Yes, the id is not always set. So, I add a check in V4. > Why you don't just check if i->dev == dev? > No, if we check directly i->dev == dev, we will not handle the virtio devices. For example, the common user configure a virtio-net nic using command line like " -device virtio-net-pci,netdev=net0,bootindex=3,id=nic1 ". Then the id property will be added for virtio-net-pci device, not virtio-net device which stored in the global fw_boot_order list. So, the i->dev When common users want to change the bootindex of virtio-net. They only are concerned that they have configured an id for virtio-net nic card. So, they can pass the id to QEMU. But we should handle those scenes, meanwhile the device object gained by id is virtio-net-pci device not equals i->dev. > > 2. not every kinds of device can configure bootindex property, such as usb > > host adapters. It is a waste and useless for those devices. This is the > > main reason. > > I would prefer to waste a few cycles scanning the boot index list every > time a device is removed, than risking crashing QEMU in case somebody > forget to add a del_boot_device_path() call. > OK, fine! Maybe I should do this in device_finalize() as Gerd's previous suggestion, like yours. Thanks. > > > 3. virtio-net device's parent is virtio-pci device, which configured id > > property, > > But the device saved in global fw_boot_order list is virtio-net device have > not > > id property. If we put call del_boot_device_path(dev) in > virtio_net_device_unrealize > > we can delete it from fw_boot_order directly. > > Sorry, I don't understand what you mean here. If virtio-net doesn't have > an id property, would the current version of del_boot_device_path() even > work? > Please see above comments. Thanks for your review! Best regards, -Gonglei
