On Wed, 3 Mar 2021 08:07:50 +0100 Gerd Hoffmann <kra...@redhat.com> wrote:
> Hi, > > > The only approaches I can think of to make type_register_mayfail() > > "work" involve adding a dependency check in type_register_internal() > > before the call to type_table_add() is made. This can "work" for modules, > > because for types loaded from we can hope, that all dependencies are > > already, as modules are loaded relatively late. > > Yes, for modules the lazy binding should not be needed and we should be > able to check for the parent at registration time. module.c keeps track > of whenever phase1 init for builtin qom objects did happen already, so > we can use that instead of passing mayfail through a bunch of function > calls. Quick & dirty test hack below. Hi Gerd! Thank you very much for your patience. I've sent out a v3 yesterday, which takes a similar, yet slightly different approach. I will comment on the proposed diff below. Could you please have a look at my v3? I would prefer if we can go forward with that solution, but I am more than willing to prepare a v4 based on this proposal. > > BTW: "qemu-system-x86_64 -device help" tries to load all modules and is > a nice test case ;) Yes, I've reported the problem in Message-ID: <20210219035206.730f145e.pa...@linux.ibm.com> using that, for the trace I took -device virtio-gpu-ccw because the trace looked nicer to me. ;) > > HTH, > Gerd > > commit 75ca3012e626318b562bcb51ecdc34400e25f2d0 > Author: Gerd Hoffmann <kra...@redhat.com> > Date: Tue Mar 2 16:26:39 2021 +0100 > > [hack] modular type init check > > diff --git a/qom/object.c b/qom/object.c > index 491823db4a2d..01785e73f495 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -135,11 +135,22 @@ static TypeImpl *type_new(const TypeInfo *info) > return ti; > } > > +/* HACK: util/module.c */ > +extern bool modules_init_done[MODULE_INIT_MAX]; > +static TypeImpl *type_get_by_name(const char *name); > + > static TypeImpl *type_register_internal(const TypeInfo *info) > { > TypeImpl *ti; > ti = type_new(info); > > + if (modules_init_done[MODULE_INIT_QOM]) { > + if (ti->parent && !type_get_by_name(ti->parent)) { > + g_free(ti); The function type_new() has some g_strdup() action. We would need a type_delete() in order to clean those up properly if we are taking this approach. In v3 I decided to check the info, and avoid instantiating a ti so I don't have to clean it up. > + return NULL; > + } > + } > + This would change the postcondition of the type_register*() familiy of functions. It effectively makes type_register_*() a 'mayfail' depending on a global state, which is modules_init_done[MODULE_INIT_QOM]. It affects all modules. IMHO we should also update the documentation if we decide to move forward with this approach. I will give this a spin later. Please have a look at my v3 and let us decide how to move forward based on that. Regards, Halil [..]