>>> On 2/6/2017 at 4:57 AM, <dgilb...@redhat.com> wrote:
> * Paolo Bonzini (pbonz...@redhat.com) 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 <f...@redhat.com>
>> 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 <f...@redhat.com>
>> Reviewed‑by: Paolo Bonzini <pbonz...@redhat.com>
>> Message‑id: 1410329871‑28885‑4‑git‑send‑email‑f...@redhat.com
>> Signed‑off‑by: Stefan Hajnoczi <stefa...@redhat.com>
>>
>>
>> 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 / dgilb...@redhat.com / 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
