>
> On 09/26/2014 11:14 PM, arei.gong...@huawei.com wrote:
> > From: Gonglei <arei.gong...@huawei.com>
> >
> > Add a new "description" field to DevicePropertyInfo.
> > The descriptions can serve as documentation in the code,
> > and they can be used to provide better help. For example:
> >
> > $./qemu-system-x86_64 -device virtio-blk-pci,?
> >
>
> > +++ b/qapi-schema.json
> > @@ -1615,11 +1615,12 @@
> > #
> > # @name: the name of the property
> > # @type: the typename of the property
> > +# @description: the description of the property
>
> Please add a '(since 2.2)' designation.
OK.
> Will 'description' always be
> present (that is, do ALL properties have a description), or is it
> optional and only provided when a non-NULL description is available?
>
> [/me reads rest of patch]
>
> Yes, given your code, it looks like you meant for it to be optional.
>
Yes.
> > #
> > # Since: 1.2
> > ##
> > { 'type': 'DevicePropertyInfo',
> > - 'data': { 'name': 'str', 'type': 'str' } }
> > + 'data': { 'name': 'str', 'type': 'str', 'description': 'str' } }
>
> Therefore, this needs to be '*description'...
>
OK.
> >
> > ##
> > # @device-list-properties:
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index 5ec6606..5ee6bf2 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -213,9 +213,14 @@ int qdev_device_help(QemuOpts *opts)
> > }
> >
> > for (prop = prop_list; prop; prop = prop->next) {
> > - error_printf("%s.%s=%s\n", driver,
> > + error_printf("%s.%s=%s", driver,
> > prop->value->name,
> > prop->value->type);
> > + if (prop->value->description) {
>
> ...and this needs to be 'prop->value->has_description'. We don't
> support NULL values of non-optional strings in QMP yet.
>
OK.
> > @@ -465,7 +466,9 @@ static DevicePropertyInfo
> *make_device_property_info(ObjectClass *klass,
> >
> > info = g_malloc0(sizeof(*info));
> > info->name = g_strdup(prop->name);
> > - info->type = g_strdup(prop->info->legacy_name ?:
> prop->info->name);
> > + info->type = g_strdup(prop->info->name);
> > + info->description = prop->info->description ?
> > + g_strdup(prop->info->description) :
> NULL;
>
> This needs to be:
>
> info->has_description = !!prop->info->description;
> info->description = g_strdup(prop->info->description);
>
> (it's safe to call g_strdup(NULL), avoiding the need for ?: in the
> assignment).
>
OK.
> > return info;
> > }
> > klass = object_class_get_parent(klass);
> > @@ -475,6 +478,8 @@ static DevicePropertyInfo
> *make_device_property_info(ObjectClass *klass,
> > info = g_malloc0(sizeof(*info));
> > info->name = g_strdup(name);
> > info->type = g_strdup(default_type);
> > + info->description = description ? g_strdup(description) : NULL;
>
> Another place that should avoid the ?:, and set has_description.
>
OK. Thanks a lot, Eric!
Will fix them in v4.
Best regards,
-Gonglei
> --
> Eric Blake eblake redhat com +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
