[PATCH v2 7/8] migration/rdma: Remove redundant migration_in_postcopy checks

Li Zhijian via posted 8 patches 11 months, 3 weeks ago
There is a newer version of this series
[PATCH v2 7/8] migration/rdma: Remove redundant migration_in_postcopy checks
Posted by Li Zhijian via 11 months, 3 weeks ago
Since we have disabled RDMA + postcopy, it's safe to remove
the migration_in_postcopy()  that follows the migration_rdma().

Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
---
 migration/ram.c  | 2 +-
 migration/rdma.c | 5 +++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index e07651aee8d..c363034c882 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -1939,7 +1939,7 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
     int res;
 
     /* Hand over to RDMA first */
-    if (migrate_rdma() && !migration_in_postcopy()) {
+    if (migrate_rdma()) {
         res = rdma_control_save_page(pss->pss_channel, pss->block->offset,
                                      offset, TARGET_PAGE_SIZE);
 
diff --git a/migration/rdma.c b/migration/rdma.c
index c6876347e1e..0349dd4a8b8 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3826,7 +3826,7 @@ int rdma_block_notification_handle(QEMUFile *f, const char *name)
 
 int rdma_registration_start(QEMUFile *f, uint64_t flags)
 {
-    if (!migrate_rdma() || migration_in_postcopy()) {
+    if (!migrate_rdma()) {
         return 0;
     }
 
@@ -3858,7 +3858,8 @@ int rdma_registration_stop(QEMUFile *f, uint64_t flags)
     RDMAControlHeader head = { .len = 0, .repeat = 1 };
     int ret;
 
-    if (!migrate_rdma() || migration_in_postcopy()) {
+    /* Hand over to RDMA first */
+    if (!migrate_rdma()) {
         return 0;
     }
 
-- 
2.44.0
Re: [PATCH v2 7/8] migration/rdma: Remove redundant migration_in_postcopy checks
Posted by Peter Xu 11 months, 2 weeks ago
On Fri, Feb 21, 2025 at 02:36:11PM +0800, Li Zhijian wrote:
> Since we have disabled RDMA + postcopy, it's safe to remove
> the migration_in_postcopy()  that follows the migration_rdma().
> 
> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> ---
>  migration/ram.c  | 2 +-
>  migration/rdma.c | 5 +++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/ram.c b/migration/ram.c
> index e07651aee8d..c363034c882 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1939,7 +1939,7 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
>      int res;
>  
>      /* Hand over to RDMA first */
> -    if (migrate_rdma() && !migration_in_postcopy()) {

This line was just added in previous patch.

Would it be better move 5/6 above, then somehow squash 2/3/4/7 so that it
doesn't need to add something and got removed again?  I feel like the four
patches can be squashed into 1 or 2 instead when reorder them.

> +    if (migrate_rdma()) {
>          res = rdma_control_save_page(pss->pss_channel, pss->block->offset,
>                                       offset, TARGET_PAGE_SIZE);
>  
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c6876347e1e..0349dd4a8b8 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3826,7 +3826,7 @@ int rdma_block_notification_handle(QEMUFile *f, const char *name)
>  
>  int rdma_registration_start(QEMUFile *f, uint64_t flags)
>  {
> -    if (!migrate_rdma() || migration_in_postcopy()) {
> +    if (!migrate_rdma()) {
>          return 0;
>      }
>  
> @@ -3858,7 +3858,8 @@ int rdma_registration_stop(QEMUFile *f, uint64_t flags)
>      RDMAControlHeader head = { .len = 0, .repeat = 1 };
>      int ret;
>  
> -    if (!migrate_rdma() || migration_in_postcopy()) {
> +    /* Hand over to RDMA first */
> +    if (!migrate_rdma()) {
>          return 0;
>      }
>  
> -- 
> 2.44.0
> 

-- 
Peter Xu
Re: [PATCH v2 7/8] migration/rdma: Remove redundant migration_in_postcopy checks
Posted by Zhijian Li (Fujitsu) via 11 months, 2 weeks ago

On 25/02/2025 04:00, Peter Xu wrote:
> On Fri, Feb 21, 2025 at 02:36:11PM +0800, Li Zhijian wrote:
>> Since we have disabled RDMA + postcopy, it's safe to remove
>> the migration_in_postcopy()  that follows the migration_rdma().
>>
>> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
>> ---
>>   migration/ram.c  | 2 +-
>>   migration/rdma.c | 5 +++--
>>   2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index e07651aee8d..c363034c882 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -1939,7 +1939,7 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
>>       int res;
>>   
>>       /* Hand over to RDMA first */
>> -    if (migrate_rdma() && !migration_in_postcopy()) {
> 
> This line was just added in previous patch.
> 
> Would it be better move 5/6 above, then somehow squash 2/3/4/7 so that it
> doesn't need to add something and got removed again? 

Yeah, it sound good to me.
I tried to reorder the pathes and squash previous 2 3 4 to a single one

So the new layout will be like below:

e5b1998ad30 migration: Add qtest for migration over RDMA
9a1b87e2db6 migration: Unfold control_save_page()  << this one squashed previous 2/3/4
b6ccd49e934 migration/rdma: Remove redundant migration_in_postcopy checks
c7c4209db6f migration: disable RDMA + postcopy-ram
0463b54d7f9 migration: Add migration_capabilities_and_transport_compatible() helper
21c76dcabee migration: Prioritize RDMA in ram_save_target_page()


Thanks
Zhijian


> I feel like the four
> patches can be squashed into 1 or 2 instead when reorder them.
> 
>> +    if (migrate_rdma()) {
>>           res = rdma_control_save_page(pss->pss_channel, pss->block->offset,
>>                                        offset, TARGET_PAGE_SIZE);
>>   
>> diff --git a/migration/rdma.c b/migration/rdma.c
>> index c6876347e1e..0349dd4a8b8 100644
>> --- a/migration/rdma.c
>> +++ b/migration/rdma.c
>> @@ -3826,7 +3826,7 @@ int rdma_block_notification_handle(QEMUFile *f, const char *name)
>>   
>>   int rdma_registration_start(QEMUFile *f, uint64_t flags)
>>   {
>> -    if (!migrate_rdma() || migration_in_postcopy()) {
>> +    if (!migrate_rdma()) {
>>           return 0;
>>       }
>>   
>> @@ -3858,7 +3858,8 @@ int rdma_registration_stop(QEMUFile *f, uint64_t flags)
>>       RDMAControlHeader head = { .len = 0, .repeat = 1 };
>>       int ret;
>>   
>> -    if (!migrate_rdma() || migration_in_postcopy()) {
>> +    /* Hand over to RDMA first */
>> +    if (!migrate_rdma()) {
>>           return 0;
>>       }
>>   
>> -- 
>> 2.44.0
>>
> 
Re: [PATCH v2 7/8] migration/rdma: Remove redundant migration_in_postcopy checks
Posted by Peter Xu 11 months, 2 weeks ago
On Tue, Feb 25, 2025 at 06:21:20AM +0000, Zhijian Li (Fujitsu) wrote:
> 
> 
> On 25/02/2025 04:00, Peter Xu wrote:
> > On Fri, Feb 21, 2025 at 02:36:11PM +0800, Li Zhijian wrote:
> >> Since we have disabled RDMA + postcopy, it's safe to remove
> >> the migration_in_postcopy()  that follows the migration_rdma().
> >>
> >> Signed-off-by: Li Zhijian <lizhijian@fujitsu.com>
> >> ---
> >>   migration/ram.c  | 2 +-
> >>   migration/rdma.c | 5 +++--
> >>   2 files changed, 4 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/migration/ram.c b/migration/ram.c
> >> index e07651aee8d..c363034c882 100644
> >> --- a/migration/ram.c
> >> +++ b/migration/ram.c
> >> @@ -1939,7 +1939,7 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
> >>       int res;
> >>   
> >>       /* Hand over to RDMA first */
> >> -    if (migrate_rdma() && !migration_in_postcopy()) {
> > 
> > This line was just added in previous patch.
> > 
> > Would it be better move 5/6 above, then somehow squash 2/3/4/7 so that it
> > doesn't need to add something and got removed again? 
> 
> Yeah, it sound good to me.
> I tried to reorder the pathes and squash previous 2 3 4 to a single one
> 
> So the new layout will be like below:
> 
> e5b1998ad30 migration: Add qtest for migration over RDMA
> 9a1b87e2db6 migration: Unfold control_save_page()  << this one squashed previous 2/3/4
> b6ccd49e934 migration/rdma: Remove redundant migration_in_postcopy checks
> c7c4209db6f migration: disable RDMA + postcopy-ram
> 0463b54d7f9 migration: Add migration_capabilities_and_transport_compatible() helper
> 21c76dcabee migration: Prioritize RDMA in ram_save_target_page()

I'll have another look when repost, but so far looks good, thanks.

-- 
Peter Xu