Re: [PATCH 2/2] [NOT-FOR-MERGE] Add qtest for migration over RDMA

[Peter Xu <peterx@redhat.com>]

On Tue, Feb 18, 2025 at 06:03:48PM -0300, Fabiano Rosas wrote:
> Li Zhijian via <qemu-devel@nongnu.org> writes:
> 
> > This qtest requirs there is RXE link in the host.
> >
> > Here is an example to show how to add this RXE link:
> > $ ./new-rdma-link.sh
> > 192.168.22.93
> >
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > ---
> > The RDMA migration was broken again...due to lack of sufficient test/qtest.
> >
> > It's urgly to add and execute a script to establish an RDMA link in
> > the C program. If anyone has a better suggestion, please let me know.
> >
> > $ cat ./new-rdma-link.sh
> > get_ipv4_addr() {
> >         ip -4 -o addr show dev "$1" |
> >                 sed -n 's/.*[[:blank:]]inet[[:blank:]]*\([^[:blank:]/]*\).*/\1/p'
> > }
> >
> > has_soft_rdma() {
> >         rdma link | grep -q " netdev $1[[:blank:]]*\$"
> > }
> >
> > start_soft_rdma() {
> >         local type
> >
> >         modprobe rdma_rxe || return $?
> >         type=rxe
> >         (
> >                 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
> >                                 rdma link add "${i}_$type" type $type netdev "$i" && break
> >                         done
> >                 has_soft_rdma "$i" && echo $i
> >         )
> >
> > }
> >
> > rxe_link=$(start_soft_rdma)
> > [[ "$rxe_link" ]] && get_ipv4_addr $rxe_link
> >
> > Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> > ---
> >  tests/qtest/migration/new-rdma-link.sh |  34 ++++++++
> >  tests/qtest/migration/precopy-tests.c  | 103 +++++++++++++++++++++++++
> >  2 files changed, 137 insertions(+)
> >  create mode 100644 tests/qtest/migration/new-rdma-link.sh
> >
> > diff --git a/tests/qtest/migration/new-rdma-link.sh b/tests/qtest/migration/new-rdma-link.sh
> > new file mode 100644
> > index 00000000000..ca20594eaae
> > --- /dev/null
> > +++ b/tests/qtest/migration/new-rdma-link.sh
> > @@ -0,0 +1,34 @@
> > +#!/bin/bash
> > +
> > +# Copied from blktests
> > +get_ipv4_addr() {
> > +	ip -4 -o addr show dev "$1" |
> > +		sed -n 's/.*[[:blank:]]inet[[:blank:]]*\([^[:blank:]/]*\).*/\1/p'
> > +}
> > +
> > +has_soft_rdma() {
> > +	rdma link | grep -q " netdev $1[[:blank:]]*\$"
> > +}
> > +
> > +start_soft_rdma() {
> > +	local type
> > +
> > +	modprobe rdma_rxe || return $?
> > +	type=rxe
> > +	(
> > +		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
> > +				rdma link add "${i}_$type" type $type netdev "$i" && break
> > +			done
> > +		has_soft_rdma "$i" && echo $i
> > +	)
> > +
> > +}
> > +
> > +rxe_link=$(start_soft_rdma)
> > +[[ "$rxe_link" ]] && get_ipv4_addr $rxe_link
> > diff --git a/tests/qtest/migration/precopy-tests.c b/tests/qtest/migration/precopy-tests.c
> > index 162fa695318..d2a1c9c9438 100644
> > --- a/tests/qtest/migration/precopy-tests.c
> > +++ b/tests/qtest/migration/precopy-tests.c
> > @@ -98,6 +98,105 @@ static void test_precopy_unix_dirty_ring(void)
> >      test_precopy_common(&args);
> >  }
> >  
> > +static int new_rdma_link(char *buffer) {
> > +    // Copied from blktests
> > +    const char *script =
> > +        "#!/bin/bash\n"
> > +        "\n"
> > +        "get_ipv4_addr() {\n"
> > +        "    ip -4 -o addr show dev \"$1\" |\n"
> > +        "    sed -n 's/.*[[:blank:]]inet[[:blank:]]*\\([^[:blank:]/]*\\).*/\\1/p'\n"
> > +        "}\n"
> > +        "\n"
> > +        "has_soft_rdma() {\n"
> > +        "    rdma link | grep -q \" netdev $1[[:blank:]]*\\$\"\n"
> > +        "}\n"
> > +        "\n"
> > +        "start_soft_rdma() {\n"
> > +        "    local type\n"
> > +        "\n"
> > +        "    modprobe rdma_rxe || return $?\n"
> > +        "    type=rxe\n"
> > +        "    (\n"
> > +        "        cd /sys/class/net &&\n"
> > +        "        for i in *; do\n"
> > +        "            [ -e \"$i\" ] || continue\n"
> > +        "            [ \"$i\" = \"lo\" ] && continue\n"
> > +        "            [ \"$(<$i/addr_len)\" = 6 ] || continue\n"
> > +        "            [ \"$(<$i/carrier)\" = 1 ] || continue\n"
> > +        "            has_soft_rdma \"$i\" && break\n"
> > +        "            rdma link add \"${i}_$type\" type $type netdev \"$i\" && break\n"
> > +        "        done\n"
> > +        "        has_soft_rdma \"$i\" && echo $i\n"
> > +        "    )\n"
> > +        "}\n"
> > +        "\n"
> > +        "rxe_link=$(start_soft_rdma)\n"
> > +        "[[ \"$rxe_link\" ]] && get_ipv4_addr $rxe_link\n";
> > +
> > +    char script_filename[] = "/tmp/temp_scriptXXXXXX";
> > +    int fd = mkstemp(script_filename);
> > +    if (fd == -1) {
> > +        perror("Failed to create temporary file");
> > +        return 1;
> > +    }
> > +
> > +    FILE *fp = fdopen(fd, "w");
> > +    if (fp == NULL) {
> > +        perror("Failed to open file stream");
> > +        close(fd);
> > +        return 1;
> > +    }
> > +    fprintf(fp, "%s", script);
> > +    fclose(fp);
> > +
> > +    if (chmod(script_filename, 0700) == -1) {
> > +        perror("Failed to set execute permission");
> > +        return 1;
> > +    }
> > +
> > +    FILE *pipe = popen(script_filename, "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);
> > +    }
> > +    if (buffer[idx - 1] == '\n')
> > +        buffer[idx - 1] = 0;
> > +
> > +    int status = pclose(pipe);
> > +    if (status == -1) {
> > +        perror("Error reported by pclose()");
> > +    } else if (!WIFEXITED(status)) {
> > +        printf("Script did not terminate normally\n");
> > +    }
> > +
> > +    remove(script_filename);

The script can be put separately instead if hard-coded here, right?

> > +
> > +    return 0;
> > +}
> > +
> > +static void test_precopy_rdma_plain(void)
> > +{
> > +    char buffer[128] = {};
> > +
> > +    if (new_rdma_link(buffer))
> > +        return;
> > +
> > +    g_autofree char *uri = g_strdup_printf("rdma:%s:7777", buffer);
> > +
> > +    MigrateCommon args = {
> > +        .listen_uri = uri,
> > +        .connect_uri = uri,
> > +    };
> > +
> > +    test_precopy_common(&args);
> > +}
> > +
> >  static void test_precopy_tcp_plain(void)
> >  {
> >      MigrateCommon args = {
> > @@ -968,6 +1067,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)
> 
> Thanks, that's definitely better than nothing. I'll experiment with this
> locally, see if I can at least run it before sending a pull request.

With your newly added --full, IIUC we can add whatever we want there.
E.g. we can add --rdma and iff specified, migration-test adds the rdma test.

Or.. skip the test when the rdma link isn't available.

If we could separate the script into a file, it'll be better.  We could
create scripts/migration dir and put all migration scripts over there, then
in the test it tries to detect rdma link and fetch the ip only (aka, we'd
better keep the test itself not rely on root permission..).

-- 
Peter Xu