On 21.04.23 05:22, Zhang, Chen wrote:
-----Original Message-----
From: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Sent: Thursday, April 20, 2023 7:26 PM
To: Zhang, Chen <chen.zh...@intel.com>; qemu-devel@nongnu.org
Cc: qemu-bl...@nongnu.org; michael.r...@amd.com; arm...@redhat.com;
ebl...@redhat.com; jasow...@redhat.com; quint...@redhat.com; Zhang,
Hailiang <zhanghaili...@xfusion.com>; phi...@linaro.org;
th...@redhat.com; berra...@redhat.com; marcandre.lur...@redhat.com;
pbonz...@redhat.com; d...@treblig.org; hre...@redhat.com;
kw...@redhat.com; lizhij...@fujitsu.com
Subject: Re: [PATCH v2 4/4] configure: add --disable-colo-filters option
On 20.04.23 12:09, Zhang, Chen wrote:
-----Original Message-----
From: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Sent: Thursday, April 20, 2023 6:53 AM
To: qemu-devel@nongnu.org
Cc: qemu-bl...@nongnu.org; michael.r...@amd.com;
arm...@redhat.com;
ebl...@redhat.com; jasow...@redhat.com; quint...@redhat.com;
Zhang,
Hailiang <zhanghaili...@xfusion.com>; phi...@linaro.org;
th...@redhat.com; berra...@redhat.com;
marcandre.lur...@redhat.com;
pbonz...@redhat.com; d...@treblig.org; hre...@redhat.com;
kw...@redhat.com; Zhang, Chen <chen.zh...@intel.com>;
lizhij...@fujitsu.com; Vladimir Sementsov-Ogievskiy
<vsementsov@yandex- team.ru>
Subject: [PATCH v2 4/4] configure: add --disable-colo-filters option
Add option to not build COLO Proxy subsystem if it is not needed.
I think no need to add the --disable-colo-filter option.
Net-filters just general infrastructure. Another example is COLO also
use the -chardev socket to connect each filters. No need to add the --
disable-colo-chardev....
Please drop this patch.
I don't follow your reasoning. Of course, we don't need any option like this,
and can simply include to build all the components we don't use. So "no
need" is correct for any --disable-* option.
Still, I think, it's good, when you can exclude from build the subsystems that
you don't need / don't want to maintain or ship to your end users.
Yes, I agree with your idea.
In MAINTAINERS these two filters are in "COLO Proxy" susbsystem. Is it
correct? What's wrong with adding an option to not build this subsystem? I
can rename it to --disable-colo-proxy.
The history is COLO project contributed QEMU filter framework and
filter-mirror/redirector...etc..
And the "COLO Proxy" susbsystem covered the filters do not means it belong to
COLO.
So, It is unreasonable to tell users that you cannot use
filter-mirror/rediretor for network debugging
Or other purpose because you have not enabled COLO proxy.
But we don't say it to users, as these filters are enabled by default.
But I see your point. And looking at filter-mirror.c I see that there is no
relations with colo. Can't say this about filter-rewriter.c
So, absolutely correct would be just have separate options
--disable-net-filter-mirror
--disable-net-filter-rewriter
and for any other filter we want to be "disable-able", like options for block
drivers (I mean --disable-parallels, --disable-qcow1, --disable-qed, etc for files
describing these drivers in block/)
But for COLO network part, still something need to do:
You can add --disable-colo-proxy not to build the net/colo-compare.c if it is
not needed.
This file just for COLO and not belong to network filters.
net/colo-compare.c is used only only for COLO, which in turn used only with
CONFIG_REPLICATION enabled, see patch 3. So, no reason to add separate
option for it, it should be disabled with --disable-replication.
Yes, and as Lukas said, COLO is the only user for the filter-rewriter currently.
So, maybe simply move filter-rewriter.c under CONFIG_REPLICATION, if it's
needed only for COLO?
It is more appropriate to add filter-rewriter replace the filter-mirror here.
I saw the patch 3, it is better to move it to this patch.
Hmm what do you mean? Both filter-rewriter and filter-mirror are now handled in
this patch, so what to replace?
Thanks
Chen
--
Best regards,
Vladimir
--
Best regards,
Vladimir
