Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-15 Thread Juan Quintela 
Cédric Le Goater  wrote:
> On 5/9/23 13:51, Juan Quintela wrote:
>> Harsh Prateek Bora  wrote:
>>> On 5/8/23 18:38, Juan Quintela wrote:
 Use 0 instead.
 Signed-off-by: Juan Quintela 
 ---
migration/migration.c | 4 ++--
migration/qemu-file.c | 3 +++
2 files changed, 5 insertions(+), 2 deletions(-)
 diff --git a/migration/migration.c b/migration/migration.c
 index 1192f1ebf1..3979a98949 100644
 --- a/migration/migration.c
 +++ b/migration/migration.c
 @@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
}
if (ret >= 0) {
s->block_inactive = !migrate_colo();
 -qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
 +qemu_file_set_rate_limit(s->to_dst_file, 0);
>>>
>>> #define RATE_LIMIT_MAX 0
>>>
>>> How about having a macro and use that which conveys the meaning in all
>>> call instances wherever it is getting passed ?
>> I almost preffer the macro.
>>qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_MAX);
>> seems quite explanatory?
>
> yep. and I would drop the comment qemu_file_rate_limit().

I dropped it once by error.
And reviewer didn't noticed either.

So

Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-15 Thread Cédric Le Goater 

On 5/9/23 13:51, Juan Quintela wrote:

Harsh Prateek Bora  wrote:

On 5/8/23 18:38, Juan Quintela wrote:

Use 0 instead.
Signed-off-by: Juan Quintela 
---
   migration/migration.c | 4 ++--
   migration/qemu-file.c | 3 +++
   2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1192f1ebf1..3979a98949 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
   }
   if (ret >= 0) {
   s->block_inactive = !migrate_colo();
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);


#define RATE_LIMIT_MAX 0

How about having a macro and use that which conveys the meaning in all
call instances wherever it is getting passed ?


I almost preffer the macro.

   qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_MAX);

seems quite explanatory?


yep. and I would drop the comment qemu_file_rate_limit().

Thanks,

C.




Thanks, Juan.




   ret = qemu_savevm_state_complete_precopy(s->to_dst_file, 
false,
s->block_inactive);
   }
@@ -3044,7 +3044,7 @@ static void *bg_migration_thread(void *opaque)
   rcu_register_thread();
   object_ref(OBJECT(s));
   -qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);
 setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
   /*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index f4cfd05c67..745361d238 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
   if (qemu_file_get_error(f)) {
   return 1;
   }
+/*
+ *  rate_limit_max == 0 means no rate_limit enfoncement.
+ */
   if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
   return 1;
   }

Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-09 Thread Harsh Prateek Bora 




On 5/9/23 17:21, Juan Quintela wrote:

Harsh Prateek Bora  wrote:

On 5/8/23 18:38, Juan Quintela wrote:

Use 0 instead.
Signed-off-by: Juan Quintela 
---
   migration/migration.c | 4 ++--
   migration/qemu-file.c | 3 +++
   2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1192f1ebf1..3979a98949 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
   }
   if (ret >= 0) {
   s->block_inactive = !migrate_colo();
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);


#define RATE_LIMIT_MAX 0

How about having a macro and use that which conveys the meaning in all
call instances wherever it is getting passed ?


I almost preffer the macro.

   qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_MAX);

seems quite explanatory?


Yes, definitely.

Thanks
Harsh

Thanks, Juan.




   ret = qemu_savevm_state_complete_precopy(s->to_dst_file, 
false,
s->block_inactive);
   }
@@ -3044,7 +3044,7 @@ static void *bg_migration_thread(void *opaque)
   rcu_register_thread();
   object_ref(OBJECT(s));
   -qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);
 setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
   /*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index f4cfd05c67..745361d238 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
   if (qemu_file_get_error(f)) {
   return 1;
   }
+/*
+ *  rate_limit_max == 0 means no rate_limit enfoncement.
+ */
   if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
   return 1;
   }

Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-09 Thread Harsh Prateek Bora 




On 5/8/23 18:38, Juan Quintela wrote:

Use 0 instead.

Signed-off-by: Juan Quintela 
---
  migration/migration.c | 4 ++--
  migration/qemu-file.c | 3 +++
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1192f1ebf1..3979a98949 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
  }
  if (ret >= 0) {
  s->block_inactive = !migrate_colo();
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);


#define RATE_LIMIT_MAX 0

How about having a macro and use that which conveys the meaning in all 
call instances wherever it is getting passed ?




  ret = qemu_savevm_state_complete_precopy(s->to_dst_file, 
false,
   s->block_inactive);
  }
@@ -3044,7 +3044,7 @@ static void *bg_migration_thread(void *opaque)
  rcu_register_thread();
  object_ref(OBJECT(s));
  
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);

+qemu_file_set_rate_limit(s->to_dst_file, 0);
  
  setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);

  /*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index f4cfd05c67..745361d238 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
  if (qemu_file_get_error(f)) {
  return 1;
  }
+/*
+ *  rate_limit_max == 0 means no rate_limit enfoncement.
+ */
  if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
  return 1;
  }

Re: [PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-09 Thread Juan Quintela 
Harsh Prateek Bora  wrote:
> On 5/8/23 18:38, Juan Quintela wrote:
>> Use 0 instead.
>> Signed-off-by: Juan Quintela 
>> ---
>>   migration/migration.c | 4 ++--
>>   migration/qemu-file.c | 3 +++
>>   2 files changed, 5 insertions(+), 2 deletions(-)
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 1192f1ebf1..3979a98949 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
>>   }
>>   if (ret >= 0) {
>>   s->block_inactive = !migrate_colo();
>> -qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
>> +qemu_file_set_rate_limit(s->to_dst_file, 0);
>
> #define RATE_LIMIT_MAX 0
>
> How about having a macro and use that which conveys the meaning in all
> call instances wherever it is getting passed ?

I almost preffer the macro.

  qemu_file_set_rate_limit(s->to_dst_file, RATE_LIMIT_MAX);

seems quite explanatory?

Thanks, Juan.

>
>>   ret = qemu_savevm_state_complete_precopy(s->to_dst_file, 
>> false,
>>
>> s->block_inactive);
>>   }
>> @@ -3044,7 +3044,7 @@ static void *bg_migration_thread(void *opaque)
>>   rcu_register_thread();
>>   object_ref(OBJECT(s));
>>   -qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
>> +qemu_file_set_rate_limit(s->to_dst_file, 0);
>> setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
>>   /*
>> diff --git a/migration/qemu-file.c b/migration/qemu-file.c
>> index f4cfd05c67..745361d238 100644
>> --- a/migration/qemu-file.c
>> +++ b/migration/qemu-file.c
>> @@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
>>   if (qemu_file_get_error(f)) {
>>   return 1;
>>   }
>> +/*
>> + *  rate_limit_max == 0 means no rate_limit enfoncement.
>> + */
>>   if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
>>   return 1;
>>   }

[PATCH 02/21] migration: Don't use INT64_MAX for unlimited rate

2023-05-08 Thread Juan Quintela 
Use 0 instead.

Signed-off-by: Juan Quintela 
---
 migration/migration.c | 4 ++--
 migration/qemu-file.c | 3 +++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index 1192f1ebf1..3979a98949 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2296,7 +2296,7 @@ static void migration_completion(MigrationState *s)
 }
 if (ret >= 0) {
 s->block_inactive = !migrate_colo();
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);
 ret = qemu_savevm_state_complete_precopy(s->to_dst_file, false,
  s->block_inactive);
 }
@@ -3044,7 +3044,7 @@ static void *bg_migration_thread(void *opaque)
 rcu_register_thread();
 object_ref(OBJECT(s));
 
-qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX);
+qemu_file_set_rate_limit(s->to_dst_file, 0);
 
 setup_start = qemu_clock_get_ms(QEMU_CLOCK_HOST);
 /*
diff --git a/migration/qemu-file.c b/migration/qemu-file.c
index f4cfd05c67..745361d238 100644
--- a/migration/qemu-file.c
+++ b/migration/qemu-file.c
@@ -731,6 +731,9 @@ int qemu_file_rate_limit(QEMUFile *f)
 if (qemu_file_get_error(f)) {
 return 1;
 }
+/*
+ *  rate_limit_max == 0 means no rate_limit enfoncement.
+ */
 if (f->rate_limit_max > 0 && f->rate_limit_used > f->rate_limit_max) {
 return 1;
 }
-- 
2.40.0