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

[Paolo Bonzini <pbonzini@redhat.com>]

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.

Paolo