On 06/03/2017 11:03, Zhang Chen wrote: > > > On 03/06/2017 05:08 PM, Dr. David Alan Gilbert wrote: >> * Bruce Rogers (brog...@suse.com) wrote: >>>>>> 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': >>> /home/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 >> We should fix that. > > COLO needs replication enable. > So, should I add a new option enable/disable COLO ? > Then, If you disable replication, colo will be disabled automatically. > Like that: > # ./configure --disable-colo
Then I would just define --enable-colo/--disable-colo which includes replication, the network filter and everything else. Paolo
