Re: [PATCH RESEND v6 08/36] multi-process: Add stub functions to facilitate build of multi-process

[Jag Raman]

> On Apr 24, 2020, at 9:12 AM, Stefan Hajnoczi <address@hidden> wrote:
> 
> On Wed, Apr 22, 2020 at 09:13:43PM -0700, address@hidden wrote:
>> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
>> index f884bb6180..f74c7e927b 100644
>> --- a/stubs/Makefile.objs
>> +++ b/stubs/Makefile.objs
>> @@ -20,6 +20,7 @@ stub-obj-y += migr-blocker.o
>> stub-obj-y += change-state-handler.o
>> stub-obj-y += monitor.o
>> stub-obj-y += monitor-core.o
>> +stub-obj-y += get-fd.o
>> stub-obj-y += notify-event.o
>> stub-obj-y += qtest.o
>> stub-obj-y += replay.o
> 
> audio.c, vl-stub.c, and xen-mapcache.c are added by this patch but not
> added to Makefile.objs?  Can they be removed?

Hey Stefan,

Sorry it’s not clear. but these files are referenced in Makefile.target.

> 
> This entire patch requires justification.  Stubs exist so that common
> code can be linked without optional features.
> 
> For example, common code may call into kvm but that callback isn't
> relevant when building with kvm accelerator support (e.g. say qemu-nbd).
> That's where the stub function comes in.  It fulfills the dependency
> without dragging in the actual kvm accelerator code.
> 
> Adding lots of stubs suggests you are building QEMU in a new way that
> wasn't done before (this is true and expected for this patch series).  I
> would like to understand the reason for these stubs though.  For
> example, why do you need to stub audio?

These stub functions are only used by the remote process, and not by
QEMU itself.

Our goal is to ensure that the remote process is building the smallest
set of files necessary and these stub functions are necessary to meet
that goal.

For example, the remote process needs to build some of the functions
defined in “hw/core/qdev-properties-system.c”. However, this file
depends on audio.c (references audio_state_by_name()), which is not
needed for the remote process. The alternative to stub functions would
be to compile audio.c into the remote process, but that was not necessary
in our judgement. When the project started out, we spent a lot of time
figuring out which functions/files are necessary for the remote process, and
we stubbed out the ones which are needed to resolve dependency during
compilation, but not needed for functionality.

audio.c is just an example of tens of other places where we needed to
make similar judgements.

Would you prefer if we moved these stub functions into a separate
library (instead of stub-obj-y) which is only linked by the remote process?

--
Jag

> 
> Without a reason for each of these stubs we have no way of knowing if
> they are actually used/needed.  Maybe an earlier version of the code
> needed it but the latest version of the patch no longer does...
> 
> Stefan