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

[Vladimir Sementsov-Ogievskiy]

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.

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.

> 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.

--
Best regards,
Vladimir