From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46600) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtJtP-0000C9-OB for qemu-devel@nongnu.org; Wed, 18 Dec 2013 11:22:45 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VtJtJ-0005mh-Lb for qemu-devel@nongnu.org; Wed, 18 Dec 2013 11:22:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:17999) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VtJtJ-0005lT-Co for qemu-devel@nongnu.org; Wed, 18 Dec 2013 11:22:33 -0500 Date: Wed, 18 Dec 2013 18:26:07 +0200 From: "Michael S. Tsirkin" Message-ID: <20131218162607.GA21916@redhat.com> References: <1386938688-22433-1-git-send-email-imammedo@redhat.com> <87r49c1ioi.fsf@codemonkey.ws> <52B045BB.6010900@redhat.com> <52B17AC4.6080709@redhat.com> <20131218164809.7b4ddb07@nial.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20131218164809.7b4ddb07@nial.usersys.redhat.com> Subject: Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: Kevin Wolf , Peter Maydell , Peter Crosthwaite , Eduardo Habkost , marcel.a@redhat.com, qemu-devel , Blue Swirl , Alex Williamson , Gerd Hoffmann , Anthony Liguori , Paolo Bonzini , dkoch@verizon.co, Andreas =?iso-8859-1?Q?F=E4rber?= On Wed, Dec 18, 2013 at 04:48:09PM +0100, Igor Mammedov wrote: > On Wed, 18 Dec 2013 11:36:52 +0100 > Paolo Bonzini wrote: > > > Il 17/12/2013 20:38, Anthony Liguori ha scritto: > > > On Tue, Dec 17, 2013 at 4:38 AM, Paolo Bonzini wrote: > > >> Il 17/12/2013 00:26, Anthony Liguori ha scritto: > > >>> Sharing hot plug code is a good thing. Making hotplug a qdev-level > > >>> concept seems like a bad thing to me. > > >> > > >> Can you explain what you mean? > > > > > > The question is whether "hotpluggable" as a property applies to all > > > devices or not. > I think Andreas asked me to provide "hotpluggable" property to > distinguish hotpluggable vs not hotpluggable DimmDevice via qom interface. > > > > > > > But hotplug is strictly a bus level concept. It's a sequence of > > > events that correspond to what happens when you add a new device to a > > > bus after power on. > > > > Hotplugging a device is a special case of plugging a device. If a bus > > or device only supports cold-plug, that can be done using > > "bc->allow_hotplug = false" or "dc->hotpluggable = false". > Do we need per instance ability to set "hotpluggable" property? > For example board might want to mark some CPUs as not hotpluggable. It could be useful. In real life same device can be on-board or on a plugin card. But it's not a must, we survived without this so far. So maybe start not supporting it, add later? > > > > Igor's interface applies just as well to the case of plugging a device > > at startup; I think separating the two makes little sense. And once you > > have cold-plug and hot-plug in qdev core, it makes sense to add unplug > > as well. Also because we already have surprise removal in qdev core > > (that's unparent) and we have some kind of unplug request support > > (device_del/dc->unplug). > > > > One possibility that remains is to put cold/hot-plug in a "BusDevice" > > class rather than in the core qdev: > > > > Device > > BusDevice <-- can be cold/hot-plugged > > > > but this adds more complication. For example, the same CPU can be > > hotpluggable or not depending on the board model, should the superclass > > be Device or BusDevice. And if we ever have multi-CPU targets, with the > > "core" CPU not hotpluggable and additional hotpluggable ones (e.g. for > > GPUs) what would be the superclass of the common CPU superclass? > > > > > The question is whether there can be code sharing without touching the > > > base class. You could certainly have a HotpluggableBusState and then > > > a HotpluggableDeviceState. > > > > > > Interfaces would be another option too. > > > > Interfaces are fine, but the question is who finds them and calls them. > > In this case, the discovery mechanism is a link property, and the > > calling mechanism is an explicit hook in the "realized" property. > If we don't need per instance "hotpluggable" state and we can call > interfaces from generic qdev/device code, then we would need at first > only TYPE_HOTPLUGGABLE_BUS_DEVICE_IF and later for link<> based hotplug > we could add just TYPE_HOTPLUGGABLE_DEVICE_IF. Difference would be in > the way they get access to hotplug device link, former one will use bus > for it and second some other way. > > > > > If we had aspect-oriented programming, we would be marking join points > > instead of writing "if (dev->foo) bar(dev->foo)" conditionals. But the > > idea is the same. > > > > > The general concern is about polluting widely used base classes. It's > > > better if we can avoid adding things to DeviceState and Object > > > whenever possible. > > > > I agree. At the same time we should make base classes as small as > > possible, but not smaller than that. > > > > Paolo > >