* [PATCH] test/vsock: add install target @ 2024-07-09 13:50 Peng Fan (OSS) 2024-07-10 7:34 ` Stefano Garzarella 0 siblings, 1 reply; 11+ messages in thread From: Peng Fan (OSS) @ 2024-07-09 13:50 UTC (permalink / raw) To: sgarzare; +Cc: virtualization, netdev, linux-kernel, Peng Fan From: Peng Fan <peng.fan@nxp.com> Add install target for vsock to make Yocto easy to install the images. Signed-off-by: Peng Fan <peng.fan@nxp.com> --- tools/testing/vsock/Makefile | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460 100644 --- a/tools/testing/vsock/Makefile +++ b/tools/testing/vsock/Makefile @@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o msg_zerocopy_common.o vsock_uring_test: LDLIBS = -luring vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o msg_zerocopy_common.o +VSOCK_INSTALL_PATH ?= $(abspath .) +# Avoid changing the rest of the logic here and lib.mk. +INSTALL_PATH := $(VSOCK_INSTALL_PATH) + CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE .PHONY: all test clean clean: ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf vsock_uring_test -include *.d + +install: all + @# Ask all targets to install their files + mkdir -p $(INSTALL_PATH)/vsock + install -m 744 vsock_test $(INSTALL_PATH)/vsock/ + install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ + install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ + install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ -- 2.37.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-09 13:50 [PATCH] test/vsock: add install target Peng Fan (OSS) @ 2024-07-10 7:34 ` Stefano Garzarella 2024-07-10 8:11 ` Peng Fan 0 siblings, 1 reply; 11+ messages in thread From: Stefano Garzarella @ 2024-07-10 7:34 UTC (permalink / raw) To: Peng Fan (OSS); +Cc: virtualization, netdev, linux-kernel, Peng Fan On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote: >From: Peng Fan <peng.fan@nxp.com> > >Add install target for vsock to make Yocto easy to install the images. > >Signed-off-by: Peng Fan <peng.fan@nxp.com> >--- > tools/testing/vsock/Makefile | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > >diff --git a/tools/testing/vsock/Makefile b/tools/testing/vsock/Makefile >index a7f56a09ca9f..5c8442fa9460 100644 >--- a/tools/testing/vsock/Makefile >+++ b/tools/testing/vsock/Makefile >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o msg_zerocopy_common.o > vsock_uring_test: LDLIBS = -luring > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o msg_zerocopy_common.o > >+VSOCK_INSTALL_PATH ?= $(abspath .) >+# Avoid changing the rest of the logic here and lib.mk. >+INSTALL_PATH := $(VSOCK_INSTALL_PATH) >+ > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE -D_GNU_SOURCE > .PHONY: all test clean > clean: > ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf vsock_uring_test > -include *.d >+ >+install: all >+ @# Ask all targets to install their files >+ mkdir -p $(INSTALL_PATH)/vsock why using the "vsock" subdir? IIUC you were inspired by selftests/Makefile, but it installs under $(INSTALL_PATH)/kselftest/ the scripts used by the main one `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead. So in this case I would install everything in $(INSTALL_PATH). WDYT? >+ install -m 744 vsock_test $(INSTALL_PATH)/vsock/ >+ install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ >+ install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ >+ install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ Also from selftests/Makefile, what about using the ifdef instead of using $(abspath .) as default place? I mean this: install: all ifdef INSTALL_PATH ... else $(error Error: set INSTALL_PATH to use install) endif Thanks, Stefano ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] test/vsock: add install target 2024-07-10 7:34 ` Stefano Garzarella @ 2024-07-10 8:11 ` Peng Fan 2024-07-10 9:11 ` Stefano Garzarella 0 siblings, 1 reply; 11+ messages in thread From: Peng Fan @ 2024-07-10 8:11 UTC (permalink / raw) To: Stefano Garzarella, Peng Fan (OSS) Cc: virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org > Subject: Re: [PATCH] test/vsock: add install target > > On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote: > >From: Peng Fan <peng.fan@nxp.com> > > > >Add install target for vsock to make Yocto easy to install the images. > > > >Signed-off-by: Peng Fan <peng.fan@nxp.com> > >--- > > tools/testing/vsock/Makefile | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > >diff --git a/tools/testing/vsock/Makefile > >b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460 > 100644 > >--- a/tools/testing/vsock/Makefile > >+++ b/tools/testing/vsock/Makefile > >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o > msg_zerocopy_common.o > > vsock_uring_test: LDLIBS = -luring > > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o > >msg_zerocopy_common.o > > > >+VSOCK_INSTALL_PATH ?= $(abspath .) > >+# Avoid changing the rest of the logic here and lib.mk. > >+INSTALL_PATH := $(VSOCK_INSTALL_PATH) > >+ > > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include > > -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow > > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE - > D_GNU_SOURCE > > .PHONY: all test clean > > clean: > > ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf > vsock_uring_test > > -include *.d > >+ > >+install: all > >+ @# Ask all targets to install their files > >+ mkdir -p $(INSTALL_PATH)/vsock > > why using the "vsock" subdir? > > IIUC you were inspired by selftests/Makefile, but it installs under > $(INSTALL_PATH)/kselftest/ the scripts used by the main one > `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead. > So in this case I would install everything in $(INSTALL_PATH). > > WDYT? I agree. > > >+ install -m 744 vsock_test $(INSTALL_PATH)/vsock/ > >+ install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ > >+ install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ > >+ install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ > > Also from selftests/Makefile, what about using the ifdef instead of > using $(abspath .) as default place? > > I mean this: > > install: all > ifdef INSTALL_PATH > ... > else > $(error Error: set INSTALL_PATH to use install) endif Is the following looks good to you? # Avoid conflict with INSTALL_PATH set by the main Makefile VSOCK_INSTALL_PATH ?= INSTALL_PATH := $(VSOCK_INSTALL_PATH) install: all ifdef INSTALL_PATH mkdir -p $(INSTALL_PATH) install -m 744 vsock_test $(INSTALL_PATH) install -m 744 vsock_perf $(INSTALL_PATH) install -m 744 vsock_diag_test $(INSTALL_PATH) install -m 744 vsock_uring_test $(INSTALL_PATH) else $(error Error: set INSTALL_PATH to use install) Endif Thanks, Peng. > > Thanks, > Stefano ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-10 8:11 ` Peng Fan @ 2024-07-10 9:11 ` Stefano Garzarella 2024-07-10 11:34 ` Peng Fan 0 siblings, 1 reply; 11+ messages in thread From: Stefano Garzarella @ 2024-07-10 9:11 UTC (permalink / raw) To: Peng Fan Cc: Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Jul 10, 2024 at 08:11:32AM GMT, Peng Fan wrote: >> Subject: Re: [PATCH] test/vsock: add install target >> >> On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote: >> >From: Peng Fan <peng.fan@nxp.com> >> > >> >Add install target for vsock to make Yocto easy to install the images. >> > >> >Signed-off-by: Peng Fan <peng.fan@nxp.com> >> >--- >> > tools/testing/vsock/Makefile | 12 ++++++++++++ >> > 1 file changed, 12 insertions(+) >> > >> >diff --git a/tools/testing/vsock/Makefile >> >b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460 >> 100644 >> >--- a/tools/testing/vsock/Makefile >> >+++ b/tools/testing/vsock/Makefile >> >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o >> msg_zerocopy_common.o >> > vsock_uring_test: LDLIBS = -luring >> > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o >> >msg_zerocopy_common.o >> > >> >+VSOCK_INSTALL_PATH ?= $(abspath .) >> >+# Avoid changing the rest of the logic here and lib.mk. >> >+INSTALL_PATH := $(VSOCK_INSTALL_PATH) >> >+ >> > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include >> > -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow >> > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE - >> D_GNU_SOURCE >> > .PHONY: all test clean >> > clean: >> > ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf >> vsock_uring_test >> > -include *.d >> >+ >> >+install: all >> >+ @# Ask all targets to install their files >> >+ mkdir -p $(INSTALL_PATH)/vsock >> >> why using the "vsock" subdir? >> >> IIUC you were inspired by selftests/Makefile, but it installs under >> $(INSTALL_PATH)/kselftest/ the scripts used by the main one >> `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead. >> So in this case I would install everything in $(INSTALL_PATH). >> >> WDYT? > >I agree. > >> >> >+ install -m 744 vsock_test $(INSTALL_PATH)/vsock/ >> >+ install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ >> >+ install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ >> >+ install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ >> >> Also from selftests/Makefile, what about using the ifdef instead of >> using $(abspath .) as default place? >> >> I mean this: >> >> install: all >> ifdef INSTALL_PATH >> ... >> else >> $(error Error: set INSTALL_PATH to use install) endif > >Is the following looks good to you? > ># Avoid conflict with INSTALL_PATH set by the main Makefile >VSOCK_INSTALL_PATH ?= >INSTALL_PATH := $(VSOCK_INSTALL_PATH) I'm not a super Makefile expert, but why do we need both VSOCK_INSTALL_PATH and INSTALL_PATH? Stefano > >install: all >ifdef INSTALL_PATH > mkdir -p $(INSTALL_PATH) > install -m 744 vsock_test $(INSTALL_PATH) > install -m 744 vsock_perf $(INSTALL_PATH) > install -m 744 vsock_diag_test $(INSTALL_PATH) > install -m 744 vsock_uring_test $(INSTALL_PATH) >else > $(error Error: set INSTALL_PATH to use install) >Endif > >Thanks, >Peng. >> >> Thanks, >> Stefano > ^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: [PATCH] test/vsock: add install target 2024-07-10 9:11 ` Stefano Garzarella @ 2024-07-10 11:34 ` Peng Fan 2024-07-10 11:58 ` Stefano Garzarella 0 siblings, 1 reply; 11+ messages in thread From: Peng Fan @ 2024-07-10 11:34 UTC (permalink / raw) To: Stefano Garzarella Cc: Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org > Subject: Re: [PATCH] test/vsock: add install target > > On Wed, Jul 10, 2024 at 08:11:32AM GMT, Peng Fan wrote: > >> Subject: Re: [PATCH] test/vsock: add install target > >> > >> On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote: > >> >From: Peng Fan <peng.fan@nxp.com> > >> > > >> >Add install target for vsock to make Yocto easy to install the > images. > >> > > >> >Signed-off-by: Peng Fan <peng.fan@nxp.com> > >> >--- > >> > tools/testing/vsock/Makefile | 12 ++++++++++++ > >> > 1 file changed, 12 insertions(+) > >> > > >> >diff --git a/tools/testing/vsock/Makefile > >> >b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460 > >> 100644 > >> >--- a/tools/testing/vsock/Makefile > >> >+++ b/tools/testing/vsock/Makefile > >> >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o > >> msg_zerocopy_common.o > >> > vsock_uring_test: LDLIBS = -luring > >> > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o > >> >msg_zerocopy_common.o > >> > > >> >+VSOCK_INSTALL_PATH ?= $(abspath .) > >> >+# Avoid changing the rest of the logic here and lib.mk. > >> >+INSTALL_PATH := $(VSOCK_INSTALL_PATH) > >> >+ > >> > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include > >> > -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow > >> > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE - > >> D_GNU_SOURCE > >> > .PHONY: all test clean > >> > clean: > >> > ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf > >> vsock_uring_test > >> > -include *.d > >> >+ > >> >+install: all > >> >+ @# Ask all targets to install their files > >> >+ mkdir -p $(INSTALL_PATH)/vsock > >> > >> why using the "vsock" subdir? > >> > >> IIUC you were inspired by selftests/Makefile, but it installs under > >> $(INSTALL_PATH)/kselftest/ the scripts used by the main one > >> `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead. > >> So in this case I would install everything in $(INSTALL_PATH). > >> > >> WDYT? > > > >I agree. > > > >> > >> >+ install -m 744 vsock_test $(INSTALL_PATH)/vsock/ > >> >+ install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ > >> >+ install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ > >> >+ install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ > >> > >> Also from selftests/Makefile, what about using the ifdef instead of > >> using $(abspath .) as default place? > >> > >> I mean this: > >> > >> install: all > >> ifdef INSTALL_PATH > >> ... > >> else > >> $(error Error: set INSTALL_PATH to use install) endif > > > >Is the following looks good to you? > > > ># Avoid conflict with INSTALL_PATH set by the main Makefile > >VSOCK_INSTALL_PATH ?= INSTALL_PATH := $(VSOCK_INSTALL_PATH) > > I'm not a super Makefile expert, but why do we need both > VSOCK_INSTALL_PATH and INSTALL_PATH? INSTALL_PATH is exported by kernel root directory makefile. So to user, we need to avoid export INSTALL_PATH here. So I just follow selftests/Makefile using KSFT_INSTALL_PATH Regards, Peng. > > Stefano > > > > >install: all > >ifdef INSTALL_PATH > > mkdir -p $(INSTALL_PATH) > > install -m 744 vsock_test $(INSTALL_PATH) > > install -m 744 vsock_perf $(INSTALL_PATH) > > install -m 744 vsock_diag_test $(INSTALL_PATH) > > install -m 744 vsock_uring_test $(INSTALL_PATH) else > > $(error Error: set INSTALL_PATH to use install) Endif > > > >Thanks, > >Peng. > >> > >> Thanks, > >> Stefano > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-10 11:34 ` Peng Fan @ 2024-07-10 11:58 ` Stefano Garzarella 2024-07-11 2:00 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Stefano Garzarella @ 2024-07-10 11:58 UTC (permalink / raw) To: Peng Fan Cc: Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, Jul 10, 2024 at 11:34:05AM GMT, Peng Fan wrote: >> Subject: Re: [PATCH] test/vsock: add install target >> >> On Wed, Jul 10, 2024 at 08:11:32AM GMT, Peng Fan wrote: >> >> Subject: Re: [PATCH] test/vsock: add install target >> >> >> >> On Tue, Jul 09, 2024 at 09:50:51PM GMT, Peng Fan (OSS) wrote: >> >> >From: Peng Fan <peng.fan@nxp.com> >> >> > >> >> >Add install target for vsock to make Yocto easy to install the >> images. >> >> > >> >> >Signed-off-by: Peng Fan <peng.fan@nxp.com> >> >> >--- >> >> > tools/testing/vsock/Makefile | 12 ++++++++++++ >> >> > 1 file changed, 12 insertions(+) >> >> > >> >> >diff --git a/tools/testing/vsock/Makefile >> >> >b/tools/testing/vsock/Makefile index a7f56a09ca9f..5c8442fa9460 >> >> 100644 >> >> >--- a/tools/testing/vsock/Makefile >> >> >+++ b/tools/testing/vsock/Makefile >> >> >@@ -8,8 +8,20 @@ vsock_perf: vsock_perf.o >> >> msg_zerocopy_common.o >> >> > vsock_uring_test: LDLIBS = -luring >> >> > vsock_uring_test: control.o util.o vsock_uring_test.o timeout.o >> >> >msg_zerocopy_common.o >> >> > >> >> >+VSOCK_INSTALL_PATH ?= $(abspath .) >> >> >+# Avoid changing the rest of the logic here and lib.mk. >> >> >+INSTALL_PATH := $(VSOCK_INSTALL_PATH) >> >> >+ >> >> > CFLAGS += -g -O2 -Werror -Wall -I. -I../../include >> >> > -I../../../usr/include -Wno-pointer-sign -fno-strict-overflow >> >> > -fno-strict-aliasing -fno-common -MMD -U_FORTIFY_SOURCE - >> >> D_GNU_SOURCE >> >> > .PHONY: all test clean >> >> > clean: >> >> > ${RM} *.o *.d vsock_test vsock_diag_test vsock_perf >> >> vsock_uring_test >> >> > -include *.d >> >> >+ >> >> >+install: all >> >> >+ @# Ask all targets to install their files >> >> >+ mkdir -p $(INSTALL_PATH)/vsock >> >> >> >> why using the "vsock" subdir? >> >> >> >> IIUC you were inspired by selftests/Makefile, but it installs under >> >> $(INSTALL_PATH)/kselftest/ the scripts used by the main one >> >> `run_kselftest.sh`, which is installed in $(INSTALL_PATH instead. >> >> So in this case I would install everything in $(INSTALL_PATH). >> >> >> >> WDYT? >> > >> >I agree. >> > >> >> >> >> >+ install -m 744 vsock_test $(INSTALL_PATH)/vsock/ >> >> >+ install -m 744 vsock_perf $(INSTALL_PATH)/vsock/ >> >> >+ install -m 744 vsock_diag_test $(INSTALL_PATH)/vsock/ >> >> >+ install -m 744 vsock_uring_test $(INSTALL_PATH)/vsock/ >> >> >> >> Also from selftests/Makefile, what about using the ifdef instead of >> >> using $(abspath .) as default place? >> >> >> >> I mean this: >> >> >> >> install: all >> >> ifdef INSTALL_PATH >> >> ... >> >> else >> >> $(error Error: set INSTALL_PATH to use install) endif >> > >> >Is the following looks good to you? >> > >> ># Avoid conflict with INSTALL_PATH set by the main Makefile >> >VSOCK_INSTALL_PATH ?= INSTALL_PATH := $(VSOCK_INSTALL_PATH) >> >> I'm not a super Makefile expert, but why do we need both >> VSOCK_INSTALL_PATH and INSTALL_PATH? > >INSTALL_PATH is exported by kernel root directory makefile. >So to user, we need to avoid export INSTALL_PATH here. >So I just follow selftests/Makefile using KSFT_INSTALL_PATH There is a comment there: # Avoid changing the rest of the logic here and lib.mk. Added by commit 17eac6c2db8b2cdfe33d40229bdda2acd86b304a. IIUC they re-used INSTALL_PATH, just to avoid too many changes in that file and in tools/testing/selftests/lib.mk So, IMHO we should not care about it and only use VSOCK_INSTALL_PATH if you don't want to conflict with INSTALL_PATH. Stefano ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-10 11:58 ` Stefano Garzarella @ 2024-07-11 2:00 ` Jakub Kicinski 2024-07-11 7:07 ` Stefano Garzarella 0 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2024-07-11 2:00 UTC (permalink / raw) To: Stefano Garzarella Cc: Peng Fan, Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Wed, 10 Jul 2024 13:58:39 +0200 Stefano Garzarella wrote: > There is a comment there: > > # Avoid changing the rest of the logic here and lib.mk. > > Added by commit 17eac6c2db8b2cdfe33d40229bdda2acd86b304a. > > IIUC they re-used INSTALL_PATH, just to avoid too many changes in that > file and in tools/testing/selftests/lib.mk > > So, IMHO we should not care about it and only use VSOCK_INSTALL_PATH if > you don't want to conflict with INSTALL_PATH. Any reason why vsock isn't part of selftests in the first place? ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-11 2:00 ` Jakub Kicinski @ 2024-07-11 7:07 ` Stefano Garzarella 2024-07-11 13:38 ` Stefan Hajnoczi 0 siblings, 1 reply; 11+ messages in thread From: Stefano Garzarella @ 2024-07-11 7:07 UTC (permalink / raw) To: Jakub Kicinski, stefanha Cc: Peng Fan, Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org CCing Stefan. On Wed, Jul 10, 2024 at 07:00:59PM GMT, Jakub Kicinski wrote: >On Wed, 10 Jul 2024 13:58:39 +0200 Stefano Garzarella wrote: >> There is a comment there: >> >> # Avoid changing the rest of the logic here and lib.mk. >> >> Added by commit 17eac6c2db8b2cdfe33d40229bdda2acd86b304a. >> >> IIUC they re-used INSTALL_PATH, just to avoid too many changes in that >> file and in tools/testing/selftests/lib.mk >> >> So, IMHO we should not care about it and only use VSOCK_INSTALL_PATH if >> you don't want to conflict with INSTALL_PATH. > >Any reason why vsock isn't part of selftests in the first place? > Usually vsock tests test both the driver (virtio-vsock) in the guest and the device in the host kernel (vhost-vsock). So I usually run the tests in 2 nested VMs to test the latest changes for both the guest and the host. I don't know enough selftests, but do you think it is possible to integrate them? CCing Stefan who is the original author and may remember more reasons about this choice. Thanks, Stefano ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-11 7:07 ` Stefano Garzarella @ 2024-07-11 13:38 ` Stefan Hajnoczi 2024-07-11 14:14 ` Jakub Kicinski 0 siblings, 1 reply; 11+ messages in thread From: Stefan Hajnoczi @ 2024-07-11 13:38 UTC (permalink / raw) To: Stefano Garzarella Cc: Jakub Kicinski, Peng Fan, Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org [-- Attachment #1: Type: text/plain, Size: 1946 bytes --] On Thu, Jul 11, 2024 at 09:07:04AM +0200, Stefano Garzarella wrote: > CCing Stefan. > > On Wed, Jul 10, 2024 at 07:00:59PM GMT, Jakub Kicinski wrote: > > On Wed, 10 Jul 2024 13:58:39 +0200 Stefano Garzarella wrote: > > > There is a comment there: > > > > > > # Avoid changing the rest of the logic here and lib.mk. > > > > > > Added by commit 17eac6c2db8b2cdfe33d40229bdda2acd86b304a. > > > > > > IIUC they re-used INSTALL_PATH, just to avoid too many changes in that > > > file and in tools/testing/selftests/lib.mk > > > > > > So, IMHO we should not care about it and only use VSOCK_INSTALL_PATH if > > > you don't want to conflict with INSTALL_PATH. > > > > Any reason why vsock isn't part of selftests in the first place? > > > > Usually vsock tests test both the driver (virtio-vsock) in the guest and the > device in the host kernel (vhost-vsock). So I usually run the tests in 2 > nested VMs to test the latest changes for both the guest and the host. > > I don't know enough selftests, but do you think it is possible to integrate > them? > > CCing Stefan who is the original author and may remember more reasons about > this choice. It's probably because of the manual steps in tools/testing/vsock/README: The following prerequisite steps are not automated and must be performed prior to running tests: 1. Build the kernel, make headers_install, and build these tests. 2. Install the kernel and tests on the host. 3. Install the kernel and tests inside the guest. 4. Boot the guest and ensure that the AF_VSOCK transport is enabled. If you want to automate this for QEMU, VMware, and Hyper-V that would be great. It relies on having a guest running under these hypervisors and that's not trivial to automate (plus it involves proprietary software for VMware and Hyper-V that may not be available without additional license agreements and/or payment). Stefan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-11 13:38 ` Stefan Hajnoczi @ 2024-07-11 14:14 ` Jakub Kicinski 2024-07-12 8:53 ` Stefano Garzarella 0 siblings, 1 reply; 11+ messages in thread From: Jakub Kicinski @ 2024-07-11 14:14 UTC (permalink / raw) To: Stefan Hajnoczi Cc: Stefano Garzarella, Peng Fan, Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Thu, 11 Jul 2024 15:38:01 +0200 Stefan Hajnoczi wrote: > > Usually vsock tests test both the driver (virtio-vsock) in the guest and the > > device in the host kernel (vhost-vsock). So I usually run the tests in 2 > > nested VMs to test the latest changes for both the guest and the host. > > > > I don't know enough selftests, but do you think it is possible to integrate > > them? > > > > CCing Stefan who is the original author and may remember more reasons about > > this choice. > > It's probably because of the manual steps in tools/testing/vsock/README: > > The following prerequisite steps are not automated and must be performed prior > to running tests: > > 1. Build the kernel, make headers_install, and build these tests. > 2. Install the kernel and tests on the host. > 3. Install the kernel and tests inside the guest. > 4. Boot the guest and ensure that the AF_VSOCK transport is enabled. > > If you want to automate this for QEMU, VMware, and Hyper-V that would be > great. It relies on having a guest running under these hypervisors and > that's not trivial to automate (plus it involves proprietary software > for VMware and Hyper-V that may not be available without additional > license agreements and/or payment). Not sure if there's a requirement that full process is automated. Or at least if there is we are already breaking it in networking because for some tests we need user to export some env variables to point the test to the right interfaces and even a remote machine to generate traffic. If the env isn't set up tests return 4 (SKIP). I don't feel strongly that ksft + env approach is better but at least it gives us easy access to the basic build and packaging features from ksft. Up to you but thought I'd ask. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] test/vsock: add install target 2024-07-11 14:14 ` Jakub Kicinski @ 2024-07-12 8:53 ` Stefano Garzarella 0 siblings, 0 replies; 11+ messages in thread From: Stefano Garzarella @ 2024-07-12 8:53 UTC (permalink / raw) To: Jakub Kicinski Cc: Stefan Hajnoczi, Peng Fan, Peng Fan (OSS), virtualization@lists.linux.dev, netdev@vger.kernel.org, linux-kernel@vger.kernel.org On Thu, Jul 11, 2024 at 07:14:55AM GMT, Jakub Kicinski wrote: >On Thu, 11 Jul 2024 15:38:01 +0200 Stefan Hajnoczi wrote: >> > Usually vsock tests test both the driver (virtio-vsock) in the guest and the >> > device in the host kernel (vhost-vsock). So I usually run the tests in 2 >> > nested VMs to test the latest changes for both the guest and the host. >> > >> > I don't know enough selftests, but do you think it is possible to integrate >> > them? >> > >> > CCing Stefan who is the original author and may remember more reasons about >> > this choice. >> >> It's probably because of the manual steps in tools/testing/vsock/README: >> >> The following prerequisite steps are not automated and must be performed prior >> to running tests: >> >> 1. Build the kernel, make headers_install, and build these tests. >> 2. Install the kernel and tests on the host. >> 3. Install the kernel and tests inside the guest. >> 4. Boot the guest and ensure that the AF_VSOCK transport is enabled. >> >> If you want to automate this for QEMU, VMware, and Hyper-V that would be >> great. It relies on having a guest running under these hypervisors and >> that's not trivial to automate (plus it involves proprietary software >> for VMware and Hyper-V that may not be available without additional >> license agreements and/or payment). > >Not sure if there's a requirement that full process is automated. >Or at least if there is we are already breaking it in networking >because for some tests we need user to export some env variables >to point the test to the right interfaces and even a remote machine >to generate traffic. If the env isn't set up tests return 4 (SKIP). >I don't feel strongly that ksft + env approach is better but at >least it gives us easy access to the basic build and packaging >features from ksft. Up to you but thought I'd ask. > Yeah, I'll try to allocate some cycles to look into that. Tracking it here: https://gitlab.com/vsock/vsock/-/issues/13 What about this patch, can we queue it for now? Thanks, Stefano ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-07-12 8:53 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-09 13:50 [PATCH] test/vsock: add install target Peng Fan (OSS) 2024-07-10 7:34 ` Stefano Garzarella 2024-07-10 8:11 ` Peng Fan 2024-07-10 9:11 ` Stefano Garzarella 2024-07-10 11:34 ` Peng Fan 2024-07-10 11:58 ` Stefano Garzarella 2024-07-11 2:00 ` Jakub Kicinski 2024-07-11 7:07 ` Stefano Garzarella 2024-07-11 13:38 ` Stefan Hajnoczi 2024-07-11 14:14 ` Jakub Kicinski 2024-07-12 8:53 ` Stefano Garzarella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).