Re: [Qemu-devel] KVM call minutes for Feb 8

[Anthony Liguori]

On 02/11/2011 12:14 PM, Blue Swirl wrote:
> On Thu, Feb 10, 2011 at 6:05 PM, Anthony Liguori<address@hidden>  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?
>    

address@hidden:~/git/qemu/hw$ grep qdev_create *.c | wc -l
73
address@hidden:~/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