Re: [PATCH v4 04/10] configure: add --disable-colo-proxy option
Vladimir Sementsov-Ogievskiy wrote:
> On 10.05.23 15:18, Juan Quintela wrote:
>> Vladimir Sementsov-Ogievskiy wrote:
>>> On 09.05.23 21:42, Juan Quintela wrote:
"Zhang, Chen" wrote:
>> -Original Message-
>> From: Vladimir Sementsov-Ogievskiy
>> Sent: Saturday, April 29, 2023 3:49 AM
>> To: qemu-devel@nongnu.org
>> Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
>> ; vsement...@yandex-team.ru; Paolo Bonzini
>> ; Marc-André Lureau
>> ; Daniel P. Berrangé
>> ; Thomas Huth ; Philippe
>> Mathieu-Daudé ; Jason Wang
>> Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
>>
>> Add option to not build filter-mirror, filter-rewriter and colo-compare
>> when
>> they are not needed.
>
> Typo: This patch still build the filter-mirror/filter-redirector in
> filter-mirror.c.
> Please remove the "filter-mirror" here.
> Other code look good to me.
Vladimir, I was doing this myself, with the bit attached.
But then I noticed that one needs to also disable
tests/qtest/test-filter-mirror and test-filter-rewriter.
>>>
>>> Hmm, but we decided not touch filter-mirror in this patch, only
>>> filter-rewriter.
>>>
>>> And there is no tests/qtest/test-filter-rewriter test.
>>>
Can you resend with that fixed? Or I am missing something more
fundamental.
Thanks, Juan.
>> --- a/net/meson.build
>> +++ b/net/meson.build
>> @@ -1,13 +1,10 @@
>>softmmu_ss.add(files(
>> 'announce.c',
>> 'checksum.c',
>> - 'colo-compare.c',
>> - 'colo.c',
>> 'dump.c',
>> 'eth.c',
>> 'filter-buffer.c',
>> 'filter-mirror.c',
>> - 'filter-rewriter.c',
>> 'filter.c',
>> 'hub.c',
>> 'net-hmp-cmds.c',
>> @@ -19,6 +16,16 @@ softmmu_ss.add(files(
>> 'util.c',
>>))
>>
>> +if get_option('replication').allowed() or \
>> +get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('colo-compare.c'))
>> + softmmu_ss.add(files('colo.c'))
>> +endif
>> +
>> +if get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('filter-rewriter.c'))
>> +endif
>> +
>>softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
This is the change needed, right?
>>>
>>> No, we decided to keep filter-mirror as is.
>> Ok. Anyways, this bit needs an ACK from Network Maintainer or go
>> through their tree.
>>
>
> I think r-b from Zhang is enough, he is maintainer of COLO Proxy, which
> includes filter-rewriter.
>
> (anyway, I'll resend the rest of the series when you PULL request merged)
Thanks.
Re: [PATCH v4 04/10] configure: add --disable-colo-proxy option
On 10.05.23 15:18, Juan Quintela wrote:
Vladimir Sementsov-Ogievskiy wrote:
On 09.05.23 21:42, Juan Quintela wrote:
"Zhang, Chen" wrote:
-Original Message-
From: Vladimir Sementsov-Ogievskiy
Sent: Saturday, April 29, 2023 3:49 AM
To: qemu-devel@nongnu.org
Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
; vsement...@yandex-team.ru; Paolo Bonzini
; Marc-André Lureau
; Daniel P. Berrangé
; Thomas Huth ; Philippe
Mathieu-Daudé ; Jason Wang
Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
Add option to not build filter-mirror, filter-rewriter and colo-compare when
they are not needed.
Typo: This patch still build the filter-mirror/filter-redirector in
filter-mirror.c.
Please remove the "filter-mirror" here.
Other code look good to me.
Vladimir, I was doing this myself, with the bit attached.
But then I noticed that one needs to also disable
tests/qtest/test-filter-mirror and test-filter-rewriter.
Hmm, but we decided not touch filter-mirror in this patch, only filter-rewriter.
And there is no tests/qtest/test-filter-rewriter test.
Can you resend with that fixed? Or I am missing something more
fundamental.
Thanks, Juan.
--- a/net/meson.build
+++ b/net/meson.build
@@ -1,13 +1,10 @@
softmmu_ss.add(files(
'announce.c',
'checksum.c',
- 'colo-compare.c',
- 'colo.c',
'dump.c',
'eth.c',
'filter-buffer.c',
'filter-mirror.c',
- 'filter-rewriter.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -19,6 +16,16 @@ softmmu_ss.add(files(
'util.c',
))
+if get_option('replication').allowed() or \
+get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('colo-compare.c'))
+ softmmu_ss.add(files('colo.c'))
+endif
+
+if get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('filter-rewriter.c'))
+endif
+
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
This is the change needed, right?
No, we decided to keep filter-mirror as is.
Ok. Anyways, this bit needs an ACK from Network Maintainer or go
through their tree.
I think r-b from Zhang is enough, he is maintainer of COLO Proxy, which
includes filter-rewriter.
(anyway, I'll resend the rest of the series when you PULL request merged)
--
Best regards,
Vladimir
Re: [PATCH v4 04/10] configure: add --disable-colo-proxy option
Vladimir Sementsov-Ogievskiy wrote:
> On 09.05.23 21:42, Juan Quintela wrote:
>> "Zhang, Chen" wrote:
-Original Message-
From: Vladimir Sementsov-Ogievskiy
Sent: Saturday, April 29, 2023 3:49 AM
To: qemu-devel@nongnu.org
Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
; vsement...@yandex-team.ru; Paolo Bonzini
; Marc-André Lureau
; Daniel P. Berrangé
; Thomas Huth ; Philippe
Mathieu-Daudé ; Jason Wang
Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
Add option to not build filter-mirror, filter-rewriter and colo-compare
when
they are not needed.
>>>
>>> Typo: This patch still build the filter-mirror/filter-redirector in
>>> filter-mirror.c.
>>> Please remove the "filter-mirror" here.
>>> Other code look good to me.
>> Vladimir, I was doing this myself, with the bit attached.
>> But then I noticed that one needs to also disable
>> tests/qtest/test-filter-mirror and test-filter-rewriter.
>
> Hmm, but we decided not touch filter-mirror in this patch, only
> filter-rewriter.
>
> And there is no tests/qtest/test-filter-rewriter test.
>
>> Can you resend with that fixed? Or I am missing something more
>> fundamental.
>> Thanks, Juan.
>>
--- a/net/meson.build
+++ b/net/meson.build
@@ -1,13 +1,10 @@
softmmu_ss.add(files(
'announce.c',
'checksum.c',
- 'colo-compare.c',
- 'colo.c',
'dump.c',
'eth.c',
'filter-buffer.c',
'filter-mirror.c',
- 'filter-rewriter.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -19,6 +16,16 @@ softmmu_ss.add(files(
'util.c',
))
+if get_option('replication').allowed() or \
+get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('colo-compare.c'))
+ softmmu_ss.add(files('colo.c'))
+endif
+
+if get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('filter-rewriter.c'))
+endif
+
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
>> This is the change needed, right?
>
> No, we decided to keep filter-mirror as is.
Ok. Anyways, this bit needs an ACK from Network Maintainer or go
through their tree.
Later, Juan.
Re: [PATCH v4 04/10] configure: add --disable-colo-proxy option
On 09.05.23 21:42, Juan Quintela wrote:
"Zhang, Chen" wrote:
-Original Message-
From: Vladimir Sementsov-Ogievskiy
Sent: Saturday, April 29, 2023 3:49 AM
To: qemu-devel@nongnu.org
Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
; vsement...@yandex-team.ru; Paolo Bonzini
; Marc-André Lureau
; Daniel P. Berrangé
; Thomas Huth ; Philippe
Mathieu-Daudé ; Jason Wang
Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
Add option to not build filter-mirror, filter-rewriter and colo-compare when
they are not needed.
Typo: This patch still build the filter-mirror/filter-redirector in
filter-mirror.c.
Please remove the "filter-mirror" here.
Other code look good to me.
Vladimir, I was doing this myself, with the bit attached.
But then I noticed that one needs to also disable
tests/qtest/test-filter-mirror and test-filter-rewriter.
Hmm, but we decided not touch filter-mirror in this patch, only filter-rewriter.
And there is no tests/qtest/test-filter-rewriter test.
Can you resend with that fixed? Or I am missing something more
fundamental.
Thanks, Juan.
--- a/net/meson.build
+++ b/net/meson.build
@@ -1,13 +1,10 @@
softmmu_ss.add(files(
'announce.c',
'checksum.c',
- 'colo-compare.c',
- 'colo.c',
'dump.c',
'eth.c',
'filter-buffer.c',
'filter-mirror.c',
- 'filter-rewriter.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -19,6 +16,16 @@ softmmu_ss.add(files(
'util.c',
))
+if get_option('replication').allowed() or \
+get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('colo-compare.c'))
+ softmmu_ss.add(files('colo.c'))
+endif
+
+if get_option('colo_proxy').allowed()
+ softmmu_ss.add(files('filter-rewriter.c'))
+endif
+
softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
This is the change needed, right?
No, we decided to keep filter-mirror as is.
diff --git a/net/meson.build b/net/meson.build
index 6f4ecde57f..e623bb9acb 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -4,7 +4,6 @@ softmmu_ss.add(files(
'dump.c',
'eth.c',
'filter-buffer.c',
- 'filter-mirror.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -23,7 +22,7 @@ if get_option('replication').allowed() or \
endif
if get_option('colo_proxy').allowed()
- softmmu_ss.add(files('filter-rewriter.c'))
+ softmmu_ss.add(files('filter-rewriter.c', 'filter-mirror.c'))
endif
--
Best regards,
Vladimir
Re: [PATCH v4 04/10] configure: add --disable-colo-proxy option
"Zhang, Chen" wrote:
>> -Original Message-
>> From: Vladimir Sementsov-Ogievskiy
>> Sent: Saturday, April 29, 2023 3:49 AM
>> To: qemu-devel@nongnu.org
>> Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
>> ; vsement...@yandex-team.ru; Paolo Bonzini
>> ; Marc-André Lureau
>> ; Daniel P. Berrangé
>> ; Thomas Huth ; Philippe
>> Mathieu-Daudé ; Jason Wang
>> Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
>>
>> Add option to not build filter-mirror, filter-rewriter and colo-compare when
>> they are not needed.
>
> Typo: This patch still build the filter-mirror/filter-redirector in
> filter-mirror.c.
> Please remove the "filter-mirror" here.
> Other code look good to me.
Vladimir, I was doing this myself, with the bit attached.
But then I noticed that one needs to also disable
tests/qtest/test-filter-mirror and test-filter-rewriter.
Can you resend with that fixed? Or I am missing something more
fundamental.
Thanks, Juan.
>> --- a/net/meson.build
>> +++ b/net/meson.build
>> @@ -1,13 +1,10 @@
>> softmmu_ss.add(files(
>>'announce.c',
>>'checksum.c',
>> - 'colo-compare.c',
>> - 'colo.c',
>>'dump.c',
>>'eth.c',
>>'filter-buffer.c',
>>'filter-mirror.c',
>> - 'filter-rewriter.c',
>>'filter.c',
>>'hub.c',
>>'net-hmp-cmds.c',
>> @@ -19,6 +16,16 @@ softmmu_ss.add(files(
>>'util.c',
>> ))
>>
>> +if get_option('replication').allowed() or \
>> +get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('colo-compare.c'))
>> + softmmu_ss.add(files('colo.c'))
>> +endif
>> +
>> +if get_option('colo_proxy').allowed()
>> + softmmu_ss.add(files('filter-rewriter.c'))
>> +endif
>> +
>> softmmu_ss.add(when: 'CONFIG_TCG', if_true: files('filter-replay.c'))
This is the change needed, right?
diff --git a/net/meson.build b/net/meson.build
index 6f4ecde57f..e623bb9acb 100644
--- a/net/meson.build
+++ b/net/meson.build
@@ -4,7 +4,6 @@ softmmu_ss.add(files(
'dump.c',
'eth.c',
'filter-buffer.c',
- 'filter-mirror.c',
'filter.c',
'hub.c',
'net-hmp-cmds.c',
@@ -23,7 +22,7 @@ if get_option('replication').allowed() or \
endif
if get_option('colo_proxy').allowed()
- softmmu_ss.add(files('filter-rewriter.c'))
+ softmmu_ss.add(files('filter-rewriter.c', 'filter-mirror.c'))
endif
RE: [PATCH v4 04/10] configure: add --disable-colo-proxy option
> -Original Message-
> From: Vladimir Sementsov-Ogievskiy
> Sent: Saturday, April 29, 2023 3:49 AM
> To: qemu-devel@nongnu.org
> Cc: lukasstra...@web.de; quint...@redhat.com; Zhang, Chen
> ; vsement...@yandex-team.ru; Paolo Bonzini
> ; Marc-André Lureau
> ; Daniel P. Berrangé
> ; Thomas Huth ; Philippe
> Mathieu-Daudé ; Jason Wang
> Subject: [PATCH v4 04/10] configure: add --disable-colo-proxy option
>
> Add option to not build filter-mirror, filter-rewriter and colo-compare when
> they are not needed.
Typo: This patch still build the filter-mirror/filter-redirector in
filter-mirror.c.
Please remove the "filter-mirror" here.
Other code look good to me.
Reviewed-by: Zhang Chen
Thanks
Chen
>
> There could be more agile configuration, for example add separate options
> for each filter, but that may be done in future on demand. The aim of this
> patch is to make possible to disable the whole COLO Proxy subsystem.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy
> Reviewed-by: Juan Quintela
> ---
> meson_options.txt | 2 ++
> net/meson.build | 13 ++---
> scripts/meson-buildoptions.sh | 3 +++
> stubs/colo-compare.c | 7 +++
> stubs/meson.build | 1 +
> 5 files changed, 23 insertions(+), 3 deletions(-) create mode 100644
> stubs/colo-compare.c
>
> diff --git a/meson_options.txt b/meson_options.txt index
> 2471dd02da..b59e7ae342 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_proxy', type: 'feature', value: 'auto',
> + description: 'colo-proxy 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
> 87afca3e93..6f4ecde57f 100644
> --- a/net/meson.build
> +++ b/net/meson.build
> @@ -1,13 +1,10 @@
> softmmu_ss.add(files(
>'announce.c',
>'checksum.c',
> - 'colo-compare.c',
> - 'colo.c',
>'dump.c',
>'eth.c',
>'filter-buffer.c',
>'filter-mirror.c',
> - 'filter-rewriter.c',
>'filter.c',
>'hub.c',
>'net-hmp-cmds.c',
> @@ -19,6 +16,16 @@ softmmu_ss.add(files(
>'util.c',
> ))
>
> +if get_option('replication').allowed() or \
> +get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('colo-compare.c'))
> + softmmu_ss.add(files('colo.c'))
> +endif
> +
> +if get_option('colo_proxy').allowed()
> + softmmu_ss.add(files('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 d4369a3ad8..036047ce6f 100644
> --- a/scripts/meson-buildoptions.sh
> +++ b/scripts/meson-buildoptions.sh
> @@ -83,6 +83,7 @@ meson_options_help() {
>printf "%s\n" ' capstoneWhether 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-proxy colo-proxy support'
>printf "%s\n" ' coreaudio CoreAudio sound support'
>printf "%s\n" ' crypto-afalgLinux AF_ALG crypto backend driver'
>printf "%s\n" ' curlCURL 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-proxy) printf "%s" -Dcolo_proxy=enabled ;;
> +--disable-colo-proxy) printf "%s" -Dcolo_proxy=disabled ;;
> --enable-coreaudio) printf "%s" -Dcoreaudio=enabled ;;
> --disable-coreaudio) printf "%s" -Dcoreaudio=disabled ;;
> --enable-coroutine-pool) printf "%s" -Dcoroutine_pool=true ;; diff --git
> a/stubs/colo-compare.c b/stubs/colo-compare.c new file mode 100644 index
> 00..ec726665be
> --- /dev/null
> +++ b/stubs/colo-compare.c
> @@ -0,0 +1,7 @@
> +#include "qemu/osdep.h"
> +#include "qemu/notify.h"
> +#include "net/colo-compare.h"
> +
> +void colo_compare_cleanup(void)
> +{
> +}
> diff --git a/stubs/meson.build b/stubs/meson.build index
> 8412cad15f..a56645e2f7 100644
> --- a/stubs/meson.build
> +++ b/stubs/meson.build
> @@ -46,6 +46,7 @@ stub_ss.add(files('target-monitor-defs.c'))
> stub_ss.add(files('trace-control.c'))
> stub_ss.add(files('uuid.c'))
> stub_ss.add(files('colo.c'))
> +stub_ss.add(files('colo-compare.c'))
> stub_ss.add(files('vmstate.c'))
> stub_ss.add(files('vm-stop.c'))
> stub_ss.add(files('win32-kbd-hook.c'))
> --
> 2.34.1
