[PULL 0/7] Migration.next patches

Juan Quintela posted 7 patches 2 years, 7 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210909103346.1990-1-quintela@redhat.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Juan Quintela <quintela@redhat.com>
There is a newer version of this series
migration/multifd.h    |   4 ++
migration/migration.c  |  12 +++++
migration/multifd.c    |  35 ++++++++++---
migration/ram.c        |  13 ++---
migration/rdma.c       | 111 ++++++++++++++++++++++++++++++++++-------
migration/trace-events |   2 +
6 files changed, 143 insertions(+), 34 deletions(-)
[PULL 0/7] Migration.next patches
Posted by Juan Quintela 2 years, 7 months ago
The following changes since commit bd662023e683850c085e98c8ff8297142c2dd9f2:

  Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-20210908' into staging (2021-09-08 11:06:17 +0100)

are available in the Git repository at:

  https://github.com/juanquintela/qemu.git tags/migration.next-pull-request

for you to fetch changes up to 158cced72cb2b09b0e8b523a5b15cb10889f99d1:

  migration: allow enabling mutilfd for specific protocol only (2021-09-09 09:30:55 +0200)

----------------------------------------------------------------
Migration Pull request

This pull request includes:
- Remove RAMState unused parameter for several prototypes
- RDMA fix
- give an error when using RDMA and multifd
- Implement yank for multifd send side

Please, Apply.

----------------------------------------------------------------

David Hildenbrand (1):
  migration/ram: Don't passs RAMState to
    migration_clear_memory_region_dirty_bitmap_*()

Li Zhijian (4):
  migration/rdma: Try to register On-Demand Paging memory region
  migration/rdma: advise prefetch write for ODP region
  migration: allow multifd for socket protocol only
  migration: allow enabling mutilfd for specific protocol only

Lukas Straub (2):
  multifd: Implement yank for multifd send side
  multifd: Unconditionally unregister yank function

 migration/multifd.h    |   4 ++
 migration/migration.c  |  12 +++++
 migration/multifd.c    |  35 ++++++++++---
 migration/ram.c        |  13 ++---
 migration/rdma.c       | 111 ++++++++++++++++++++++++++++++++++-------
 migration/trace-events |   2 +
 6 files changed, 143 insertions(+), 34 deletions(-)

-- 
2.31.1



Re: [PULL 0/7] Migration.next patches
Posted by Peter Maydell 2 years, 7 months ago
On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>
> The following changes since commit bd662023e683850c085e98c8ff8297142c2dd9f2:
>
>   Merge remote-tracking branch 'remotes/mcayland/tags/qemu-openbios-20210908' into staging (2021-09-08 11:06:17 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/juanquintela/qemu.git tags/migration.next-pull-request
>
> for you to fetch changes up to 158cced72cb2b09b0e8b523a5b15cb10889f99d1:
>
>   migration: allow enabling mutilfd for specific protocol only (2021-09-09 09:30:55 +0200)
>
> ----------------------------------------------------------------
> Migration Pull request
>
> This pull request includes:
> - Remove RAMState unused parameter for several prototypes
> - RDMA fix
> - give an error when using RDMA and multifd
> - Implement yank for multifd send side

Fails to build, FreeBSD:

../src/migration/rdma.c:1146:23: error: use of undeclared identifier
'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
    int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
                      ^
../src/migration/rdma.c:1147:18: error: use of undeclared identifier
'IBV_ADVISE_MR_ADVICE_PREFETCH'
                 IBV_ADVISE_MR_ADVICE_PREFETCH;
                 ^
../src/migration/rdma.c:1150:11: warning: implicit declaration of
function 'ibv_advise_mr' is invalid in C99
[-Wimplicit-function-declaration]
    ret = ibv_advise_mr(pd, advice,
          ^
../src/migration/rdma.c:1151:25: error: use of undeclared identifier
'IBV_ADVISE_MR_FLAG_FLUSH'
                        IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
                        ^


-- PMM

Re: [PULL 0/7] Migration.next patches
Posted by Li, Zhijian 2 years, 7 months ago
on 2021/9/9 21:42, Peter Maydell wrote:
> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
> Fails to build, FreeBSD:
>
> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>                        ^
> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>                   IBV_ADVISE_MR_ADVICE_PREFETCH;
>                   ^
> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
> function 'ibv_advise_mr' is invalid in C99
> [-Wimplicit-function-declaration]
>      ret = ibv_advise_mr(pd, advice,
>            ^
> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
> 'IBV_ADVISE_MR_FLAG_FLUSH'
>                          IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>                          ^
>
it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only

Thanks
zhijian


> -- PMM
>



Re: [PULL 0/7] Migration.next patches
Posted by Peter Maydell 2 years, 7 months ago
On Thu, 9 Sept 2021 at 15:49, Li, Zhijian <lizhijian@cn.fujitsu.com> wrote:
>
> on 2021/9/9 21:42, Peter Maydell wrote:
> > On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
> > Fails to build, FreeBSD:
> >
> > ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
> > 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
> >      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
> >                        ^
> > ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
> > 'IBV_ADVISE_MR_ADVICE_PREFETCH'
> >                   IBV_ADVISE_MR_ADVICE_PREFETCH;
> >                   ^
> > ../src/migration/rdma.c:1150:11: warning: implicit declaration of
> > function 'ibv_advise_mr' is invalid in C99
> > [-Wimplicit-function-declaration]
> >      ret = ibv_advise_mr(pd, advice,
> >            ^
> > ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
> > 'IBV_ADVISE_MR_FLAG_FLUSH'
> >                          IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
> >                          ^
> >
> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only

It is only FreeBSD, but usually looking at the OS is the wrong thing.
Is this some new API that's only present in some versions of rdma?
If so, a check on "does the library have this feature" somehow is probably
what you want.

thanks
-- PMM

Re: [PULL 0/7] Migration.next patches
Posted by Juan Quintela 2 years, 7 months ago
"Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
> on 2021/9/9 21:42, Peter Maydell wrote:
>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>> Fails to build, FreeBSD:
>>
>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>                        ^
>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>                   IBV_ADVISE_MR_ADVICE_PREFETCH;
>>                   ^
>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>> function 'ibv_advise_mr' is invalid in C99
>> [-Wimplicit-function-declaration]
>>      ret = ibv_advise_mr(pd, advice,
>>            ^
>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>                          IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>                          ^
>>
> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only

The other way around I think.
What about:

It compiled on my Linux machine, but it *should* work on any (famous
last words).  Finishing compilation 

Later, Juan.

From 964e436bdb8aef7dbebc28415e4ac3c5822b552e Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Thu, 9 Sep 2021 17:07:17 +0200
Subject: [PATCH] rdma: test for ibv_advise_mr API

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 configure        | 28 ++++++++++++++++++++++++++++
 migration/rdma.c |  2 ++
 2 files changed, 30 insertions(+)

diff --git a/configure b/configure
index 8adf2127c3..70054fd702 100755
--- a/configure
+++ b/configure
@@ -339,6 +339,7 @@ whpx="auto"
 nvmm="auto"
 rdma="$default_feature"
 pvrdma="$default_feature"
+rdma_ibv_advise_mr="no"
 gprof="no"
 debug_tcg="no"
 debug="no"
@@ -2918,6 +2919,29 @@ EOF
     fi
 fi
 
+# Let's see if enhanced reg_mr is supported
+if test "$rdma" = "yes" ; then
+
+cat > $TMPC <<EOF &&
+#include <infiniband/verbs.h>
+
+int
+main(void)
+{
+    int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
+                 IBV_ADVISE_MR_ADVICE_PREFETCH;
+    struct ibv_sge sg_list = {.lkey = lkey, .addr = addr, .length = len};
+
+    ibv_advise_mr(pd, advice, IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
+    return 0;
+}
+EOF
+    if ! compile_prog "" "-libverbs"; then
+	rdma_ibv_advise_mr="yes"
+    fi
+fi
+
+
 ##########################################
 # xfsctl() probe, used for file-posix.c
 if test "$xfs" != "no" ; then
@@ -4802,6 +4826,10 @@ if test "$rdma" = "yes" ; then
   echo "RDMA_LIBS=$rdma_libs" >> $config_host_mak
 fi
 
+if test "$rdma_ibv_advise_mr" = "yes"; then
+  echo "CONFIG_IBV_ADVISE_MR=y" >> $config_host_mak
+fi
+
 if test "$pvrdma" = "yes" ; then
   echo "CONFIG_PVRDMA=y" >> $config_host_mak
 fi
diff --git a/migration/rdma.c b/migration/rdma.c
index 6c2cc3f617..aac343253f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1142,6 +1142,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
                                          uint32_t len,  uint32_t lkey,
                                          const char *name, bool wr)
 {
+#if CONFIG_IBV_ADVISE_MR
     int ret;
     int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
                  IBV_ADVISE_MR_ADVICE_PREFETCH;
@@ -1155,6 +1156,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
     } else {
         trace_qemu_rdma_advise_mr(name, len, addr, "successed");
     }
+#endif
 }
 
 static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
-- 
2.31.1


Re: [PULL 0/7] Migration.next patches
Posted by Peter Maydell 2 years, 7 months ago
On Thu, 9 Sept 2021 at 16:23, Juan Quintela <quintela@redhat.com> wrote:
> From 964e436bdb8aef7dbebc28415e4ac3c5822b552e Mon Sep 17 00:00:00 2001
> From: Juan Quintela <quintela@redhat.com>
> Date: Thu, 9 Sep 2021 17:07:17 +0200
> Subject: [PATCH] rdma: test for ibv_advise_mr API
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  configure        | 28 ++++++++++++++++++++++++++++
>  migration/rdma.c |  2 ++
>  2 files changed, 30 insertions(+)
>
> diff --git a/configure b/configure
> index 8adf2127c3..70054fd702 100755
> --- a/configure
> +++ b/configure
> @@ -339,6 +339,7 @@ whpx="auto"
>  nvmm="auto"
>  rdma="$default_feature"
>  pvrdma="$default_feature"
> +rdma_ibv_advise_mr="no"
>  gprof="no"
>  debug_tcg="no"
>  debug="no"
> @@ -2918,6 +2919,29 @@ EOF
>      fi
>  fi
>
> +# Let's see if enhanced reg_mr is supported
> +if test "$rdma" = "yes" ; then
> +
> +cat > $TMPC <<EOF &&
> +#include <infiniband/verbs.h>
> +
> +int
> +main(void)
> +{
> +    int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
> +                 IBV_ADVISE_MR_ADVICE_PREFETCH;
> +    struct ibv_sge sg_list = {.lkey = lkey, .addr = addr, .length = len};
> +
> +    ibv_advise_mr(pd, advice, IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
> +    return 0;
> +}
> +EOF
> +    if ! compile_prog "" "-libverbs"; then
> +       rdma_ibv_advise_mr="yes"
> +    fi
> +fi

We don't really want new compilation tests in configure:
anything new should be done in meson.build. I think Paolo
has work in progress to migrate the remaining configure tests.
Something like

config_host_data.set('HAVE_IBV_ADVICE_MR',
cc.has_function('ibv_advise_mr', dependencies: rdma))

in the same bit of meson.build as the other has_function tests
is probably sufficient.

thanks
-- PMM

Re: [PULL 0/7] Migration.next patches
Posted by Juan Quintela 2 years, 7 months ago
"Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
> on 2021/9/9 21:42, Peter Maydell wrote:
>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>> Fails to build, FreeBSD:
>>
>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>                        ^
>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>                   IBV_ADVISE_MR_ADVICE_PREFETCH;
>>                   ^
>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>> function 'ibv_advise_mr' is invalid in C99
>> [-Wimplicit-function-declaration]
>>      ret = ibv_advise_mr(pd, advice,
>>            ^
>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>                          IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>                          ^
>>
> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only

Second try.  I can't see an example where they search for:
a symbol on the header file
  and
a function in a library

so I assume that if you have the symbols, you have the function.

How do you see it?

Trying to compile it on vm-build-freebsd, but not being very sucessfull
so far.

Later, Juan.

From e954c1e0afc785a98d472201dafe75a7e7126b1d Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Thu, 9 Sep 2021 17:07:17 +0200
Subject: [PATCH] rdma: test for ibv_advise_mr API

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 meson.build      | 3 +++
 migration/rdma.c | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/meson.build b/meson.build
index 7e58e6279b..c2eb437df4 100644
--- a/meson.build
+++ b/meson.build
@@ -1375,6 +1375,9 @@ config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
 config_host_data.set('HAVE_STRUCT_STAT_ST_ATIM',
                      cc.has_member('struct stat', 'st_atim',
                                    prefix: '#include <sys/stat.h>'))
+config_host_data.set('CONFIG_RDMA_IBV_ADVISE_MR',
+                     cc.has_header_symbol('infiniband/verbs.h', 'IBV_ADVISE_MR_ADVICE_PREFETCH') and
+                     cc.has_header_symbol('infiniband/verbs.h', 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'))
 
 config_host_data.set('CONFIG_EVENTFD', cc.links('''
   #include <sys/eventfd.h>
diff --git a/migration/rdma.c b/migration/rdma.c
index 6c2cc3f617..f0d78597fb 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1142,6 +1142,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
                                          uint32_t len,  uint32_t lkey,
                                          const char *name, bool wr)
 {
+#ifdef CONFIG_RDMA_IBV_ADVISE_MR
     int ret;
     int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
                  IBV_ADVISE_MR_ADVICE_PREFETCH;
@@ -1155,6 +1156,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
     } else {
         trace_qemu_rdma_advise_mr(name, len, addr, "successed");
     }
+#endif
 }
 
 static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
-- 
2.31.1


Re: [PULL 0/7] Migration.next patches
Posted by lizhijian@fujitsu.com 2 years, 7 months ago

On 10/09/2021 00:10, Juan Quintela wrote:
> "Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
>> on 2021/9/9 21:42, Peter Maydell wrote:
>>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>>> Fails to build, FreeBSD:
>>>
>>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>>                         ^
>>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
>>>                    ^
>>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>>> function 'ibv_advise_mr' is invalid in C99
>>> [-Wimplicit-function-declaration]
>>>       ret = ibv_advise_mr(pd, advice,
>>>             ^
>>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>>                           IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>>                           ^
>>>
>> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
>> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
>> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only
> Second try.  I can't see an example where they search for:
> a symbol on the header file
>    and
> a function in a library
>
> so I assume that if you have the symbols, you have the function.
>
> How do you see it?
>
> Trying to compile it on vm-build-freebsd, but not being very sucessfull
> so far.

Your patch does work! But i still followed PMM's suggestion, converted it to has_function
as another option.
I have verified it on FreeBSD and Linux.

 From 67f386acc2092ecf6e71b8951b6af5d5b8366f80 Mon Sep 17 00:00:00 2001
From: Juan Quintela <quintela@redhat.com>
Date: Thu, 9 Sep 2021 17:07:17 +0200
Subject: [PATCH] rdma: test for ibv_advise_mr API

Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
---
  meson.build      | 6 ++++++
  migration/rdma.c | 2 ++
  2 files changed, 8 insertions(+)

diff --git a/meson.build b/meson.build
index 6e4d2d80343..97406d1b79b 100644
--- a/meson.build
+++ b/meson.build
@@ -1328,6 +1328,12 @@ config_host_data.set('HAVE_COPY_FILE_RANGE', cc.has_function('copy_file_range'))
  config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
  config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
  config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
+if rdma.found()
+  config_host_data.set('HAVE_IBV_ADVISE_MR',
+                       cc.has_function('ibv_advise_mr',
+                                       args: config_host['RDMA_LIBS'].split(),
+                                       prefix: '#include <infiniband/verbs.h>'))
+endif
  
  # has_header_symbol
  config_host_data.set('CONFIG_BYTESWAP_H',
diff --git a/migration/rdma.c b/migration/rdma.c
index 6c2cc3f617c..2a3c7889b9f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -1142,6 +1142,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
                                           uint32_t len,  uint32_t lkey,
                                           const char *name, bool wr)
  {
+#ifdef HAVE_IBV_ADVISE_MR
      int ret;
      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
                   IBV_ADVISE_MR_ADVICE_PREFETCH;
@@ -1155,6 +1156,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
      } else {
          trace_qemu_rdma_advise_mr(name, len, addr, "successed");
      }
+#endif
  }
  
  static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
-- 
2.31.1




> Later, Juan.
>
>  From e954c1e0afc785a98d472201dafe75a7e7126b1d Mon Sep 17 00:00:00 2001
> From: Juan Quintela <quintela@redhat.com>
> Date: Thu, 9 Sep 2021 17:07:17 +0200
> Subject: [PATCH] rdma: test for ibv_advise_mr API
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>   meson.build      | 3 +++
>   migration/rdma.c | 2 ++
>   2 files changed, 5 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 7e58e6279b..c2eb437df4 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1375,6 +1375,9 @@ config_host_data.set('HAVE_SIGEV_NOTIFY_THREAD_ID',
>   config_host_data.set('HAVE_STRUCT_STAT_ST_ATIM',
>                        cc.has_member('struct stat', 'st_atim',
>                                      prefix: '#include <sys/stat.h>'))
> +config_host_data.set('CONFIG_RDMA_IBV_ADVISE_MR',
> +                     cc.has_header_symbol('infiniband/verbs.h', 'IBV_ADVISE_MR_ADVICE_PREFETCH') and
> +                     cc.has_header_symbol('infiniband/verbs.h', 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'))
>   
>   config_host_data.set('CONFIG_EVENTFD', cc.links('''
>     #include <sys/eventfd.h>
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6c2cc3f617..f0d78597fb 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1142,6 +1142,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
>                                            uint32_t len,  uint32_t lkey,
>                                            const char *name, bool wr)
>   {
> +#ifdef CONFIG_RDMA_IBV_ADVISE_MR
>       int ret;
>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
> @@ -1155,6 +1156,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
>       } else {
>           trace_qemu_rdma_advise_mr(name, len, addr, "successed");
>       }
> +#endif
>   }
>   
>   static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
Re: [PULL 0/7] Migration.next patches
Posted by lizhijian@fujitsu.com 2 years, 7 months ago

On 10/09/2021 13:20, Li Zhijian wrote:
>
>
> On 10/09/2021 00:10, Juan Quintela wrote:
>> "Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
>>> on 2021/9/9 21:42, Peter Maydell wrote:
>>>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>>>> Fails to build, FreeBSD:
>>>>
>>>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>>>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>>>                         ^
>>>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>>>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
>>>>                    ^
>>>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>>>> function 'ibv_advise_mr' is invalid in C99
>>>> [-Wimplicit-function-declaration]
>>>>       ret = ibv_advise_mr(pd, advice,
>>>>             ^
>>>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>>>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>>>                           IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>>>                           ^
>>>>
>>> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
>>> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
>>> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only
>> Second try.  I can't see an example where they search for:
>> a symbol on the header file
>>    and
>> a function in a library
>>
>> so I assume that if you have the symbols, you have the function.
>>
>> How do you see it?
>>
>> Trying to compile it on vm-build-freebsd, but not being very sucessfull
>> so far.

BTW: Does QEMU provide any mean to set http(s)_proxy to building vm ? Currently, i have to
hack the code like:

-        self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
+        self.ssh_root_check("setenv HTTP_PROXY http://myproxy; setenv HTTPS_PROXY http://myproxy; pkg install -y %s\n" % " ".join(self.pkgs))


Thanks
Zhijian


>
> Your patch does work! But i still followed PMM's suggestion, converted it to has_function
> as another option.
> I have verified it on FreeBSD and Linux.
>
> From 67f386acc2092ecf6e71b8951b6af5d5b8366f80 Mon Sep 17 00:00:00 2001
> From: Juan Quintela <quintela@redhat.com>
> Date: Thu, 9 Sep 2021 17:07:17 +0200
> Subject: [PATCH] rdma: test for ibv_advise_mr API
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com>
> ---
>  meson.build      | 6 ++++++
>  migration/rdma.c | 2 ++
>  2 files changed, 8 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index 6e4d2d80343..97406d1b79b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1328,6 +1328,12 @@ config_host_data.set('HAVE_COPY_FILE_RANGE', cc.has_function('copy_file_range'))
>  config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
>  config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
>  config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
> +if rdma.found()
> +  config_host_data.set('HAVE_IBV_ADVISE_MR',
> +                       cc.has_function('ibv_advise_mr',
> +                                       args: config_host['RDMA_LIBS'].split(),
> +                                       prefix: '#include <infiniband/verbs.h>'))
> +endif
>
>  # has_header_symbol
>  config_host_data.set('CONFIG_BYTESWAP_H',
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 6c2cc3f617c..2a3c7889b9f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1142,6 +1142,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
>                                           uint32_t len,  uint32_t lkey,
>                                           const char *name, bool wr)
>  {
> +#ifdef HAVE_IBV_ADVISE_MR
>      int ret;
>      int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>                   IBV_ADVISE_MR_ADVICE_PREFETCH;
> @@ -1155,6 +1156,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
>      } else {
>          trace_qemu_rdma_advise_mr(name, len, addr, "successed");
>      }
> +#endif
>  }
>
>  static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma)
Re: [PULL 0/7] Migration.next patches
Posted by Juan Quintela 2 years, 7 months ago
"lizhijian@fujitsu.com" <lizhijian@fujitsu.com> wrote:
> On 10/09/2021 13:20, Li Zhijian wrote:
>>
>>
>> On 10/09/2021 00:10, Juan Quintela wrote:
>>> "Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
>>>> on 2021/9/9 21:42, Peter Maydell wrote:
>>>>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>>>>> Fails to build, FreeBSD:
>>>>>
>>>>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>>>>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>>>>                         ^
>>>>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>>>>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
>>>>>                    ^
>>>>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>>>>> function 'ibv_advise_mr' is invalid in C99
>>>>> [-Wimplicit-function-declaration]
>>>>>       ret = ibv_advise_mr(pd, advice,
>>>>>             ^
>>>>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>>>>                           IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>>>>                           ^
>>>>>
>>>> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
>>>> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
>>>> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only
>>> Second try.  I can't see an example where they search for:
>>> a symbol on the header file
>>>    and
>>> a function in a library
>>>
>>> so I assume that if you have the symbols, you have the function.
>>>
>>> How do you see it?
>>>
>>> Trying to compile it on vm-build-freebsd, but not being very sucessfull
>>> so far.
>
> BTW: Does QEMU provide any mean to set http(s)_proxy to building vm ? Currently, i have to
> hack the code like:
>
> -        self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
> + self.ssh_root_check("setenv HTTP_PROXY http://myproxy; setenv
> HTTPS_PROXY http://myproxy; pkg install -y %s\n" % "
> ".join(self.pkgs))

Dunno.  I don't need http proxy, for me what fails is the "tar" stage.

(master)$ make HOME=/scratch/tmp/ vm-build-fedora
    VM-BUILD fedora 
tar: Skipping to next header
tar: Exiting with failure status due to previous errors
failed append submodule slirp to /mnt/code/qemu/full/vm-test-eqebvy5r.tmp/data-3a52c.tar
Failed to prepare guest environment
Traceback (most recent call last):
  File "/mnt/code/qemu/full/tests/vm/basevm.py", line 636, in main
    vm.add_source_dir(args.build_qemu)
  File "/mnt/code/qemu/full/tests/vm/basevm.py", line 270, in add_source_dir
    subprocess.check_call(["./scripts/archive-source.sh", tarfile],
  File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['./scripts/archive-source.sh', '/mnt/code/qemu/full/vm-test-eqebvy5r.tmp/data-3a52c.tar']' returned non-zero exit status 1.
make: *** [tests/vm/Makefile.include:96: vm-build-fedora] Error 2

forget about the HOME= change, it also fails if I don't use it.  And the
problem is the archive bits.


>>>> on 2021/9/9 21:42, Peter Maydell wrote:
>>>>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>>>>> Fails to build, FreeBSD:
>>>>>
>>>>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>>>>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>>>>                         ^
>>>>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>>>>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
>>>>>                    ^
>>>>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>>>>> function 'ibv_advise_mr' is invalid in C99
>>>>> [-Wimplicit-function-declaration]
>>>>>       ret = ibv_advise_mr(pd, advice,
>>>>>             ^
>>>>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>>>>                           IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>>>>                           ^
>>>>>
>>>> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
>>>> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
>>>> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only
>>> Second try.  I can't see an example where they search for:
>>> a symbol on the header file
>>>    and
>>> a function in a library
>>>
>>> so I assume that if you have the symbols, you have the function.
>>>
>>> How do you see it?
>>>
>>> Trying to compile it on vm-build-freebsd, but not being very sucessfull
>>> so far.
>
> BTW: Does QEMU provide any mean to set http(s)_proxy to building vm ? Currently, i have to
> hack the code like:
>
> -        self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
> + self.ssh_root_check("setenv HTTP_PROXY http://myproxy; setenv
> HTTPS_PROXY http://myproxy; pkg install -y %s\n" % "
> ".join(self.pkgs))

Dunno.  I don't need http proxy, for me what fails is the "tar" stage.

(master)$ make HOME=/scratch/tmp/ vm-build-fedora
    VM-BUILD fedora 
tar: Skipping to next header
tar: Exiting with failure status due to previous errors
failed append submodule slirp to /mnt/code/qemu/full/vm-test-eqebvy5r.tmp/data-3a52c.tar
Failed to prepare guest environment
Traceback (most recent call last):
  File "/mnt/code/qemu/full/tests/vm/basevm.py", line 636, in main
    vm.add_source_dir(args.build_qemu)
  File "/mnt/code/qemu/full/tests/vm/basevm.py", line 270, in add_source_dir
    subprocess.check_call(["./scripts/archive-source.sh", tarfile],
  File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['./scripts/archive-source.sh', '/mnt/code/qemu/full/vm-test-eqebvy5r.tmp/data-3a52c.tar']' returned non-zero exit status 1.
make: *** [tests/vm/Makefile.include:96: vm-build-fedora] Error 2

forget about the HOME= change, it also fails if I don't use it.  And the
problem is the archive bits.

master)$  ./scripts/archive-source.sh /tmp/kk.tar
tar: Skipping to next header
tar: Exiting with failure status due to previous errors
failed append submodule slirp to /tmp/kk.tar

See that the problem is the slirp submodule, but it has the "right"
version, i.e. not a case of updating the module.  the dtc works without problem.

(master)$ sh -x ./scripts/archive-source.sh /tmp/kk.tar
+ test 1 -lt 1
++ realpath /tmp/kk.tar
+ tar_file=/tmp/kk.tar
++ mktemp -d /tmp/kk.sub.XXXXXXXX
+ sub_tdir=/tmp/kk.sub.WKj1o6oP
+ sub_file=/tmp/kk.sub.WKj1o6oP/submodule.tar
+ submodules='dtc slirp meson ui/keycodemapdb'
+ submodules='dtc slirp meson ui/keycodemapdb tests/fp/berkeley-softfloat-3 tests/fp/berkeley-testfloat-3'
+ sub_deinit=
+ trap cleanup 0 1 2 3 15
++ tree_ish
++ local retval=HEAD
++ git diff-index --quiet --ignore-submodules=all HEAD --
++ echo HEAD
+ git archive --format tar HEAD
+ test 0 -ne 0
+ for sm in $submodules
++ git submodule status dtc
+ status=' 85e5d839847af54efab170f2b1331b2a6421e647 dtc (v1.6.0-4-g85e5d83)'
+ smhash='85e5d839847af54efab170f2b1331b2a6421e647 dtc (v1.6.0-4-g85e5d83)'
+ smhash=85e5d839847af54efab170f2b1331b2a6421e647
+ case "$status" in
+ cd dtc
++ tree_ish
++ local retval=HEAD
++ git diff-index --quiet --ignore-submodules=all HEAD --
++ echo HEAD
+ git archive --format tar --prefix dtc/ HEAD
+ test 0 -ne 0
+ tar --concatenate --file /tmp/kk.tar /tmp/kk.sub.WKj1o6oP/submodule.tar
+ test 0 -ne 0
+ for sm in $submodules
++ git submodule status slirp
+ status=' a88d9ace234a24ce1c17189642ef9104799425e0 slirp (v4.6.1-7-ga88d9ac)'
+ smhash='a88d9ace234a24ce1c17189642ef9104799425e0 slirp (v4.6.1-7-ga88d9ac)'
+ smhash=a88d9ace234a24ce1c17189642ef9104799425e0
+ case "$status" in
+ cd slirp
++ tree_ish
++ local retval=HEAD
++ git diff-index --quiet --ignore-submodules=all HEAD --
++ echo HEAD
+ git archive --format tar --prefix slirp/ HEAD
+ test 0 -ne 0
+ tar --concatenate --file /tmp/kk.tar /tmp/kk.sub.WKj1o6oP/submodule.tar
tar: Skipping to next header
tar: Exiting with failure status due to previous errors
+ test 2 -ne 0
+ error 'failed append submodule slirp to /tmp/kk.tar'
+ printf '%s\n' 'failed append submodule slirp to /tmp/kk.tar'
failed append submodule slirp to /tmp/kk.tar
+ exit 1
+ cleanup
+ local status=1
+ rm -rf /tmp/kk.sub.WKj1o6oP
+ test '' '!=' ''
+ exit 1
(master)$ 

Doing the things on the command line, the 

  git archive --format tar --prefix slirp/ HEAD

Creates a tar archive, so I get completely lost.

I showed here fedora, but it fails exactly the same for freebsd,
openbsd, ... and everything that I decided to build.  It fails in the
smae stage.

Later, Juan.


Re: [PULL 0/7] Migration.next patches
Posted by lizhijian@fujitsu.com 2 years, 7 months ago

On 10/09/2021 15:00, Juan Quintela wrote:
> ++ git diff-index --quiet --ignore-submodules=all HEAD --
> ++ echo HEAD
> + git archive --format tar --prefix slirp/ HEAD
> + test 0 -ne 0
> + tar --concatenate --file /tmp/kk.tar /tmp/kk.sub.WKj1o6oP/submodule.tar
> tar: Skipping to next header
> tar: Exiting with failure status due to previous errors
> + test 2 -ne 0
> + error 'failed append submodule slirp to /tmp/kk.tar'
> + printf '%s\n' 'failed append submodule slirp to /tmp/kk.tar'
> failed append submodule slirp to /tmp/kk.tar
> + exit 1
> + cleanup
> + local status=1
> + rm -rf /tmp/kk.sub.WKj1o6oP
> + test '' '!=' ''
> + exit 1
> (master)$
>
> Doing the things on the command line, the
>
>    git archive --format tar --prefix slirp/ HEAD

It's so weird, i have no idea about it.
It works fine for me. :)



> Creates a tar archive, so I get completely lost.
>
> I showed here fedora, but it fails exactly the same for freebsd,
> openbsd, ... and everything that I decided to build.  It fails in the
> smae stage.
Re: [PULL 0/7] Migration.next patches
Posted by Philippe Mathieu-Daudé 2 years, 7 months ago
On 9/10/21 7:27 AM, lizhijian@fujitsu.com wrote:
> On 10/09/2021 13:20, Li Zhijian wrote:
>> On 10/09/2021 00:10, Juan Quintela wrote:
>>> "Li, Zhijian" <lizhijian@cn.fujitsu.com> wrote:
>>>> on 2021/9/9 21:42, Peter Maydell wrote:
>>>>> On Thu, 9 Sept 2021 at 11:36, Juan Quintela <quintela@redhat.com> wrote:
>>>>> Fails to build, FreeBSD:
>>>>>
>>>>> ../src/migration/rdma.c:1146:23: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE'
>>>>>       int advice = wr ? IBV_ADVISE_MR_ADVICE_PREFETCH_WRITE :
>>>>>                         ^
>>>>> ../src/migration/rdma.c:1147:18: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_ADVICE_PREFETCH'
>>>>>                    IBV_ADVISE_MR_ADVICE_PREFETCH;
>>>>>                    ^
>>>>> ../src/migration/rdma.c:1150:11: warning: implicit declaration of
>>>>> function 'ibv_advise_mr' is invalid in C99
>>>>> [-Wimplicit-function-declaration]
>>>>>       ret = ibv_advise_mr(pd, advice,
>>>>>             ^
>>>>> ../src/migration/rdma.c:1151:25: error: use of undeclared identifier
>>>>> 'IBV_ADVISE_MR_FLAG_FLUSH'
>>>>>                           IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
>>>>>                           ^
>>>>>
>>>> it's introduced by [PULL 4/7] migration/rdma: advise prefetch write for ODP region
>>>> where it calls a ibv_advise_mr(). i have checked the latest FreeBSD, it didn't ship with this API
>>>> May i know if just FressBSD reports this failure? if so, i just need filtering out FreeBSD only
>>> Second try.  I can't see an example where they search for:
>>> a symbol on the header file
>>>    and
>>> a function in a library
>>>
>>> so I assume that if you have the symbols, you have the function.
>>>
>>> How do you see it?
>>>
>>> Trying to compile it on vm-build-freebsd, but not being very sucessfull
>>> so far.
> 
> BTW: Does QEMU provide any mean to set http(s)_proxy to building vm ? Currently, i have to
> hack the code like:
> 
> -        self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
> +        self.ssh_root_check("setenv HTTP_PROXY http://myproxy; setenv HTTPS_PROXY http://myproxy; pkg install -y %s\n" % " ".join(self.pkgs))

This is supported since commit b08ba163aaa ("tests/vm: send proxy
environment variables over ssh"). Maybe we only pass lower case
variables and should consider upper case too?

> 
> 
> Thanks
> Zhijian


Re: [PULL 0/7] Migration.next patches
Posted by Li, Zhijian 2 years, 7 months ago
on 2021/9/10 20:55, Philippe Mathieu-Daudé wrote:
>> BTW: Does QEMU provide any mean to set http(s)_proxy to building vm ? Currently, i have to
>> hack the code like:
>>
>> -        self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
>> +        self.ssh_root_check("setenv HTTP_PROXY http://myproxy; setenv HTTPS_PROXY http://myproxy; pkg install -y %s\n" % " ".join(self.pkgs))
> This is supported since commit b08ba163aaa ("tests/vm: send proxy
> environment variables over ssh"). Maybe we only pass lower case
> variables and should consider upper case too?

Great, I'm glad to know this. Thank you.
Lower case variables also work well on FreeBSD, so it's sufficient i think.


Thanks
Zhijian