From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWMA7-0003LZ-0H for qemu-devel@nongnu.org; Tue, 23 Sep 2014 05:13:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XWMA1-0005EG-8X for qemu-devel@nongnu.org; Tue, 23 Sep 2014 05:13:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:45104) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XWMA1-0005B6-1T for qemu-devel@nongnu.org; Tue, 23 Sep 2014 05:13:25 -0400 Date: Tue, 23 Sep 2014 12:16:35 +0300 From: "Michael S. Tsirkin" Message-ID: <20140923091635.GA17562@redhat.com> References: <20140922083421.GA13006@redhat.com> <541FE889.3050209@redhat.com> <33183CC9F5247A488A2544077AF1902086DD90B3@SZXEMA503-MBS.china.huawei.com> <541FF3F9.5060705@redhat.com> <33183CC9F5247A488A2544077AF1902086DD915A@SZXEMA503-MBS.china.huawei.com> <542010CE.3010209@redhat.com> <20140922123419.GA7992@redhat.com> <54201C35.1090502@redhat.com> <33183CC9F5247A488A2544077AF1902086DD991C@SZXEMA503-MBS.china.huawei.com> <87zjdqvh8l.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87zjdqvh8l.fsf@blackfin.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH 0/3] Fix confused output for alias properties List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: "Huangweidong (C)" , "aliguori@amazon.com" , "qemu-devel@nongnu.org" , "agraf@suse.de" , "Huangpeng (Peter)" , "Gonglei (Arei)" , "stefanha@redhat.com" , Paolo Bonzini , "lcapitulino@redhat.com" On Tue, Sep 23, 2014 at 11:06:02AM +0200, Markus Armbruster wrote: > "Gonglei (Arei)" 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.