On Thu, Oct 27, 2016 at 05:13:34PM +0200, Igor Mammedov wrote:
> On Thu, 27 Oct 2016 09:25:27 -0200
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > On Thu, Oct 27, 2016 at 09:38:28AM +0300, Dmitry Fleytman wrote:
> > >
> > > > On 27 Oct 2016, at 06:12 AM, Jason Wang <jasow...@redhat.com> wrote:
> > > >
> > > >
> > > >
> > > > On 2016年10月27日 03:21, Eduardo Habkost wrote:
> > > >> Instead of hacking custom PropertyInfo structs, use the regular
> > > >> DEFINE_PROP_<type> macros for the e1000e properties.
> > > >>
> > > >> This also fixes a bug in the disable_vnet_hdr property
> > > >> definition, that was using qdev_prop_uint8 for a bool field.
> > > >>
> > > >> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > > >> ---
> > > >> hw/net/e1000e.c | 25 +++----------------------
> > > >> 1 file changed, 3 insertions(+), 22 deletions(-)
> > > >>
> > > >> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> > > >> index 4994e1c..df24e55 100644
> > > >> --- a/hw/net/e1000e.c
> > > >> +++ b/hw/net/e1000e.c
> > > >> @@ -638,19 +638,11 @@ static const VMStateDescription e1000e_vmstate =
> > > >> {
> > > >> }
> > > >> };
> > > >> -static PropertyInfo e1000e_prop_disable_vnet,
> > > >> - e1000e_prop_subsys_ven,
> > > >> - e1000e_prop_subsys;
> > > >> -
> > > >> static Property e1000e_properties[] = {
> > > >> DEFINE_NIC_PROPERTIES(E1000EState, conf),
> > > >> - DEFINE_PROP_DEFAULT("disable_vnet_hdr", E1000EState,
> > > >> disable_vnet, false,
> > > >> - e1000e_prop_disable_vnet, bool),
> > > >> - DEFINE_PROP_DEFAULT("subsys_ven", E1000EState, subsys_ven,
> > > >> - PCI_VENDOR_ID_INTEL,
> > > >> - e1000e_prop_subsys_ven, uint16_t),
> > > >> - DEFINE_PROP_DEFAULT("subsys", E1000EState, subsys, 0,
> > > >> - e1000e_prop_subsys, uint16_t),
> > > >> + DEFINE_PROP_BOOL("disable_vnet_hdr", E1000EState, disable_vnet,
> > > >> false),
> > > >> + DEFINE_PROP_UINT16("subsys_ven", E1000EState, subsys_ven,
> > > >> PCI_VENDOR_ID_INTEL),
> > > >> + DEFINE_PROP_UINT16("subsys", E1000EState, subsys, 0),
> > > >> DEFINE_PROP_END_OF_LIST(),
> > > >> };
> > > >> @@ -673,17 +665,6 @@ static void e1000e_class_init(ObjectClass
> > > >> *class, void *data)
> > > >> dc->vmsd = &e1000e_vmstate;
> > > >> dc->props = e1000e_properties;
> > > >> - e1000e_prop_disable_vnet = qdev_prop_uint8;
> > > >> - e1000e_prop_disable_vnet.description = "Do not use virtio
> > > >> headers, "
> > > >> - "perform SW offloads
> > > >> emulation "
> > > >> - "instead";
> > > >> -
> > > >> - e1000e_prop_subsys_ven = qdev_prop_uint16;
> > > >> - e1000e_prop_subsys_ven.description = "PCI device Subsystem Vendor
> > > >> ID";
> > > >> -
> > > >> - e1000e_prop_subsys = qdev_prop_uint16;
> > > >> - e1000e_prop_subsys.description = "PCI device Subsystem ID";
> > > >> -
> > > >> set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
> > > >> }
> > > >>
> > > >
> > > > Is there a way to keep the description here? At least for "vnet" I
> > > > believe.
> > >
> > > Actually, ability to add description was the only reason for having
> > > custom PropertyInfo structs.
> > > Does DEFINE_PROP_… macros set worth an improvement?
> >
> > I think it would be reasonable to add a descrition field to
> > struct Property.
> >
> > Alternatively, you can call object_property_set_description() on
> > instance_init.
> that would look sort of convoluted in case of class properties.
> description should be set at the same time the property is added.
In the case of class properties, we have
object_class_property_set_description().
It's easier to redo this series on top of the
qdev-class-properties series, to avoid conflicts. I will do that.
--
Eduardo
