From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41810) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VK506-0001Rs-91 for qemu-devel@nongnu.org; Thu, 12 Sep 2013 07:24:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VK500-0006uO-9j for qemu-devel@nongnu.org; Thu, 12 Sep 2013 07:23:54 -0400 Received: from mx1.redhat.com ([209.132.183.28]:10949) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VK500-0006uH-0h for qemu-devel@nongnu.org; Thu, 12 Sep 2013 07:23:48 -0400 Message-ID: <1378985032.2186.36.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Thu, 12 Sep 2013 14:23:52 +0300 In-Reply-To: <874n9qe1xi.fsf@blackfin.pond.sub.org> References: <1378924006-14057-1-git-send-email-marcel.a@redhat.com> <5231721D.4000109@redhat.com> <87sixagysq.fsf@blackfin.pond.sub.org> <1378982018.2186.23.camel@localhost.localdomain> <874n9qe1xi.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qdev-monitor: Avoid exiting when hot-plugging two devices with the same bootindex value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Paolo Bonzini , aliguori@us.ibm.com, qemu-devel@nongnu.org, afaerber@suse.de, sw@weilnetz.de On Thu, 2013-09-12 at 13:04 +0200, Markus Armbruster wrote: > Marcel Apfelbaum writes: > > > On Thu, 2013-09-12 at 11:43 +0200, Markus Armbruster wrote: > >> Paolo Bonzini 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