| Message ID | 20250226063043.732455-7-lizhijian@fujitsu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration/rdma: fixes, refactor and cleanup | expand |
Li Zhijian via <qemu-devel@nongnu.org> writes: > This qtest requires there is a RDMA(RoCE) link in the host. > In order to make the test work smoothly, introduce a > scripts/rdma-migration-helper.sh to > - setup a new Soft-RoCE(aka RXE) if it's root > - detect existing RoCE link > > Test will be skipped if there is no available RoCE link. > # Start of rdma tests > # Running /x86_64/migration/precopy/rdma/plain > ok 1 /x86_64/migration/precopy/rdma/plain # SKIP > There is no available rdma link to run RDMA migration test. > To enable the test: > (1) Run 'scripts/rdma-migration-helper.sh setup' with root and rerun the test sudo scripts/rdma-migration-helper.sh setup QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test --full -r /x86_64/migration/precopy/rdma/plain # { # "error": { # "class": "GenericError", # "desc": "RDMA ERROR: rdma migration: error registering 0 control!" # } # } > or > (2) Run the test with root privilege This one works fine. > > # End of rdma tests > > Reviewed-by: Peter Xu <peterx@redhat.com> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> > --- > MAINTAINERS | 1 + > scripts/rdma-migration-helper.sh | 41 +++++++++++++++++ > tests/qtest/migration/precopy-tests.c | 64 +++++++++++++++++++++++++++ > 3 files changed, 106 insertions(+) > create mode 100755 scripts/rdma-migration-helper.sh > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3848d37a38d..15360fcdc4b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3480,6 +3480,7 @@ R: Li Zhijian <lizhijian@fujitsu.com> > R: Peter Xu <peterx@redhat.com> > S: Odd Fixes > F: migration/rdma* > +F: scripts/rdma-migration-helper.sh > > Migration dirty limit and dirty page rate > M: Hyman Huang <yong.huang@smartx.com> > diff --git a/scripts/rdma-migration-helper.sh b/scripts/rdma-migration-helper.sh > new file mode 100755 > index 00000000000..66557d9e267 > --- /dev/null > +++ b/scripts/rdma-migration-helper.sh > @@ -0,0 +1,41 @@ > +#!/bin/bash > + I'd prefer a command -v rdma check around here. With the way the script pipes commands into one another will cause bash to emit a couple of "rdma: command not found" in case rdma command is not present. > +# Copied from blktests > +get_ipv4_addr() > +{ > + ip -4 -o addr show dev "$1" | > + sed -n 's/.*[[:blank:]]inet[[:blank:]]*\([^[:blank:]/]*\).*/\1/p' | > + tr -d '\n' > +} > + > +has_soft_rdma() > +{ > + rdma link | grep -q " netdev $1[[:blank:]]*\$" > +} > + > +rdma_rxe_setup_detect() > +{ > + ( > + cd /sys/class/net && > + for i in *; do > + [ -e "$i" ] || continue > + [ "$i" = "lo" ] && continue > + [ "$(<"$i/addr_len")" = 6 ] || continue > + [ "$(<"$i/carrier")" = 1 ] || continue > + > + has_soft_rdma "$i" && break > + [ "$operation" = "setup" ] && > + rdma link add "${i}_rxe" type rxe netdev "$i" && break > + done > + has_soft_rdma "$i" || return > + get_ipv4_addr "$i" > + ) > +} > + > +operation=${1:-setup} > + > +if [ "$operation" == "setup" ] || [ "$operation" == "detect" ]; then > + rdma_rxe_setup_detect > +else > + echo "Usage: $0 [setup | detect]" > +fi What happened to the cleanup option? I think I missed some discussion on this... We can't expect people to know how to clean this up without any hint. > diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c > index ba273d10b9a..bf97f4e9325 100644 > --- a/tests/qtest/migration/precopy-tests.c > +++ b/tests/qtest/migration/precopy-tests.c > @@ -99,6 +99,66 @@ static void test_precopy_unix_dirty_ring(void) > test_precopy_common(&args); > } > > +#ifdef CONFIG_RDMA > + > +#define RDMA_MIGRATION_HELPER "scripts/rdma-migration-helper.sh" > +static int new_rdma_link(char *buffer) > +{ > + const char *argument = (geteuid() == 0) ? "setup" : "detect"; > + char cmd[1024]; > + > + snprintf(cmd, sizeof(cmd), "%s %s", RDMA_MIGRATION_HELPER, argument); > + > + FILE *pipe = popen(cmd, "r"); This needs to be silenced, otherwise messages from the script will break TAP output. I suggest: bool verbose = g_getenv("QTEST_LOG"); snprintf(cmd, sizeof(cmd), "%s %s %s", RDMA_MIGRATION_HELPER, argument, verbose ? "" : "2>/dev/null"); > + if (pipe == NULL) { > + perror("Failed to run script"); > + return -1; > + } > + > + int idx = 0; > + while (fgets(buffer + idx, 128 - idx, pipe) != NULL) { > + idx += strlen(buffer); > + } > + > + int status = pclose(pipe); > + if (status == -1) { > + perror("Error reported by pclose()"); > + return -1; > + } else if (WIFEXITED(status)) { > + return WEXITSTATUS(status); > + } > + > + return -1; > +} > + > +static void test_precopy_rdma_plain(void) > +{ > + char buffer[128] = {}; > + > + if (new_rdma_link(buffer)) { > + g_test_skip("\nThere is no available rdma link to run RDMA migration test.\n" > + "To enable the test:\n" > + "(1) Run \'" RDMA_MIGRATION_HELPER " setup\' with root and rerun the test\n" > + "or\n" > + "(2) Run the test with root privilege\n"); g_test_skip() needs a one-line message, otherwise it breaks TAP output. You can turn this into a g_test_message(), put it under QTEST_LOG=1 and add a g_test_skip("no rdma link available") below. > + return; > + } > + > + /* > + * TODO: query a free port instead of hard code. > + * 29200=('R'+'D'+'M'+'A')*100 > + **/ > + g_autofree char *uri = g_strdup_printf("rdma:%s:29200", buffer); > + > + MigrateCommon args = { > + .listen_uri = uri, > + .connect_uri = uri, > + }; > + > + test_precopy_common(&args); > +} > +#endif > + > static void test_precopy_tcp_plain(void) > { > MigrateCommon args = { > @@ -1124,6 +1184,10 @@ static void migration_test_add_precopy_smoke(MigrationTestEnv *env) > test_multifd_tcp_uri_none); > migration_test_add("/migration/multifd/tcp/plain/cancel", > test_multifd_tcp_cancel); > +#ifdef CONFIG_RDMA > + migration_test_add("/migration/precopy/rdma/plain", > + test_precopy_rdma_plain); > +#endif > } > > void migration_test_add_precopy(MigrationTestEnv *env)
Fabiano Thanks for your testing. On 28/02/2025 21:49, Fabiano Rosas wrote: > Li Zhijian via <qemu-devel@nongnu.org> writes: > >> This qtest requires there is a RDMA(RoCE) link in the host. >> In order to make the test work smoothly, introduce a >> scripts/rdma-migration-helper.sh to >> - setup a new Soft-RoCE(aka RXE) if it's root >> - detect existing RoCE link >> >> Test will be skipped if there is no available RoCE link. >> # Start of rdma tests >> # Running /x86_64/migration/precopy/rdma/plain >> ok 1 /x86_64/migration/precopy/rdma/plain # SKIP >> There is no available rdma link to run RDMA migration test. >> To enable the test: >> (1) Run 'scripts/rdma-migration-helper.sh setup' with root and rerun the test > > sudo scripts/rdma-migration-helper.sh setup > QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test > --full -r /x86_64/migration/precopy/rdma/plain > > # { > # "error": { > # "class": "GenericError", > # "desc": "RDMA ERROR: rdma migration: error registering 0 control!" > # } > # } > 1333 static int qemu_rdma_reg_control(RDMAContext *rdma, int idx) 1334 { 1335 rdma->wr_data[idx].control_mr = ibv_reg_mr(rdma->pd, 1336 rdma->wr_data[idx].control, RDMA_CONTROL_MAX_BUFFER, 1337 IBV_ACCESS_LOCAL_WRITE | IBV_ACCESS_REMOTE_WRITE); <<<=== It failed here 1338 if (rdma->wr_data[idx].control_mr) { 1339 rdma->total_registrations++; 1340 return 0; 1341 } 1342 return -1; 1343 } It appears to have failed at ibv_reg_mr() This worked on my Ubuntu2204 and Fedora40. I wonder if your distro's security policy is preventing MR registration without root privileges...? >> or >> (2) Run the test with root privilege > > This one works fine. > >> >> # End of rdma tests >> >> Reviewed-by: Peter Xu <peterx@redhat.com> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com> >> --- >> MAINTAINERS | 1 + >> scripts/rdma-migration-helper.sh | 41 +++++++++++++++++ >> tests/qtest/migration/precopy-tests.c | 64 +++++++++++++++++++++++++++ >> 3 files changed, 106 insertions(+) >> create mode 100755 scripts/rdma-migration-helper.sh >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index 3848d37a38d..15360fcdc4b 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -3480,6 +3480,7 @@ R: Li Zhijian <lizhijian@fujitsu.com> >> R: Peter Xu <peterx@redhat.com> >> S: Odd Fixes >> F: migration/rdma* >> +F: scripts/rdma-migration-helper.sh >> >> Migration dirty limit and dirty page rate >> M: Hyman Huang <yong.huang@smartx.com> >> diff --git a/scripts/rdma-migration-helper.sh b/scripts/rdma-migration-helper.sh >> new file mode 100755 >> index 00000000000..66557d9e267 >> --- /dev/null >> +++ b/scripts/rdma-migration-helper.sh >> @@ -0,0 +1,41 @@ >> +#!/bin/bash >> + > > I'd prefer a command -v rdma check around here. With the way the script > pipes commands into one another will cause bash to emit a couple of > "rdma: command not found" in case rdma command is not present. > It sounds good to me. >> +# Copied from blktests >> +get_ipv4_addr() >> +{ >> + ip -4 -o addr show dev "$1" | >> + sed -n 's/.*[[:blank:]]inet[[:blank:]]*\([^[:blank:]/]*\).*/\1/p' | >> + tr -d '\n' >> +} >> + >> +has_soft_rdma() >> +{ >> + rdma link | grep -q " netdev $1[[:blank:]]*\$" >> +} >> + >> +rdma_rxe_setup_detect() >> +{ >> + ( >> + cd /sys/class/net && >> + for i in *; do >> + [ -e "$i" ] || continue >> + [ "$i" = "lo" ] && continue >> + [ "$(<"$i/addr_len")" = 6 ] || continue >> + [ "$(<"$i/carrier")" = 1 ] || continue >> + >> + has_soft_rdma "$i" && break >> + [ "$operation" = "setup" ] && >> + rdma link add "${i}_rxe" type rxe netdev "$i" && break >> + done >> + has_soft_rdma "$i" || return >> + get_ipv4_addr "$i" >> + ) >> +} >> + >> +operation=${1:-setup} >> + >> +if [ "$operation" == "setup" ] || [ "$operation" == "detect" ]; then >> + rdma_rxe_setup_detect >> +else >> + echo "Usage: $0 [setup | detect]" >> +fi > > What happened to the cleanup option? I think I missed some discussion on > this... We can't expect people to know how to clean this up without any > hint. Nothing special, one reason could be to keep it as simple as possible in the beginning. I'm fine to add it back. > >> diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c >> index ba273d10b9a..bf97f4e9325 100644 >> --- a/tests/qtest/migration/precopy-tests.c >> +++ b/tests/qtest/migration/precopy-tests.c >> @@ -99,6 +99,66 @@ static void test_precopy_unix_dirty_ring(void) >> test_precopy_common(&args); >> } >> >> +#ifdef CONFIG_RDMA >> + >> +#define RDMA_MIGRATION_HELPER "scripts/rdma-migration-helper.sh" >> +static int new_rdma_link(char *buffer) >> +{ >> + const char *argument = (geteuid() == 0) ? "setup" : "detect"; >> + char cmd[1024]; >> + >> + snprintf(cmd, sizeof(cmd), "%s %s", RDMA_MIGRATION_HELPER, argument); >> + >> + FILE *pipe = popen(cmd, "r"); > > This needs to be silenced, otherwise messages from the script will break > TAP output. I suggest: > > bool verbose = g_getenv("QTEST_LOG"); > > snprintf(cmd, sizeof(cmd), "%s %s %s", RDMA_MIGRATION_HELPER, argument, > verbose ? "" : "2>/dev/null"); > It sound good to me, i will update it. >> + if (pipe == NULL) { >> + perror("Failed to run script"); >> + return -1; >> + } >> + >> + int idx = 0; >> + while (fgets(buffer + idx, 128 - idx, pipe) != NULL) { >> + idx += strlen(buffer); >> + } >> + >> + int status = pclose(pipe); >> + if (status == -1) { >> + perror("Error reported by pclose()"); >> + return -1; >> + } else if (WIFEXITED(status)) { >> + return WEXITSTATUS(status); >> + } >> + >> + return -1; >> +} >> + >> +static void test_precopy_rdma_plain(void) >> +{ >> + char buffer[128] = {}; >> + >> + if (new_rdma_link(buffer)) { >> + g_test_skip("\nThere is no available rdma link to run RDMA migration test.\n" >> + "To enable the test:\n" >> + "(1) Run \'" RDMA_MIGRATION_HELPER " setup\' with root and rerun the test\n" >> + "or\n" >> + "(2) Run the test with root privilege\n"); > > g_test_skip() needs a one-line message, otherwise it breaks TAP > output. You can turn this into a g_test_message(), put it under > QTEST_LOG=1 and add a g_test_skip("no rdma link available") below. Ditto. Thanks Zhijian
diff --git a/MAINTAINERS b/MAINTAINERS index 3848d37a38d..15360fcdc4b 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3480,6 +3480,7 @@ R: Li Zhijian <lizhijian@fujitsu.com> R: Peter Xu <peterx@redhat.com> S: Odd Fixes F: migration/rdma* +F: scripts/rdma-migration-helper.sh Migration dirty limit and dirty page rate M: Hyman Huang <yong.huang@smartx.com> diff --git a/scripts/rdma-migration-helper.sh b/scripts/rdma-migration-helper.sh new file mode 100755 index 00000000000..66557d9e267 --- /dev/null +++ b/scripts/rdma-migration-helper.sh @@ -0,0 +1,41 @@ +#!/bin/bash + +# Copied from blktests +get_ipv4_addr() +{ + ip -4 -o addr show dev "$1" | + sed -n 's/.*[[:blank:]]inet[[:blank:]]*\([^[:blank:]/]*\).*/\1/p' | + tr -d '\n' +} + +has_soft_rdma() +{ + rdma link | grep -q " netdev $1[[:blank:]]*\$" +} + +rdma_rxe_setup_detect() +{ + ( + cd /sys/class/net && + for i in *; do + [ -e "$i" ] || continue + [ "$i" = "lo" ] && continue + [ "$(<"$i/addr_len")" = 6 ] || continue + [ "$(<"$i/carrier")" = 1 ] || continue + + has_soft_rdma "$i" && break + [ "$operation" = "setup" ] && + rdma link add "${i}_rxe" type rxe netdev "$i" && break + done + has_soft_rdma "$i" || return + get_ipv4_addr "$i" + ) +} + +operation=${1:-setup} + +if [ "$operation" == "setup" ] || [ "$operation" == "detect" ]; then + rdma_rxe_setup_detect +else + echo "Usage: $0 [setup | detect]" +fi diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c index ba273d10b9a..bf97f4e9325 100644 --- a/tests/qtest/migration/precopy-tests.c +++ b/tests/qtest/migration/precopy-tests.c @@ -99,6 +99,66 @@ static void test_precopy_unix_dirty_ring(void) test_precopy_common(&args); } +#ifdef CONFIG_RDMA + +#define RDMA_MIGRATION_HELPER "scripts/rdma-migration-helper.sh" +static int new_rdma_link(char *buffer) +{ + const char *argument = (geteuid() == 0) ? "setup" : "detect"; + char cmd[1024]; + + snprintf(cmd, sizeof(cmd), "%s %s", RDMA_MIGRATION_HELPER, argument); + + FILE *pipe = popen(cmd, "r"); + if (pipe == NULL) { + perror("Failed to run script"); + return -1; + } + + int idx = 0; + while (fgets(buffer + idx, 128 - idx, pipe) != NULL) { + idx += strlen(buffer); + } + + int status = pclose(pipe); + if (status == -1) { + perror("Error reported by pclose()"); + return -1; + } else if (WIFEXITED(status)) { + return WEXITSTATUS(status); + } + + return -1; +} + +static void test_precopy_rdma_plain(void) +{ + char buffer[128] = {}; + + if (new_rdma_link(buffer)) { + g_test_skip("\nThere is no available rdma link to run RDMA migration test.\n" + "To enable the test:\n" + "(1) Run \'" RDMA_MIGRATION_HELPER " setup\' with root and rerun the test\n" + "or\n" + "(2) Run the test with root privilege\n"); + return; + } + + /* + * TODO: query a free port instead of hard code. + * 29200=('R'+'D'+'M'+'A')*100 + **/ + g_autofree char *uri = g_strdup_printf("rdma:%s:29200", buffer); + + MigrateCommon args = { + .listen_uri = uri, + .connect_uri = uri, + }; + + test_precopy_common(&args); +} +#endif + static void test_precopy_tcp_plain(void) { MigrateCommon args = { @@ -1124,6 +1184,10 @@ static void migration_test_add_precopy_smoke(MigrationTestEnv *env) test_multifd_tcp_uri_none); migration_test_add("/migration/multifd/tcp/plain/cancel", test_multifd_tcp_cancel); +#ifdef CONFIG_RDMA + migration_test_add("/migration/precopy/rdma/plain", + test_precopy_rdma_plain); +#endif } void migration_test_add_precopy(MigrationTestEnv *env)