Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
Am 03.12.2020 um 17:50 hat Paolo Bonzini geschrieben:
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.
Okay, thanks, I think I understand now.
So I assume that in the common case, we'll never have the state that you
describe, but we'll want to directly skip to QAPI generated code. But
it's good to know that we can make smaller steps if we need to in more
complicated cases.
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.
Yes, it looks like it should be working.
(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 think I'd want to do step 2 and 3 combined, because converting
user-creatable objects to oc->configure means manually writing the
configure function that will be generated from QAPI in step 3. Writing
code just to throw it away isn't my favourite pastime.
> 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.
Yes, I was just wondering why we're going through visitors at all. But
this is what provides the compatibility with the old property system, so
it makes sense if you need an intermediate step.
> 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);
}
Yes, exactly.
Kevin