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