Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties

[Michael S. Tsirkin]

On Tue, Sep 23, 2014 at 11:06:02AM +0200, Markus Armbruster wrote:
> "Gonglei (Arei)" <address@hidden> writes:
> 
> > Hi,
> >
> >> >>>> This doesn't change the fact that ObjectProperty is a generic struct,
> >> >>>> and adding alias-specific fields there is wrong.
> >> >>>
> >> >>> OK, Maybe I should find other ways to attach this purpose and
> >> >>> avoid layering violation. Thanks!
> >> >>
> >> >> Unfortunately I cannot think of any.
> >> >>
> >> >> We could add a description field to ObjectProperty, and replace
> >> >> legacy_name with a description.  The output then would be
> >> >>
> >> >> virtio-blk.drive=str (drive)
> >
> > There is a question that the QOM properties are added dynamically.
> > When we call qdev_alias_all_properties() adding alias properties to
> > the source object all qdev properties on the target DeviceState, how do we
> > judge the property's name and set the value of corresponding description 
> > field?
> > Such as setting virtio-blk-pci.drive.description = "drive".
> >
> > The only way I think of is using the property' name as the description. In 
> > object_property_add_alias() add below code:
> >
> > +   op->description = g_strdup(name);
> >
> > After those handling we can get results:
> >
> > virtio-blk-pci.physical_block_size=uint16 (physical_block_size)
> > virtio-blk-pci.logical_block_size=uint16 (logical_block_size)
> > virtio-blk-pci.drive=str (drive)
> >
> > virtio-net-pci.netdev=str (netdev)
> > virtio-net-pci.vlan=int (vlan)
> > virtio-net-pci.mac=str (mac)
> >
> > But if using this way, we just need simply modify 
> > make_device_property_info()
> > like below patch (just assure the new output resemble the old, don't need 
> > change
> > ObjectProperty struct).  Am I right? Thanks :)
> 
> Adding descriptions to properties is a big, but useful task.  The
> descriptions can serve as documentation in the code, and they can be
> used to provide better help.  This applies both to qdev and to object
> properties.
> 
> Completing this task in one go is unfortunately not practical.  We need
> to add descriptions incrementally.  Until we're done, some properties
> will lack descriptions (null pointer rather than a descriptive string).
> 
> Reusing the property name as description just to have a description adds
> no information.
> 
> What about this: add a description string (optional for now) both to
> ObjectProperty and to PropertyInfo.  Either add a description string
> parameter to object property constructors like object_property_add() and
> object_property_add_alias, or, for less churn, add a separate function
> to set an object property's description.  Update
> qdev_alias_all_properties() to set the object property's description to
> the qdev property's.
> 
> Then you can fix the help regression by giving all the regressed
> properties a useful description.
> 
> That fix should also permit retiring legacy_name.

Ack. Sounds good to me.