Re: [PATCH 00/18] qapi/qom: QAPIfy object-add

[Paolo Bonzini]

On 03/12/20 16:15, Kevin Wolf wrote:
> I don't think this is an intermediate state like Eduardo wants to have.
> Creating the object, then setting properties, then realize [1] will fail
> after your change. But keeping it working was the whole point of the
> exercise.

With the sample code, you must remove object_class_property_set calls at 
the same time as you remove the setters.  Usually that'd be when you 
convert to QAPI and oc->configure, but it doesn't have to be that way if 
there are good reasons not to do so.

Also, it still allows you to do so one class at a time, and I *think* 
the presence of subclasses or superclasses doesn't matter (only whether 
properties are still writable).  We can use chardevs (see ChardevCommon 
in qapi/char.json) to validate that before tackling devices.

(In fact, this means that your series---plus -object and object_add 
conversion---would be good, pretty much unchanged, as a first step.  The 
second would be adding oc->configure and object_configure, and 
converting all user-creatable objects to oc->configure.  The third would 
involve QAPI code generation).

> I'm also not really sure why you go from RngEgdOptions to QObject to a
> visitor, only to reconstruct RngEgdOptions at the end.

The two visits are just because you cannot create an input visitor 
directly on C data. I stole that from your patch 18/18 actually, just 
with object_new+object_configure instead of user_creatable_add_type.

But I wouldn't read too much in the automatically-generated *_new 
functions since they are already in QAPI code generator territory. 
Instead the basic object_configure idea can be applied even without 
having automatic code generation.

> I think the class
> implementations should have a normal C interface without visitors and we
> should be able to just pass the existing RngEgdOptions object (or the
> individual values for its fields for 'boxed': false).

Sure, however that requires changes to the QAPI code generator which was 
only item (3) in your list list.  Until then you can already work with a 
visitor interface:

  void rng_egd_configure(Object *obj, Visitor *v, Error **errp)
  {
      RngEgd *s = RNG_EGD(obj);
      s->config = g_new0(MemoryBackendOptions, 1);
      visit_type_MemoryBackendOptions(v, NULL, &s->config, errp);

      s->config->share = (s->config->has_share
                          ? s->config->share : false);
      ...
  }

but if you had a QAPI description

  { 'object': 'RngEgd',
    'qom-type': 'rng-egd',
    'configuration': 'RngEgdOptions',
    'boxed': true
  }

the QAPI generator could produce the oc->configure implementation. 
Similar to commands, that implementation would be an unmarshaling 
wrapper that calls out to the natural C interface:

  void qapi_RngEgd_configure(Object *obj, Visitor *v, Error **errp);
  {
      Error *local_err = NULL;
      g_autoptr(MemoryBackendOptions) *config =
          g_new0(MemoryBackendOptions, 1);
      visit_type_MemoryBackendOptions(v, NULL, &s->config, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
          return;
      }
      qom_rng_egd_configure(RNG_EGD(obj), config, errp);
  }

  void qom_rng_egd_configure(RngEng *s,
                             RngEgdOptions *config,
                             Error **errp)
  {
      config->share = (config->has_share
                       ? config->share : false);
      ...
      s->config = QAPI_CLONE(RngEgdOptions, config);
  }

Paolo