RE: [PATCH v2 4/4] configure: add --disable-colo-filters option

[Zhang, Chen]

> -----Original Message-----
> From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Sent: Thursday, April 20, 2023 7:26 PM
> To: Zhang, Chen <chen.zhang@intel.com>; qemu-devel@nongnu.org
> Cc: qemu-block@nongnu.org; michael.roth@amd.com; armbru@redhat.com;
> eblake@redhat.com; jasowang@redhat.com; quintela@redhat.com; Zhang,
> Hailiang <zhanghailiang@xfusion.com>; philmd@linaro.org;
> thuth@redhat.com; berrange@redhat.com; marcandre.lureau@redhat.com;
> pbonzini@redhat.com; dave@treblig.org; hreitz@redhat.com;
> kwolf@redhat.com; lizhijian@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 <vsementsov@yandex-team.ru>
> >> Sent: Thursday, April 20, 2023 6:53 AM
> >> To: qemu-devel@nongnu.org
> >> Cc: qemu-block@nongnu.org; michael.roth@amd.com;
> armbru@redhat.com;
> >> eblake@redhat.com; jasowang@redhat.com; quintela@redhat.com;
> Zhang,
> >> Hailiang <zhanghailiang@xfusion.com>; philmd@linaro.org;
> >> thuth@redhat.com; berrange@redhat.com;
> marcandre.lureau@redhat.com;
> >> pbonzini@redhat.com; dave@treblig.org; hreitz@redhat.com;
> >> kwolf@redhat.com; Zhang, Chen <chen.zhang@intel.com>;
> >> lizhijian@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 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.
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.

Thanks
Chen

> 
> --
> Best regards,
> Vladimir