Re: [Qemu-devel] 答复： Re: [PATCH v2] object: Add 'help' option for all available backends and properties

["Daniel P. Berrange" <berrange@redhat.com>]

On Thu, Sep 22, 2016 at 01:12:22PM +0200, Markus Armbruster wrote:
> "Daniel P. Berrange" <berrange@redhat.com> writes:
> 
> > On Thu, Sep 22, 2016 at 10:36:45AM +0200, Markus Armbruster wrote:
> >> Don't make up a description in user_creatable_help_func(), improve the
> >> description infrastructure and its use so you get more useful ones
> >> there.
> >> 
> >> The existing description infrastructure is just Property member
> >> description and object_property_set_description().  Rarely used, so
> >> description is generally null.
> >> 
> >> Calling object_property_set_description() more often could be helpful,
> >> but to come up with a sensible description string, you need to know what
> >> the property does.  Needs to be left to people actually familiar with
> >> the objects.
> >> 
> >> Aside: historically, we add properties to *instances*.  All the property
> >> meta-data gets duplicated for every instance, including property
> >> descriptions.  This is more flexible than adding the meta-data to the
> >> class.  The flexibility is rarely needed, but the price in wasted memory
> >> is always paid.  Only since commit 16bf7f5, we can add it to classes.
> >> Adding lots of helpful property descriptions would increase the cost of
> >> instance properties further.
> >
> > FWIW, we could easily optimize handling of description strings by
> > applying the same trick that GLib has done for GObject property
> > descriptions.
> >
> > Almost certainly every call to object_property_set_description is
> > going to be passing a string literal, not a dynamically allocated
> > string. So we take advantage of that and in fact mandate that it
> > is a string literal, and thus avoid the strdup() of description.
> >
> > We can place a fun game to enforce this at compile time thus:
> >
> >  - Rename object_property_set_description() to
> >    object_property_set_description_internal()
> >
> >  - Add the macro
> >
> >   #define  object_property_set_description(obj, name, desc, errp) \
> >      object_property_set_description_internal(obj, name, "" desc "", errp)
> 
> Cute :)
> 
> > None the less, we really should make an effort to switch things
> > over to use class properties instead of instance properties, as
> > its going to save us allocating a 64 byte struct per property
> > per instance
> 
> Yes, please.
> 
> Related: a way to define a bunch of properties as *data*, i.e. an array
> of property descriptions, commonly static.  Reasoning about static data
> is so much easier than reasoning about code.

IMHO we should go further and leverage QAPI schema to auto-generate all
the tedious boilerplate code for QOM objects

eg, consider the crypto/secret.c object file.

We could declare it as

{ 'object': 'QCryptoSecret',
  'parent': 'Object',
  'properties': {
     'format': 'QCryptoSecretFormat',
     'data': 'str',
     'file': 'str',
     'keyid': 'str',
     'iv': 'str'
     } }

Based on that it would have enough knowledge to generate

 - struct QCryptoSecret  definition + typedef
 - struct QCryptoSecretClass definition + typedef
 - TYPE_CRYPTO_SECRET macro
 - QCRYPT_SECRET() cast macro
 - Setters & getters aka
     qcrypto_secret_prop_set_format
     qcrypto_secret_prop_get_format
     qcrypto_secret_prop_set_data
     qcrypto_secret_prop_get_data
     qcrypto_secret_prop_set_file
     qcrypto_secret_prop_get_file
     qcrypto_secret_prop_set_keyid
     qcrypto_secret_prop_get_keyid
     qcrypto_secret_prop_set_iv
     qcrypto_secret_prop_get_iv
 - qcrypto_secret_finalize
 - qcrypto_secret_class_init
 - TypeInfo qcrypto_secret_info variable
 - qcrypto_secret_register_types() method
 - type_init(qcrypto_secret_register_types);

That'd massively reduce the work to create new objects, to just
filling in the semantically useful logic

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|