diff mbox

[v7,12/14] migration: Use an array instead of 3 parameters

Message ID 1428474011-30797-13-git-send-email-liang.z.li@intel.com
State New
Headers show

Commit Message

Li, Liang Z April 8, 2015, 6:20 a.m. UTC
Put the three parameters related to multiple thread (de)compression
into an int array, and use an enum type to index the parameter.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
---
 include/migration/migration.h |  4 +---
 migration/migration.c         | 31 +++++++++++++++++++------------
 qapi-schema.json              | 23 +++++++++++++++++++++++
 3 files changed, 43 insertions(+), 15 deletions(-)

Comments

Juan Quintela April 8, 2015, 11:37 a.m. UTC | #1
Liang Li <liang.z.li@intel.com> wrote:
> Put the three parameters related to multiple thread (de)compression
> into an int array, and use an enum type to index the parameter.
>
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>


BTW, for the libvirt people, what do you think about moving the other
parameters here, and make the other accessors deprecated?

I mean here:
- xbzrle_cache_size
-> get mbps
-> set/get migration_speed
-> get/set migration_downtime
-> autoconverge?

You get the idea.

Later, Juan.
Eric Blake April 14, 2015, 12:01 p.m. UTC | #2
On 04/08/2015 05:37 AM, Juan Quintela wrote:
> Liang Li <liang.z.li@intel.com> wrote:
>> Put the three parameters related to multiple thread (de)compression
>> into an int array, and use an enum type to index the parameter.
>>
>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> 
> Reviewed-by: Juan Quintela <quintela@redhat.com>
> 
> 
> BTW, for the libvirt people, what do you think about moving the other
> parameters here, and make the other accessors deprecated?
> 
> I mean here:
> - xbzrle_cache_size
> -> get mbps
> -> set/get migration_speed
> -> get/set migration_downtime
> -> autoconverge?
> 
> You get the idea.

Setting multiple bits through one command is nicer. Libvirt will still
have to keep the code for driving older qemu, but I'm not opposed to
consolidation.
Eric Blake April 14, 2015, 12:03 p.m. UTC | #3
On 04/08/2015 12:20 AM, Liang Li wrote:
> Put the three parameters related to multiple thread (de)compression
> into an int array, and use an enum type to index the parameter.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> ---
>  include/migration/migration.h |  4 +---
>  migration/migration.c         | 31 +++++++++++++++++++------------
>  qapi-schema.json              | 23 +++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 15 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -569,6 +569,29 @@
>  ##
>  { 'command': 'query-migrate-capabilities', 'returns':   ['MigrationCapabilityStatus']}
>  
> +# @MigrationParameter
> +#
> +# Migration parameters enumeration
> +#
> +# @compress-level: Set the compression level to be used in live migration,
> +#          the compression level is an integer between 0 and 9, where 0 means
> +#          no compression, 1 means the best compression speed, and 9 means best
> +#          compression ratio which will consume more CPU.
> +#
> +# @compress-threads: Set compression thread count to be used in live migration,
> +#          the compression thread count is an integer between 1 and 255.
> +#
> +# @decompress-threads: Set decompression thread count to be used in live
> +#          migration, the decompression thread count is an integer between 1
> +#          and 255. Usually, decompression is at least 4 times as fast as
> +#          compression, so set the decompress-threads to the number about 1/4
> +#          of compress-threads is adequate.

s/so set/so setting/

> +#
> +# Since: 2.3

2.4

Maintainer can fix if a respin is not needed.
Juan Quintela April 15, 2015, 8:13 a.m. UTC | #4
Eric Blake <eblake@redhat.com> wrote:
> On 04/08/2015 12:20 AM, Liang Li wrote:
>> Put the three parameters related to multiple thread (de)compression
>> into an int array, and use an enum type to index the parameter.
>> 
>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
>> ---
>>  include/migration/migration.h |  4 +---
>>  migration/migration.c         | 31 +++++++++++++++++++------------
>>  qapi-schema.json              | 23 +++++++++++++++++++++++
>>  3 files changed, 43 insertions(+), 15 deletions(-)
>> 
>
>> +++ b/qapi-schema.json
>> @@ -569,6 +569,29 @@
>>  ##
>>  { 'command': 'query-migrate-capabilities', 'returns':   ['MigrationCapabilityStatus']}
>>  
>> +# @MigrationParameter
>> +#
>> +# Migration parameters enumeration
>> +#
>> +# @compress-level: Set the compression level to be used in live migration,
>> +#          the compression level is an integer between 0 and 9, where 0 means
>> +#          no compression, 1 means the best compression speed, and 9 means best
>> +#          compression ratio which will consume more CPU.
>> +#
>> +# @compress-threads: Set compression thread count to be used in live migration,
>> +#          the compression thread count is an integer between 1 and 255.
>> +#
>> +# @decompress-threads: Set decompression thread count to be used in live
>> +#          migration, the decompression thread count is an integer between 1
>> +#          and 255. Usually, decompression is at least 4 times as fast as
>> +#          compression, so set the decompress-threads to the number about 1/4
>> +#          of compress-threads is adequate.
>
> s/so set/so setting/
>
>> +#
>> +# Since: 2.3
>
> 2.4
>
> Maintainer can fix if a respin is not needed.

Liang, I will fix the s/2.3/2.4/ while merging, no need of respin for
this.

Later, Juan.
Li, Liang Z April 15, 2015, 8:35 a.m. UTC | #5
> Eric Blake <eblake@redhat.com> wrote:
> > On 04/08/2015 12:20 AM, Liang Li wrote:
> >> Put the three parameters related to multiple thread (de)compression
> >> into an int array, and use an enum type to index the parameter.
> >>
> >> Signed-off-by: Liang Li <liang.z.li@intel.com>
> >> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> >> ---
> >>  include/migration/migration.h |  4 +---
> >>  migration/migration.c         | 31 +++++++++++++++++++------------
> >>  qapi-schema.json              | 23 +++++++++++++++++++++++
> >>  3 files changed, 43 insertions(+), 15 deletions(-)
> >>
> >
> >> +++ b/qapi-schema.json
> >> @@ -569,6 +569,29 @@
> >>  ##
> >>  { 'command': 'query-migrate-capabilities', 'returns':
> ['MigrationCapabilityStatus']}
> >>
> >> +# @MigrationParameter
> >> +#
> >> +# Migration parameters enumeration
> >> +#
> >> +# @compress-level: Set the compression level to be used in live
> migration,
> >> +#          the compression level is an integer between 0 and 9, where 0
> means
> >> +#          no compression, 1 means the best compression speed, and 9
> means best
> >> +#          compression ratio which will consume more CPU.
> >> +#
> >> +# @compress-threads: Set compression thread count to be used in live
> migration,
> >> +#          the compression thread count is an integer between 1 and 255.
> >> +#
> >> +# @decompress-threads: Set decompression thread count to be used in
> live
> >> +#          migration, the decompression thread count is an integer between
> 1
> >> +#          and 255. Usually, decompression is at least 4 times as fast as
> >> +#          compression, so set the decompress-threads to the number
> about 1/4
> >> +#          of compress-threads is adequate.
> >
> > s/so set/so setting/
> >
> >> +#
> >> +# Since: 2.3
> >
> > 2.4
> >
> > Maintainer can fix if a respin is not needed.
> 
> Liang, I will fix the s/2.3/2.4/ while merging, no need of respin for this.
> 
> Later, Juan.

OK, the v8 is very soon.

Liang
diff mbox

Patch

diff --git a/include/migration/migration.h b/include/migration/migration.h
index d4a1062..a6e025a 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -50,9 +50,7 @@  struct MigrationState
     QemuThread thread;
     QEMUBH *cleanup_bh;
     QEMUFile *file;
-    int compress_thread_count;
-    int decompress_thread_count;
-    int compress_level;
+    int parameters[MIGRATION_PARAMETER_MAX];
 
     int state;
     MigrationParams params;
diff --git a/migration/migration.c b/migration/migration.c
index dc7db87..533717c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -60,9 +60,12 @@  MigrationState *migrate_get_current(void)
         .bandwidth_limit = MAX_THROTTLE,
         .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE,
         .mbps = -1,
-        .compress_thread_count = DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
-        .decompress_thread_count = DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
-        .compress_level = DEFAULT_MIGRATE_COMPRESS_LEVEL,
+        .parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] =
+                DEFAULT_MIGRATE_COMPRESS_LEVEL,
+        .parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
+                DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT,
+        .parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
+                DEFAULT_MIGRATE_DECOMPRESS_THREAD_COUNT,
     };
 
     return &current_migration;
@@ -406,9 +409,11 @@  static MigrationState *migrate_init(const MigrationParams *params)
     int64_t bandwidth_limit = s->bandwidth_limit;
     bool enabled_capabilities[MIGRATION_CAPABILITY_MAX];
     int64_t xbzrle_cache_size = s->xbzrle_cache_size;
-    int compress_level = s->compress_level;
-    int compress_thread_count = s->compress_thread_count;
-    int decompress_thread_count = s->decompress_thread_count;
+    int compress_level = s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
+    int compress_thread_count =
+            s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
+    int decompress_thread_count =
+            s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
 
     memcpy(enabled_capabilities, s->enabled_capabilities,
            sizeof(enabled_capabilities));
@@ -419,9 +424,11 @@  static MigrationState *migrate_init(const MigrationParams *params)
            sizeof(enabled_capabilities));
     s->xbzrle_cache_size = xbzrle_cache_size;
 
-    s->compress_level = compress_level;
-    s->compress_thread_count = compress_thread_count;
-    s->decompress_thread_count = decompress_thread_count;
+    s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL] = compress_level;
+    s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS] =
+               compress_thread_count;
+    s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS] =
+               decompress_thread_count;
     s->bandwidth_limit = bandwidth_limit;
     s->state = MIGRATION_STATUS_SETUP;
     trace_migrate_set_state(MIGRATION_STATUS_SETUP);
@@ -624,7 +631,7 @@  int migrate_compress_level(void)
 
     s = migrate_get_current();
 
-    return s->compress_level;
+    return s->parameters[MIGRATION_PARAMETER_COMPRESS_LEVEL];
 }
 
 int migrate_compress_threads(void)
@@ -633,7 +640,7 @@  int migrate_compress_threads(void)
 
     s = migrate_get_current();
 
-    return s->compress_thread_count;
+    return s->parameters[MIGRATION_PARAMETER_COMPRESS_THREADS];
 }
 
 int migrate_decompress_threads(void)
@@ -642,7 +649,7 @@  int migrate_decompress_threads(void)
 
     s = migrate_get_current();
 
-    return s->decompress_thread_count;
+    return s->parameters[MIGRATION_PARAMETER_DECOMPRESS_THREADS];
 }
 
 int migrate_use_xbzrle(void)
diff --git a/qapi-schema.json b/qapi-schema.json
index b117008..e7e2343 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -569,6 +569,29 @@ 
 ##
 { 'command': 'query-migrate-capabilities', 'returns':   ['MigrationCapabilityStatus']}
 
+# @MigrationParameter
+#
+# Migration parameters enumeration
+#
+# @compress-level: Set the compression level to be used in live migration,
+#          the compression level is an integer between 0 and 9, where 0 means
+#          no compression, 1 means the best compression speed, and 9 means best
+#          compression ratio which will consume more CPU.
+#
+# @compress-threads: Set compression thread count to be used in live migration,
+#          the compression thread count is an integer between 1 and 255.
+#
+# @decompress-threads: Set decompression thread count to be used in live
+#          migration, the decompression thread count is an integer between 1
+#          and 255. Usually, decompression is at least 4 times as fast as
+#          compression, so set the decompress-threads to the number about 1/4
+#          of compress-threads is adequate.
+#
+# Since: 2.3
+##
+{ 'enum': 'MigrationParameter',
+  'data': ['compress-level', 'compress-threads', 'decompress-threads'] }
+
 ##
 # @MouseInfo:
 #