qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Zhang Chen
Subject: Re: [Qemu-devel] [RFC PATCH] configure: remove --enable-replication/--disable-replication
Date: Tue, 7 Mar 2017 11:50:02 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0



On 03/06/2017 08:54 PM, Paolo Bonzini wrote:

On 06/03/2017 11:03, Zhang Chen wrote:

On 03/06/2017 05:08 PM, Dr. David Alan Gilbert wrote:
* Bruce Rogers (address@hidden) wrote:
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':
/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.

OK, I'm happy to hear that.

Thanks
Zhang Chen


Paolo


.


--
Thanks
Zhang Chen






reply via email to

[Prev in Thread] Current Thread [Next in Thread]