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