[Top][All Lists]

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

Re: [Qemu-devel] [PATCH 6/8] qom: introduce get/set methods for Property

From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH 6/8] qom: introduce get/set methods for Property
Date: Fri, 16 Dec 2011 14:51:14 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1

On 12/16/2011 02:11 PM, Gerd Hoffmann wrote:
I think we should not touch these.  First it doesn't buy us much as we
are using the old parse/print functions for the visitor-based access,
which doesn't look like a good idea to me.  Also they will not stay but
will be converted to child<>  and link<>, so I think it is better to keep
the old stuff in the legacy<>  namespace.

I thought the same initially. However, I noticed that the visitor interfaces for links is also a string. So, even if a block/char/netdev property later becomes a link<>, the interface would not change.

Using the old parse/print functions and get_set/generic is only to avoid code duplication, I can surely inline everything but it would be uglier. And again, I found an example in the code of using a similar adapter pattern (the string properties).

There is one case where I had doubts, namely the PCI address properties. They will be replaced by links that you set in the parent. However, in the end I decided to start this way because:

1) QOM properties can still come and go at this stage;

2) The PCI address property can still stay forever as a synthetic property declared by PCIDevice, so the "qom-get" ABI won't change. The "qom-set" ABI will, so it might be better to do:

 PropertyInfo qdev_prop_pci_devfn = {
     .name  = "pci-devfn",
     .type  = PROP_TYPE_UINT32,
     .size  = sizeof(uint32_t),
     .parse = parse_pci_devfn,
     .print = print_pci_devfn,
+    .get   = get_pci_devfn,
+    .set   = NULL,

Advantages: it shows that setting the PCI address is (going to be) a legacy feature;

Disadvantages: looks a little ad-hoc.  See below for an alternative.

Agree on the bit/bool/int types.  Although we maybe should apply some
care to integer bus properties, some of them are used for addressing and
will most likely replaced by child<>  and link<>  too.

Yes, these will also become synthetic and read-only. So an alternative could be:

   for (prop = dev->info->props; prop && prop->name; prop++) {
       qdev_property_add_legacy(dev, prop, NULL);

       /* Let the generic initializer register alternative definitions
        * for qdev properties.
       if (!qdev_property_find(dev, prop->name) {
           qdev_property_add_static(dev, prop, NULL);

   for (prop = dev->info->bus_info->props; prop && prop->name; prop++) {
       qdev_property_add_legacy(dev, prop, NULL);
       if (!qdev_property_find(dev, prop->name) {
           qdev_property_add_static(dev, prop, NULL);

For now the pci_devfn property remains read-write, but as soon as the PCIDevice will be able to define it as synthetic, it will become read-only.


reply via email to

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