| Message ID | 20230419225232.508121-5-vsementsov@yandex-team.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | COLO: improve build options | expand |
> -----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. 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. Thanks Chen > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > --- > meson.build | 1 + > meson_options.txt | 2 ++ > net/meson.build | 11 ++++++++--- > scripts/meson-buildoptions.sh | 3 +++ > 4 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/meson.build b/meson.build > index c44d05a13f..5b2fdfbd3a 100644 > --- a/meson.build > +++ b/meson.build > @@ -1962,6 +1962,7 @@ config_host_data.set('CONFIG_GPROF', > get_option('gprof')) > config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION', > get_option('live_block_migration').allowed()) > config_host_data.set('CONFIG_QOM_CAST_DEBUG', > get_option('qom_cast_debug')) > config_host_data.set('CONFIG_REPLICATION', > get_option('replication').allowed()) > +config_host_data.set('CONFIG_COLO_FILTERS', > +get_option('colo_filters').allowed()) > > # has_header > config_host_data.set('CONFIG_EPOLL', cc.has_header('sys/epoll.h')) diff --git > a/meson_options.txt b/meson_options.txt index fc9447d267..ffe81317cb > 100644 > --- a/meson_options.txt > +++ b/meson_options.txt > @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: > 'auto', > description: 'block migration in the main migration stream') > option('replication', type: 'feature', value: 'auto', > description: 'replication support') > +option('colo_filters', type: 'feature', value: 'auto', > + description: 'colo_filters support') > option('bochs', type: 'feature', value: 'auto', > description: 'bochs image format support') option('cloop', type: 'feature', > value: 'auto', diff --git a/net/meson.build b/net/meson.build index > 38d50b8c96..7e54744aea 100644 > --- a/net/meson.build > +++ b/net/meson.build > @@ -1,12 +1,9 @@ > softmmu_ss.add(files( > 'announce.c', > 'checksum.c', > - 'colo.c', > 'dump.c', > 'eth.c', > 'filter-buffer.c', > - 'filter-mirror.c', > - 'filter-rewriter.c', > 'filter.c', > 'hub.c', > 'net-hmp-cmds.c', > @@ -22,6 +19,14 @@ if get_option('replication').allowed() > softmmu_ss.add(files('colo-compare.c')) > endif > > +if get_option('replication').allowed() or > +get_option('colo_filters').allowed() > + softmmu_ss.add(files('colo.c')) > +endif > + > +if get_option('colo_filters').allowed() > + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c')) endif > + > softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c')) > > if have_l2tpv3 > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh > index 009fab1515..cf9d23369f 100644 > --- a/scripts/meson-buildoptions.sh > +++ b/scripts/meson-buildoptions.sh > @@ -83,6 +83,7 @@ meson_options_help() { > printf "%s\n" ' capstone Whether and how to find the capstone library' > printf "%s\n" ' cloop cloop image format support' > printf "%s\n" ' cocoa Cocoa user interface (macOS only)' > + printf "%s\n" ' colo-filters colo_filters support' > printf "%s\n" ' coreaudio CoreAudio sound support' > printf "%s\n" ' crypto-afalg Linux AF_ALG crypto backend driver' > printf "%s\n" ' curl CURL block device driver' > @@ -236,6 +237,8 @@ _meson_option_parse() { > --disable-cloop) printf "%s" -Dcloop=disabled ;; > --enable-cocoa) printf "%s" -Dcocoa=enabled ;; > --disable-cocoa) printf "%s" -Dcocoa=disabled ;; > + --enable-colo-filters) printf "%s" -Dcolo_filters=enabled ;; > + --disable-colo-filters) printf "%s" -Dcolo_filters=disabled ;; > --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;; > --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;; > --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;; > -- > 2.34.1
On Thu, 20 Apr 2023 09:09:48 +0000 "Zhang, Chen" <chen.zhang@intel.com> 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. > 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. And net/filter-rewriter.c is just for COLO too. So in summary just drop net/filter-mirror.c from this patch? > > Thanks > Chen > > > > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > > --- > > meson.build | 1 + > > meson_options.txt | 2 ++ > > net/meson.build | 11 ++++++++--- > > scripts/meson-buildoptions.sh | 3 +++ > > 4 files changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/meson.build b/meson.build > > index c44d05a13f..5b2fdfbd3a 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -1962,6 +1962,7 @@ config_host_data.set('CONFIG_GPROF', > > get_option('gprof')) > > config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION', > > get_option('live_block_migration').allowed()) > > config_host_data.set('CONFIG_QOM_CAST_DEBUG', > > get_option('qom_cast_debug')) > > config_host_data.set('CONFIG_REPLICATION', > > get_option('replication').allowed()) > > +config_host_data.set('CONFIG_COLO_FILTERS', > > +get_option('colo_filters').allowed()) > > > > # has_header > > config_host_data.set('CONFIG_EPOLL', cc.has_header('sys/epoll.h')) diff --git > > a/meson_options.txt b/meson_options.txt index fc9447d267..ffe81317cb > > 100644 > > --- a/meson_options.txt > > +++ b/meson_options.txt > > @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: > > 'auto', > > description: 'block migration in the main migration stream') > > option('replication', type: 'feature', value: 'auto', > > description: 'replication support') > > +option('colo_filters', type: 'feature', value: 'auto', > > + description: 'colo_filters support') > > option('bochs', type: 'feature', value: 'auto', > > description: 'bochs image format support') option('cloop', type: 'feature', > > value: 'auto', diff --git a/net/meson.build b/net/meson.build index > > 38d50b8c96..7e54744aea 100644 > > --- a/net/meson.build > > +++ b/net/meson.build > > @@ -1,12 +1,9 @@ > > softmmu_ss.add(files( > > 'announce.c', > > 'checksum.c', > > - 'colo.c', > > 'dump.c', > > 'eth.c', > > 'filter-buffer.c', > > - 'filter-mirror.c', > > - 'filter-rewriter.c', > > 'filter.c', > > 'hub.c', > > 'net-hmp-cmds.c', > > @@ -22,6 +19,14 @@ if get_option('replication').allowed() > > softmmu_ss.add(files('colo-compare.c')) > > endif > > > > +if get_option('replication').allowed() or > > +get_option('colo_filters').allowed() > > + softmmu_ss.add(files('colo.c')) > > +endif > > + > > +if get_option('colo_filters').allowed() > > + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c')) endif > > + > > softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c')) > > > > if have_l2tpv3 > > diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh > > index 009fab1515..cf9d23369f 100644 > > --- a/scripts/meson-buildoptions.sh > > +++ b/scripts/meson-buildoptions.sh > > @@ -83,6 +83,7 @@ meson_options_help() { > > printf "%s\n" ' capstone Whether and how to find the capstone library' > > printf "%s\n" ' cloop cloop image format support' > > printf "%s\n" ' cocoa Cocoa user interface (macOS only)' > > + printf "%s\n" ' colo-filters colo_filters support' > > printf "%s\n" ' coreaudio CoreAudio sound support' > > printf "%s\n" ' crypto-afalg Linux AF_ALG crypto backend driver' > > printf "%s\n" ' curl CURL block device driver' > > @@ -236,6 +237,8 @@ _meson_option_parse() { > > --disable-cloop) printf "%s" -Dcloop=disabled ;; > > --enable-cocoa) printf "%s" -Dcocoa=enabled ;; > > --disable-cocoa) printf "%s" -Dcocoa=disabled ;; > > + --enable-colo-filters) printf "%s" -Dcolo_filters=enabled ;; > > + --disable-colo-filters) printf "%s" -Dcolo_filters=disabled ;; > > --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;; > > --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;; > > --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;; > > -- > > 2.34.1 > > --
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.
> -----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
On 21.04.23 05:22, Zhang, Chen wrote: > > >> -----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 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 >
> -----Original Message----- > From: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > Sent: Friday, April 21, 2023 4:53 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 21.04.23 05:22, Zhang, Chen wrote: > > > > > >> -----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 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/) > Yes. > > > > >> > >>> 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? > As I know, in QEMU side, COLO is the only user for filter-rewriter. But QEMU user(libvirt...etc...) may try to use it for other proposal. > > 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? I means it's better to make the net/colo-compare.c and net/filter-rewriter.c to this patch for The option: --disable-colo-proxy Thanks Chen > > > > > Thanks > > Chen > > > >> > >> -- > >> Best regards, > >> Vladimir > > > > -- > Best regards, > Vladimir
diff --git a/meson.build b/meson.build index c44d05a13f..5b2fdfbd3a 100644 --- a/meson.build +++ b/meson.build @@ -1962,6 +1962,7 @@ config_host_data.set('CONFIG_GPROF', get_option('gprof')) config_host_data.set('CONFIG_LIVE_BLOCK_MIGRATION', get_option('live_block_migration').allowed()) config_host_data.set('CONFIG_QOM_CAST_DEBUG', get_option('qom_cast_debug')) config_host_data.set('CONFIG_REPLICATION', get_option('replication').allowed()) +config_host_data.set('CONFIG_COLO_FILTERS', get_option('colo_filters').allowed()) # has_header config_host_data.set('CONFIG_EPOLL', cc.has_header('sys/epoll.h')) diff --git a/meson_options.txt b/meson_options.txt index fc9447d267..ffe81317cb 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -289,6 +289,8 @@ option('live_block_migration', type: 'feature', value: 'auto', description: 'block migration in the main migration stream') option('replication', type: 'feature', value: 'auto', description: 'replication support') +option('colo_filters', type: 'feature', value: 'auto', + description: 'colo_filters support') option('bochs', type: 'feature', value: 'auto', description: 'bochs image format support') option('cloop', type: 'feature', value: 'auto', diff --git a/net/meson.build b/net/meson.build index 38d50b8c96..7e54744aea 100644 --- a/net/meson.build +++ b/net/meson.build @@ -1,12 +1,9 @@ softmmu_ss.add(files( 'announce.c', 'checksum.c', - 'colo.c', 'dump.c', 'eth.c', 'filter-buffer.c', - 'filter-mirror.c', - 'filter-rewriter.c', 'filter.c', 'hub.c', 'net-hmp-cmds.c', @@ -22,6 +19,14 @@ if get_option('replication').allowed() softmmu_ss.add(files('colo-compare.c')) endif +if get_option('replication').allowed() or get_option('colo_filters').allowed() + softmmu_ss.add(files('colo.c')) +endif + +if get_option('colo_filters').allowed() + softmmu_ss.add(files('filter-mirror.c', 'filter-rewriter.c')) +endif + softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c')) if have_l2tpv3 diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 009fab1515..cf9d23369f 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -83,6 +83,7 @@ meson_options_help() { printf "%s\n" ' capstone Whether and how to find the capstone library' printf "%s\n" ' cloop cloop image format support' printf "%s\n" ' cocoa Cocoa user interface (macOS only)' + printf "%s\n" ' colo-filters colo_filters support' printf "%s\n" ' coreaudio CoreAudio sound support' printf "%s\n" ' crypto-afalg Linux AF_ALG crypto backend driver' printf "%s\n" ' curl CURL block device driver' @@ -236,6 +237,8 @@ _meson_option_parse() { --disable-cloop) printf "%s" -Dcloop=disabled ;; --enable-cocoa) printf "%s" -Dcocoa=enabled ;; --disable-cocoa) printf "%s" -Dcocoa=disabled ;; + --enable-colo-filters) printf "%s" -Dcolo_filters=enabled ;; + --disable-colo-filters) printf "%s" -Dcolo_filters=disabled ;; --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;; --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;; --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;;
Add option to not build COLO Proxy subsystem if it is not needed. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> --- meson.build | 1 + meson_options.txt | 2 ++ net/meson.build | 11 ++++++++--- scripts/meson-buildoptions.sh | 3 +++ 4 files changed, 14 insertions(+), 3 deletions(-)