Re: [Qemu-devel] [RFC PATCH] configure: remove --enable-replication/--disable-replication

[Bruce Rogers]

>>> On 2/6/2017 at 4:57 AM, <address@hidden> wrote:
> * Paolo Bonzini (address@hidden) wrote:
>> 
>> 
>> On 03/02/2017 07:00, Stefan Hajnoczi wrote:
>> > On Thu, Feb 02, 2017 at 07:05:30AM ‑0800, Paolo Bonzini wrote:
>> >> The replication feature is a small amount of code, does not
>> >> require any external library and unless used does not add
>> >> anything to the guest's attack surface.  Since any extra
>> >> configure option affects maintainability on the other hand
>> >> and is subject to bit rot, I think there is no need to
>> >> make it configurable.
>> > 
>> > I think the current state is good: replication is enabled by default but
>> > can be compiled out if desired.
>> > 
>> > Downstreams may not be comfortable supporting this feature yet since
>> > it's incomplete.  It's fair to offer an option to disable it, otherwise
>> > downstreams will have to patch this themselves.
>> 
>> I understand‑‑‑I just am not sure where to draw the line because there's
>> plenty of other incomplete features, hence the RFC.  For example,
>> record/replay cannot be enabled or disabled on the configure command
>> line.  That was the case even in the beginning, where it didn't support
>> either block or character device replay.
> 
> The line is certainly fuzzy, but I think it's worth making the following
> type of things configurable:
>    Features that have a large chunk of code
>      ‑ dont lets try and configure tiny things on and off
>    That can be trivially configured
>      ‑ lets not put big chunks of code around making them configurable
> and   that are incomplete
>    or are unused by large chunks of the users
> 
> Dave
> 
>> ‑‑enable‑coroutine‑pool is a relic of when Windows builds needed it, but
>> all other ‑‑enable‑* options require an external library or at least a
>> specific operating system.  See for example this patch:
>> 
>>     commit 52b53c04faab9f7a9879c8dc014930649a3e698d
>>     Author: Fam Zheng <address@hidden>
>>     Date:   Wed Sep 10 14:17:51 2014 +0800
>> 
>>     block: Always compile virtio‑blk dataplane
>> 
>>     Dataplane doesn't depend on linux‑aio any more, so we don't need the
>>     compiling condition now.
>> 
>>     Configure options are kept but just print a message.
>> 
>>     Signed‑off‑by: Fam Zheng <address@hidden>
>>     Reviewed‑by: Paolo Bonzini <address@hidden>
>>     Message‑id: address@hidden 
>>     Signed‑off‑by: Stefan Hajnoczi <address@hidden>
>> 
>> 
>> I would actually prefer to remove many of the latter
>> (‑‑enable‑vhost‑net, ‑‑enable‑vhost‑scsi, ‑‑enable‑vhost‑socket) and
>> just use default‑configs.  We are already doing it for ivshmem for example:
>> 
>>     CONFIG_IVSHMEM=$(CONFIG_EVENTFD)
>> 
>> Paolo
> ‑‑
> Dr. David Alan Gilbert / address@hidden / Manchester, UK

Was there ever a conclusion here? The reason I ask is that I see that currently
using --disable-replication fails for me as follows:

# ./configure --disable-replication
...
# make
...
make  all-recursive
Making all in pixman
make[3]: Nothing to be done for 'all'.
Making all in demos
make[3]: Nothing to be done for 'all'.
Making all in test
make[3]: Nothing to be done for 'all'.
        CHK version_gen.h
  LINK    aarch64-softmmu/qemu-system-aarch64
../migration/colo.o: In function `qmp_query_xen_replication_status':
/home/brogers/osr/git/qemu/migration/colo.c:181: undefined reference to 
`replication_get_error_all'
../migration/colo.o: In function `qmp_xen_set_replication':
/home/brogers/osr/git/qemu/migration/colo.c:172: undefined reference to 
`replication_stop_all'
/home/brogers/osr/git/qemu/migration/colo.c:172: undefined reference to 
`replication_stop_all'
/home/brogers/osr/git/qemu/migration/colo.c:167: undefined reference to 
`replication_start_all'
../migration/colo.o: In function `qmp_xen_colo_do_checkpoint':
/ho
me/brogers/osr/git/qemu/migration/colo.c:196: undefined reference to 
`replication_do_checkpoint_all'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:208: qemu-system-aarch64] Error 1
make: *** [Makefile:322: subdir-aarch64-softmmu] Error 2

-- 
Bruce