[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC] QEMU Object Model status/merge plan

From: Anthony Liguori
Subject: Re: [Qemu-devel] [RFC] QEMU Object Model status/merge plan
Date: Wed, 14 Dec 2011 07:46:34 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20110831 Lightning/1.0b2 Thunderbird/3.1.13

On 12/14/2011 04:33 AM, Kevin Wolf wrote:
Am 13.12.2011 17:02, schrieb Anthony Liguori:
static TypeInfo e1000_device_info = {
      .name = TYPE_E1000,
      .parent = TYPE_PCI_DEVICE,
      .instance_size = sizeof(E1000State),
      .class_init = pci_generic_class_init,
      .class_data =&e1000_device_ops,

I didn't really plan on this, but it looks like it would work pretty well.  It
might be reasonable to do for really common devices.  I don't think it's all
that nice to do for everything though because the *_generic_class_init()
functions get really ugly and makes allowing subclassing pretty hard.

It looks much nicer.

What kind of ugliness do you see in *_generic_class_init? Just that it's
a long chain of if (ops->field != NULL) k->field = ops->field;

Yeah. The other thing that gets a bit weird about it is a common trick in order to still be able to call the superclass version of the function is to squirrel away the super class pointer in a class member. Something like:

static void e1000_handle_event(PCIDevice *dev)
    E1000State *s = E1000_DEVICE(dev);
    E1000Class *k = E1000_DEVICE_GET_CLASS(s);

    // Call base class version of the function

    // Implement subclass behavior for function

static void e1000_class_init(ObjectClass *klass, void *data)
    PCIDeviceClass *pk = PCI_DEVICE_CLASS(klass);
    E1000Class *ek = E1000_CLASS(klass);

    ek->super_handle_event = pk->handle_event;
    pk->handle_event = e1000_handle_event;

There is an object_get_super() that returns the superclass that would let you do the same thing but the above is the idiomatic form in GObject at least.

This looks like something that could easily be generated in
the long term.

Also, what's the problem with subclassing? Doesn't this work:

static TypeInfo not_quite_e1000_device_info = {
       .name = TYPE_NOT_QUITE_E1000,
       .parent = TYPE_PCI_DEVICE,
       .instance_size = sizeof(NotQuiteE1000State),
       .class_init = e1000_generic_class_init,
       .class_data =&not_quite_e1000_device_ops,

static PCIDeviceOps not_quite_e1000_device_ops = {
      .bar = not_quite_e1000_bar,
      .parent_ops = {
          .foo = not_quite_e1000_foo,

void e1000_generic_class_init(ObjectClass *klass, void *data)
      E1000DeviceClass *k = E1000_DEVICE_CLASS(klass);
      E1000DeviceOps *ops = data;

      if (ops->bar) {
          k->bar = ops->bar;


It definitely works, it just ends up adding boiler plate. One of my goals was to make it as easy as possible to specify a device with a little code as possible.

I don't think there's much of a readability gain by using an ops structure. It's basically:

 .init = e1000_device_init,


  k->init = e1000_device_init;

The only advantage I see in using an ops structure is that it prevents you from doing anything other than overriding members. I'm not sure that that is a virtue.

(Btw, what's the point with klass vs. class? Do you expect that tools
which can deal with C++ will fail if we spell it class?)

Since I was doing so much code conversion, I tried to err on the side of being conservative so that we wouldn't have to touch everything again. So I avoided any C++ keywords as much as possible.

Perhaps one day we'll export a libqemu-block.so and even though it will be C, we'll want to make sure that a C++ program can include it.


This is the constructor for a type.  It is called when an object is created and
chained such that the base class constructors are called first to initialize the

Same for this one, in your serial code it looks like this doesn't do
anything interesting in the common case and could be made optional (it
adds an UART child device, but this is static property and should be
moved anyway)

It could potentially, yes.  instance_init functions will often times not be
needed.  It's really meant to do things like initialize lists and stuff like
that that property accessors may need to work with.

Right, this is what I thought.

I think even in the future the really interesting work will be done in

Yes.  The various TypeInfo methods can all be omitted if they don't do any work.

My concern is probably that currently they actually have to do some
work. Not much, just two or three lines of code, but it's enough to make
it impossible to omit them.

I think that QOM allows to do a lot of complicated things (at least you
have invested a lot of thought there, and even though I haven't seen the
patches yet, I'll assume that you got most of that right). The part that
I still see missing is that we should not only allow complicated things,
but also make the common thing simple.

Yeah, I think it's pretty simple right now. If you want to add a method to a class, you:

1) Add it to the <DEVICE>Class structure (adding the structure if necessary)
2) Set the default value of that method in the class_init function for that type

If you want to override a base class method, you:

1) Set a new value for the appropriate method in your type's class_init function

Contrast this with qdev, where to add a method, you:

1) Need to make sure a <BUS>DeviceInfo structure exists

2) Potentially update the bus registration function if a default version needs
to be implemented

There's no assumption of infrastructure in QOM. All types are self contained. You actually can't override a base class method in qdev unless it's explicitly supported by the bus registration function (and most of them don't allow it).

Maybe we can find ways to make common simple things simpler down the road, if so, I'm all for it.


Anthony Liguori


reply via email to

[Prev in Thread] Current Thread [Next in Thread]