On Wed, 24 Feb 2021 12:36:17 +0100 Gerd Hoffmann <kra...@redhat.com> wrote:
> > static void virtio_ccw_gpu_register(void) > > { > > +#ifdef CONFIG_MODULES > > + type_register_static_mayfail(&virtio_ccw_gpu); > > +#else > > type_register_static(&virtio_ccw_gpu); > > +#endif > > Move the ifdef to type_register_static_mayfail, so this is not > duplicated for every module which might need this? I am concerned about a cluttered API. Having the documentation say: /** * type_register_static_mayfail: * @info: The #TypeInfo of the new type. * * @info and all of the strings it points to should exist for the life time * that the type is registered. * * If missing a parent type and if qom/object.c is built with CONFIG_MODULES * type_register_static_mayfail() differs from type_register_static only in not * printing an error and aborting but returning NULL. If qom/object.c is * built without CONFIG_MODULES type_register_static_mayfail() is same as * type_register_static() * Returns: the new #Type or NULL if missing a parent type. */ Type type_register_static_mayfail(const TypeInfo *info); does not feel right. Indeed modules seems to be the only circumstance under which a failed type registration does not imply a programming error. So I'm absolutely against shoving this logic down into object.c. But I find the variant I posted nicer to document and nicer to read: looking at virtio_ccw_gpu_register() one sees immediately that if built as a module, it is OK if the registration fails, and if built-in it is expected to work. > > > --- a/include/hw/s390x/css.h > > +++ b/include/hw/s390x/css.h > > Move this to a separate patch? > The "add type_register_mayfail" and "modularize virtio-gpu-ccw" changes > should be separate patches too. > > > -static TypeImpl *type_register_internal(const TypeInfo *info) > > +static TypeImpl *type_register_internal(const TypeInfo *info, bool mayfail) > > { > > TypeImpl *ti; > > ti = type_new(info); > > Hmm, type_register_internal seems to not look at the new mayfail flag. > Patch looks incomplete ... It definitely is. I messed up my smoke test (used the wrong executable) so I did not notice. > > take care, > Gerd > >