[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.
Paolo
[Qemu-devel] [PATCH 2/8] qapi: protect against NULL QObject in qmp_input_get_object, Paolo Bonzini, 2011/12/16
[Qemu-devel] [PATCH 4/8] qom: push permission checks up into qdev_property_add_legacy, Paolo Bonzini, 2011/12/16
[Qemu-devel] [PATCH 6/8] qom: introduce get/set methods for Property, Paolo Bonzini, 2011/12/16
[Qemu-devel] [PATCH 8/8] qom: register qdev properties also as non-legacy properties, Paolo Bonzini, 2011/12/16
Re: [Qemu-devel] [PATCH 0/8] qom: introduce non-legacy static properties, Anthony Liguori, 2011/12/16