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

Anthony Liguori
2011-02-13T15:32:00Z

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