From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] KVM call minutes for Feb 8 Date: Sun, 13 Feb 2011 09:31:55 -0600 Message-ID: <4D57F96B.7010004@codemonkey.ws> References: <4D52F20A.7070009@codemonkey.ws> <4D539800.3070802@codemonkey.ws> <20110210090748.GD673@redhat.com> <4D53BD22.1040800@redhat.com> <20110210111354.GA21681@redhat.com> <4D53DF42.4030700@codemonkey.ws> <20110210132730.GB24525@redhat.com> <4D53F06C.9090500@codemonkey.ws> <20110210142044.GD24525@redhat.com> <4D540CC5.2@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Chris Wright , Gleb Natapov , kvm@vger.kernel.org, qemu-devel@nongnu.org, Markus Armbruster , Avi Kivity To: Blue Swirl Return-path: Received: from mail-yx0-f174.google.com ([209.85.213.174]:62685 "EHLO mail-yx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750932Ab1BMPcA (ORCPT ); Sun, 13 Feb 2011 10:32:00 -0500 Received: by yxt3 with SMTP id 3so1722514yxt.19 for ; Sun, 13 Feb 2011 07:31:59 -0800 (PST) In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 02/11/2011 12:14 PM, Blue Swirl wrote: > On Thu, Feb 10, 2011 at 6:05 PM, Anthony Liguori wrote: > >> On 02/10/2011 03:20 PM, Gleb Natapov wrote: >> >>> Jugging by how well all previous conversion went we will end up with one >>> more way of creating devices. One legacy, another qdev and your new one. >>> And what is the problem with qdev again (not that I am a big qdev fan)? >>> >>> >> We've really been arguing about probably the most minor aspect of the >> problem with qdev. >> >> All I'm really saying is that we shouldn't tie device construction to a >> factory interface as we do with qdev. >> >> That simply means that we should be able to do: >> >> RTC *rtc_create(arg1, arg2, arg2); >> > I don't see how that would help at all. Throwing qdev away and just > calling various functions directly, with all states exposed would be > like QEMU 0.9.0. > qdev doesn't expose any state today. qdev properties are construction-only properties that happen to be stored in each device state. What we really need is a full property framework that includes properties with hookable getters and setters along with the ability to mark properties as construct-only, read-only, or read-write. But I think it's reasonable to expose construct-only properties as just an initfn argument. >> And that a separate piece of code decides which devices are exposed through >> -device or device_add. Which devices are exposed is really a minor detail. >> >> That said, qdev has a number of significant limitations in my mind. The >> first is that the only relationship between devices is through the BusState >> interface. >> > There's also qemu_irq for arbitrary signals. > Yes, but qemu_irq is very restricted as it only models a signal bit of information and doesn't really have a mechanism to attach/detach in any generic way. >> I don't think we should even try to have a generic bus model. >> When you look at how badly broken PCI hotplug is current in qdev, I think >> this is symptomatic of this. >> > And how should this be fixed? The API change would not help. > Just as we have bus level creation functions, we should have bus level hotplug interfaces. >> There's also no way in qdev to really have polymorphism. Interfaces really >> aren't meaningful in qdev so you have things like PCIDevice where some >> methods are stored in the object instead of the class dispatch table and you >> have overuse of static class members. >> > QEMU is developed in C, not C++. > But we're trying to do object oriented programming in C so as long as we're doing that, we ought to do it right. >> And it's all unrelated to VMState. >> > Right, but this has also the good side that not all device state is > automatically exported. If other devices would be allowed to muck with > a devices internal state freely, bad things could happen. > > Device reset could also use standard register definitions, shared with VMState. > There's a way to have formally verifiable serialization/deserialization if we can satisfy two conditions 1) the devices rely on no global state (i.e. static variables) and 2) every field asssociated with a device is marshalled during serialization/deserialization. When we define a device, right now we say that certain state is writable during construction. It's not a stretch to want to have some properties writable during runtime. If we also had a mechanism to mark certain properties as read-only, but still were able to introspect them, we could implement serialization without having to have a second VMState definition. Compatibility will always require manipulating state, but once you have the state stored in a data structure, you can describe those transformations in a pretty high level fashion. >> And this is just the basic mechanisms of qdev. The actual implementation is >> worse. The use of qemu_irq as gpio in the base class and overuse of >> SystemBus is really quite insane. >> > Maybe qemu_irq should be renamed to QEMUSignal (and I don't like > typedeffing pointers), otherwise it looks quite sane to me. > Any interfaces of a base class should make sense even for derived classes. That means if the base class is going to expose essentially a pin-out interface, that if I have a PCIDevice and cast it to Device, I should be able to interact with the GPIO interface to interact with the PCI device. Presumably, that means interfacing at the PCI signalling level. That's insane to model in QEMU :-) In reality, GPIO only makes sense for a small class of simple devices where modelling the pin-out interface makes sense (like a 7-segment LCD). That suggests that GPIO should not be in the DeviceState interface but instead should be in a SimpleDevice subclass or something like that. > Could you point to examples of SystemBus overuse? > anthony@titi:~/git/qemu/hw$ grep qdev_create *.c | wc -l 73 anthony@titi:~/git/qemu/hw$ grep 'qdev_create(NULL' *.c | wc -l 56 SystemBus has become a catch-all for shallow qdev conversions. We've got Northbridges, RAM, and network devices sitting on the same bus... >> I don't think there is any device that has been improved by qdev. >> -device >> is a nice feature, but it could have been implemented without qdev. >> > We have 'info qtree' which can't be implemented easily without a > generic device class. Avi (or who was it) sent patches to expose even > more device state. > > With the patches I'm going to apply, if Redhat wants to disable > building various devices, it can be done without #ifdeffery. This is > not possible without a generic factory interface. > I'm not arguing against a generic factory interface, I'm arguing that it should be separate. IOW: SerialState *serial_create(int iobase, int irq, ...); static DeviceState *qdev_serial_create(QemuOpts *opts); static void serial_init(void) { qdev_register("serial", qdev_serial_create); } The key point is that when we create devices internally, we should have a C-friendly, type-safe interface to interact with. This will encourage composition and a richer device model than what we have today. Regards, Anthony Liguori