Re: [PATCH 09/10] spice: Put spice functions in a separate load module

[Gerd Hoffmann]

  Hi,

> > If so the more normal approach would be to have a struct defining
> > a set of callbacks, that can be registered. Or if there's a natural
> > fit with QOM, then a QOM interface that can then have a QOM object
> > impl registered as a singleton.
> 
> That was my second attempt (after the weak symbols). I cleaned it up a bit
> and put it here: https://github.com/c3d/qemu/commits/spice-vtable.

I think this is the direction we should take.

> What made me switch to the approach in this series is the following
> considerations:
> 
> - A vtable is useful if there can be multiple values for a method, e.g. to
>   implement inheritance, or if you have multiple instances. This is not the
>   case here.

Well, we'll have two.  The normal functions.  And the stubs.

The stubs are inline functions right now, in include/ui/qemu-spice.h, in
the !CONFIG_SPICE section.  We can turn them into normal functions, move
them to some C file.  Let QemuSpiceOpts function pointers point to the
stubs initially.  When spice initializes (no matter whenever modular or
not) it'll set QemuSpiceOpts to the normal implementation.

That way we'll also remove some spice #ifdefs as part of the spice
modularization effort.

Things like the "using_spice" variable which don't depend on the spice
shared libraries can also be moved to the new C file with the spice
stubs.

I don't think we need to hide QemuSpiceOpts with inline functions like
qemu_spice_migrate_info().  I would simply use ...

        struct QemuSpiceOps {
                [ ... ]
                int (*migrate_info)(...);
                [ ... ]
        } qemu_spice;

... then change the ...

        qemu_spice_migrate_info(...)

.. callsites into ...

        qemu_spice.migrate_info(...)

> - Overloading QOM for that purpose looked more confusing than anything else.
>   It looked like I was mixing unrelated concepts. Maybe that's just me.

Hmm?  Not sure what you mean.  There is no need for QOM here (and I
can't see anything like that in your spice-vtable branch either).

> - The required change with a vtable ends up being more extensive. Instead of
>   changing a single line to put an entry point in a DSO, you need to create
>   the vtable, add functions to it, add a register function, etc. I was
>   looking for an easier and more scalable way.

IMHO it isn't too much overhead, and I find the code is more readable
that way.

> - In particular, with a vtable, you cannot take advantage of the syntactic
>   trick I used here, which is that foo(x) is a shortcut for (*foo)(x).
>   So for a vtable, you need to manually write wrappers.

See above, I don't think we need wrappers.

take care,
  Gerd