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
