On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > Marcel Apfelbaum <marce...@redhat.com> writes: > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >> Paolo Bonzini <pbonz...@redhat.com> writes: > >> > >> > Il 11/09/2013 20:26, Marcel Apfelbaum ha scritto: > >> >> Qemu is expected to quit if the same boot index value is used by > >> >> two devices. > >> >> However, hot-plugging a device with a bootindex value already used > >> >> should > >> >> fail with a friendly message rather than quitting a running VM. > >> > > >> > I think the problem is right where QEMU exits, i.e. in > >> > add_boot_device_path. This function should return an error instead, via > >> > an Error ** argument. > >> > >> Agree. > >> > >> > Callers, typically a device's init or realize function, will either > >> > print the error before returning an error code (e.g. -EBUSY for init) or > >> > propagate the error up (for realize). > >> > > >> > Returning/propagating failure will still cause QEMU to exit when the > >> > duplicate bootindexes are found on the command line. > >> > >> I have an unfinished patch in my tree that does exactly that. It's > >> unfinished, because cleanup on error paths needs work. Current state > >> appended with FIXMEs and all. Beware, the FIXMEs may not be correct and > >> are almost certainly not complete. > > Thanks Markus, > > Should I use it as my starting point and finish it or you intend to? > > If you have cycles to spare, you're quite welcome to take this patch and > run with it! I'll try to finish it, thanks! > > You may have noticed that my patch moves the code to add the boot device > path in a few cases. I did this in the hope of simplifying error paths. > Do not hesitate to undo such moves where they turn out not to simplify > anything. > > Here's an issue that exists before my patch: DeviceClass method > unrealize() should clean up everything done by realize(). In > particular, unrealize() needs to remove any entry added to fw_boot_order > by realize() via add_boot_device_path(). Code to do that doesn't exist, > yet. Hot unplug is technically broken for all devices with bootindex. > Impact unknown. Should probably be fixed in a separate patch. The outcome is that after hot-unplugging a device with bootindex=x, the device still remains in the fw_boot_order and no device can be hot-plugged with the same bootindex 'x'. Furthermore, because of the current issue Qemu quits on an operation that should succeed! I plan to send a patch to fix this too.
Thanks, Marcel
