Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-26 Thread Dr. David Alan Gilbert 
* Li, Liang Z (liang.z...@intel.com) wrote:
  
  * Liang Li (liang.z...@intel.com) wrote:
   Add the qmp and hmp commands to tune the parameters used in live
   migration.
  
  If I understand correctly on the destination side we need to set the number
  of decompression threads very early on an incoming migration - I'm not clear
  how early that needs to be - especially if you're using fd: so it's not 
  waiting
  for a connect ?
 
 The decompression threads can be set after QEMU started (with -incomming
 options) and before the TCP accept.

But that doesn't work for an fd: incoming migration.

Dave

 
 
 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-24 Thread Li, Liang Z 
 
 * Liang Li (liang.z...@intel.com) wrote:
  Add the qmp and hmp commands to tune the parameters used in live
  migration.
 
 If I understand correctly on the destination side we need to set the number
 of decompression threads very early on an incoming migration - I'm not clear
 how early that needs to be - especially if you're using fd: so it's not 
 waiting
 for a connect ?

The decompression threads can be set after QEMU started (with -incomming
options) and before the TCP accept.

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-23 Thread Dr. David Alan Gilbert 
* Liang Li (liang.z...@intel.com) wrote:
 Add the qmp and hmp commands to tune the parameters used in live
 migration.

If I understand correctly on the destination side we need to set the number of
decompression threads very early on an incoming migration - I'm not
clear how early that needs to be - especially if you're using fd: so it's
not waiting for a connect ?

Eric: How would libvirt do that?

 
 Signed-off-by: Liang Li liang.z...@intel.com
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 ---
  hmp-commands.hx   | 15 ++
  hmp.c | 32 +
  hmp.h |  3 ++
  include/migration/migration.h |  4 +--
  migration.c   | 66 
 +++
  monitor.c | 18 
  qapi-schema.json  | 44 +
  qmp-commands.hx   | 23 +++
  8 files changed, 190 insertions(+), 15 deletions(-)
 
 diff --git a/hmp-commands.hx b/hmp-commands.hx
 index e37bc8b..535b5ba 100644
 --- a/hmp-commands.hx
 +++ b/hmp-commands.hx
 @@ -985,6 +985,21 @@ Enable/Disable the usage of a capability 
 @var{capability} for migration.
  ETEXI
  
  {
 +.name   = migrate_set_parameter,
 +.args_type  = parameter:s,value:i,
 +.params = parameter value,
 +.help   = Set the parameter for migration,
 +.mhandler.cmd = hmp_migrate_set_parameter,
 +.command_completion = migrate_set_parameter_completion,
 +},
 +
 +STEXI
 +@item migrate_set_parameter @var{parameter} @var{value}
 +@findex migrate_set_parameter
 +Set the parameter @var{parameter} for migration.
 +ETEXI
 +
 +{
  .name   = client_migrate_info,
  .args_type  = 
 protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?,
  .params = protocol hostname port tls-port cert-subject,
 diff --git a/hmp.c b/hmp.c
 index 63d7686..965c037 100644
 --- a/hmp.c
 +++ b/hmp.c
 @@ -1079,6 +1079,38 @@ void hmp_migrate_set_capability(Monitor *mon, const 
 QDict *qdict)
  }
  }
  
 +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
 +{
 +const char *param = qdict_get_str(qdict, parameter);
 +int value = qdict_get_int(qdict, value);
 +Error *err = NULL;
 +MigrationParameterStatusList *params = g_malloc0(sizeof(*params));
 +int i;
 +
 +for (i = 0; i  MIGRATION_PARAMETER_MAX; i++) {
 +if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
 +params-value = g_malloc0(sizeof(*params-value));
 +params-value-parameter = i;
 +params-value-value = value;
 +params-next = NULL;
 +qmp_migrate_set_parameters(params, err);
 +break;
 +}
 +}
 +
 +if (i == MIGRATION_PARAMETER_MAX) {
 +error_set(err, QERR_INVALID_PARAMETER, param);
 +}
 +
 +qapi_free_MigrationParameterStatusList(params);
 +
 +if (err) {
 +monitor_printf(mon, migrate_set_parameter: %s\n,
 +   error_get_pretty(err));
 +error_free(err);
 +}
 +}
 +
  void hmp_set_password(Monitor *mon, const QDict *qdict)
  {
  const char *protocol  = qdict_get_str(qdict, protocol);
 diff --git a/hmp.h b/hmp.h
 index 4bb5dca..bd1b203 100644
 --- a/hmp.h
 +++ b/hmp.h
 @@ -63,6 +63,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
 +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
  void hmp_set_password(Monitor *mon, const QDict *qdict);
  void hmp_expire_password(Monitor *mon, const QDict *qdict);
 @@ -111,6 +112,8 @@ void watchdog_action_completion(ReadLineState *rs, int 
 nb_args,
  const char *str);
  void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
 const char *str);
 +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
 +   const char *str);
  void host_net_add_completion(ReadLineState *rs, int nb_args, const char 
 *str);
  void host_net_remove_completion(ReadLineState *rs, int nb_args,
  const char *str);
 diff --git a/include/migration/migration.h b/include/migration/migration.h
 index 0c4f21c..8e09b42 100644
 --- a/include/migration/migration.h
 +++ b/include/migration/migration.h
 @@ -50,9 +50,7 @@ struct MigrationState
  QEMUBH *cleanup_bh;
  QEMUFile *file;
  QemuThread *compress_thread;
 -int compress_thread_count;
 -int decompress_thread_count;
 -int compress_level;
 +int parameters[MIGRATION_PARAMETER_MAX];
  
  int state;
  MigrationParams params;

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-23 Thread Eric Blake 
On 12/11/2014 06:29 PM, Liang Li wrote:
 Add the qmp and hmp commands to tune the parameters used in live
 migration.
 
 Signed-off-by: Liang Li liang.z...@intel.com
 Signed-off-by: Yang Zhang yang.z.zh...@intel.com
 ---
  hmp-commands.hx   | 15 ++
  hmp.c | 32 +
  hmp.h |  3 ++
  include/migration/migration.h |  4 +--
  migration.c   | 66 
 +++
  monitor.c | 18 
  qapi-schema.json  | 44 +
  qmp-commands.hx   | 23 +++
  8 files changed, 190 insertions(+), 15 deletions(-)
 


 +++ b/hmp.h
 @@ -63,6 +63,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
 +void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict);
  void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
  void hmp_set_password(Monitor *mon, const QDict *qdict);
  void hmp_expire_password(Monitor *mon, const QDict *qdict);
 @@ -111,6 +112,8 @@ void watchdog_action_completion(ReadLineState *rs, int 
 nb_args,
  const char *str);
  void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
 const char *str);
 +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
 +   const char *str);

Off-by-one on indentation.


 @@ -292,6 +295,41 @@ void 
 qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
  }
  }
  
 +void qmp_migrate_set_parameters(MigrationParameterStatusList *params,
 +  Error **errp)

Indentation is off.

 +{
 +MigrationState *s = migrate_get_current();
 +MigrationParameterStatusList *p;
 +
 +for (p = params; p; p = p-next) {
 +switch (p-value-parameter) {
 +case MIGRATION_PARAMETER_COMPRESS_LEVEL:
 +if (p-value-value  0 || p-value-value  9) {
 +error_set(errp, QERR_INVALID_PARAMETER_VALUE, 
 compress_level,
 +is invalied, it should be in the rang of 0 to 9);

s/invalied/invalid/; s/rang/range/


 +if (p-value-value  1 || p-value-value  255) {
 +error_set(errp, QERR_INVALID_PARAMETER_VALUE,
 +(de)compress_threads,
 +is invalied, it should be in the rang of 1 to 255);

and again


 +++ b/monitor.c
 @@ -4544,6 +4544,24 @@ void migrate_set_capability_completion(ReadLineState 
 *rs, int nb_args,
  }
  }
  
 +void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
 +   const char *str)

Indentation is off.


 +++ b/qapi-schema.json
 @@ -540,6 +540,50 @@
  ##
  { 'command': 'query-migrate-capabilities', 'returns':   
 ['MigrationCapabilityStatus']}
  
 +# @MigrationParameter
 +#
 +# Migration parameters enumeration
 +#
 +# @compress-level:Set the compression level to be used in live migration,

s/:/: /

 +#  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.
 +#
 +# Since: 2.3
 +##
 +{ 'enum': 'MigrationParameter',
 +  'data': ['compress-level', 'compress-threads', 'decompress-threads'] }
 +##
 +# @MigrationParameterStatus
 +#
 +# Migration parameter information
 +#
 +# @parameter: parameter enum
 +#
 +# @value: parameter value int
 +#
 +# Since: 2.3
 +##
 +{ 'type': 'MigrationParameterStatus',
 +  'data': { 'parameter' : 'MigrationParameter', 'value' : 'int' } }

Doesn't allow for non-integer parameters.  Not necessarily fatal for
input only (I think a flat union could be utilized with a
MigrationParameter as the discriminator to allow non-int values later on)...

 +##
 +# @migrate-set-parameters
 +#
 +# Set the following migration parameters (like compress-level)
 +#
 +# @parameters: json array of parameter modifications to make
 +#
 +# Since: 2.3
 +##
 +{ 'command': 'migrate-set-parameters',
 +  'data': { 'parameters': ['MigrationParameterStatus'] } }

...but on output, we might confuse callers by outputting non-int values
unless we plan _from the start_ to support them.  That is, I think we want:

{ 'type': 'MigrationParameterBase',
  'data': { 'parameter': 'MigrationParameter' } }
{ 'type':

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-23 Thread Eric Blake 
On 01/23/2015 08:59 AM, Dr. David Alan Gilbert wrote:
 * Eric Blake (ebl...@redhat.com) wrote:
 On 01/23/2015 06:48 AM, Dr. David Alan Gilbert wrote:
 * Liang Li (liang.z...@intel.com) wrote:
 Add the qmp and hmp commands to tune the parameters used in live
 migration.

 If I understand correctly on the destination side we need to set the number 
 of
 decompression threads very early on an incoming migration - I'm not
 clear how early that needs to be - especially if you're using fd: so it's
 not waiting for a connect ?

 Eric: How would libvirt do that?

 Libvirt does some handshaking before starting migration.  The source
 would advertise that I'm capable of threaded migration; and plan to use
 X send and Y receive threads; the destination would either reply that
 threads are unsupported or set the receive thread count at that point,
 then the source would know if it can enable threads and set the send
 thread count, before letting migration start.  I don't see any problems
 with the current design of starting things up.
 
 Patch 3 calls migrate_decompress_threads_create from 
 process_incoming_migration_co,
 if you're using fd based incoming migration then I think that gets called
 before libvirt would have a chance to connect to the monitor and
 call the parameter setting to say how many threads it wants.
 (A tcp incoming migration wouldn't have that problem because it
 waits until the TCP accept before calling process_incoming_migration_co)

Then it probably needs to be exposed through a command-line parameter,
so that -incoming gains the ability to specify parameters as part of
starting up qemu.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-23 Thread Eric Blake 
On 01/23/2015 06:48 AM, Dr. David Alan Gilbert wrote:
 * Liang Li (liang.z...@intel.com) wrote:
 Add the qmp and hmp commands to tune the parameters used in live
 migration.
 
 If I understand correctly on the destination side we need to set the number of
 decompression threads very early on an incoming migration - I'm not
 clear how early that needs to be - especially if you're using fd: so it's
 not waiting for a connect ?
 
 Eric: How would libvirt do that?

Libvirt does some handshaking before starting migration.  The source
would advertise that I'm capable of threaded migration; and plan to use
X send and Y receive threads; the destination would either reply that
threads are unsupported or set the receive thread count at that point,
then the source would know if it can enable threads and set the send
thread count, before letting migration start.  I don't see any problems
with the current design of starting things up.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2015-01-23 Thread Dr. David Alan Gilbert 
* Eric Blake (ebl...@redhat.com) wrote:
 On 01/23/2015 06:48 AM, Dr. David Alan Gilbert wrote:
  * Liang Li (liang.z...@intel.com) wrote:
  Add the qmp and hmp commands to tune the parameters used in live
  migration.
  
  If I understand correctly on the destination side we need to set the number 
  of
  decompression threads very early on an incoming migration - I'm not
  clear how early that needs to be - especially if you're using fd: so it's
  not waiting for a connect ?
  
  Eric: How would libvirt do that?
 
 Libvirt does some handshaking before starting migration.  The source
 would advertise that I'm capable of threaded migration; and plan to use
 X send and Y receive threads; the destination would either reply that
 threads are unsupported or set the receive thread count at that point,
 then the source would know if it can enable threads and set the send
 thread count, before letting migration start.  I don't see any problems
 with the current design of starting things up.

Patch 3 calls migrate_decompress_threads_create from 
process_incoming_migration_co,
if you're using fd based incoming migration then I think that gets called
before libvirt would have a chance to connect to the monitor and
call the parameter setting to say how many threads it wants.
(A tcp incoming migration wouldn't have that problem because it
waits until the TCP accept before calling process_incoming_migration_co)

Dave

 
 -- 
 Eric Blake   eblake redhat com+1-919-301-3266
 Libvirt virtualization library http://libvirt.org
 


--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [v3 12/13] migration: Add command to set migration parameter

2014-12-11 Thread Liang Li 
Add the qmp and hmp commands to tune the parameters used in live
migration.

Signed-off-by: Liang Li liang.z...@intel.com
Signed-off-by: Yang Zhang yang.z.zh...@intel.com
---
 hmp-commands.hx   | 15 ++
 hmp.c | 32 +
 hmp.h |  3 ++
 include/migration/migration.h |  4 +--
 migration.c   | 66 +++
 monitor.c | 18 
 qapi-schema.json  | 44 +
 qmp-commands.hx   | 23 +++
 8 files changed, 190 insertions(+), 15 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index e37bc8b..535b5ba 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -985,6 +985,21 @@ Enable/Disable the usage of a capability @var{capability} 
for migration.
 ETEXI
 
 {
+.name   = migrate_set_parameter,
+.args_type  = parameter:s,value:i,
+.params = parameter value,
+.help   = Set the parameter for migration,
+.mhandler.cmd = hmp_migrate_set_parameter,
+.command_completion = migrate_set_parameter_completion,
+},
+
+STEXI
+@item migrate_set_parameter @var{parameter} @var{value}
+@findex migrate_set_parameter
+Set the parameter @var{parameter} for migration.
+ETEXI
+
+{
 .name   = client_migrate_info,
 .args_type  = 
protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?,
 .params = protocol hostname port tls-port cert-subject,
diff --git a/hmp.c b/hmp.c
index 63d7686..965c037 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1079,6 +1079,38 @@ void hmp_migrate_set_capability(Monitor *mon, const 
QDict *qdict)
 }
 }
 
+void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
+{
+const char *param = qdict_get_str(qdict, parameter);
+int value = qdict_get_int(qdict, value);
+Error *err = NULL;
+MigrationParameterStatusList *params = g_malloc0(sizeof(*params));
+int i;
+
+for (i = 0; i  MIGRATION_PARAMETER_MAX; i++) {
+if (strcmp(param, MigrationParameter_lookup[i]) == 0) {
+params-value = g_malloc0(sizeof(*params-value));
+params-value-parameter = i;
+params-value-value = value;
+params-next = NULL;
+qmp_migrate_set_parameters(params, err);
+break;
+}
+}
+
+if (i == MIGRATION_PARAMETER_MAX) {
+error_set(err, QERR_INVALID_PARAMETER, param);
+}
+
+qapi_free_MigrationParameterStatusList(params);
+
+if (err) {
+monitor_printf(mon, migrate_set_parameter: %s\n,
+   error_get_pretty(err));
+error_free(err);
+}
+}
+
 void hmp_set_password(Monitor *mon, const QDict *qdict)
 {
 const char *protocol  = qdict_get_str(qdict, protocol);
diff --git a/hmp.h b/hmp.h
index 4bb5dca..bd1b203 100644
--- a/hmp.h
+++ b/hmp.h
@@ -63,6 +63,7 @@ void hmp_migrate_cancel(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_downtime(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict);
+void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict);
 void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict);
 void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
@@ -111,6 +112,8 @@ void watchdog_action_completion(ReadLineState *rs, int 
nb_args,
 const char *str);
 void migrate_set_capability_completion(ReadLineState *rs, int nb_args,
const char *str);
+void migrate_set_parameter_completion(ReadLineState *rs, int nb_args,
+   const char *str);
 void host_net_add_completion(ReadLineState *rs, int nb_args, const char *str);
 void host_net_remove_completion(ReadLineState *rs, int nb_args,
 const char *str);
diff --git a/include/migration/migration.h b/include/migration/migration.h
index 0c4f21c..8e09b42 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -50,9 +50,7 @@ struct MigrationState
 QEMUBH *cleanup_bh;
 QEMUFile *file;
 QemuThread *compress_thread;
-int compress_thread_count;
-int decompress_thread_count;
-int compress_level;
+int parameters[MIGRATION_PARAMETER_MAX];
 
 int state;
 MigrationParams params;
diff --git a/migration.c b/migration.c
index 9d1613d..d3d377e 100644
--- a/migration.c
+++ b/migration.c
@@ -66,9 +66,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,
-