Re: [libvirt] [PATCH v2 56/73] qemu: Refactor qemuMigrationParams

[Ján Tomko]

On Wed, Apr 11, 2018 at 04:41:46PM +0200, Jiri Denemark wrote:

Adding support for new migration parameter requires a lot of places to
be changed (most likely by copy engineering): new variables to
store the parameter value and the associated *_set bool, JSON formatter
and parser, XML formatter and parser (to be added soon), and the actual
code to set the parameter. It's pretty easy to forget about some of the
places which need to be updated and end up with incorrect support. The
goal of this patch is to let most of the places do their job without any
modifications when new parameters are added.

To achieve the goal, a new qemuMigrationParam enum is introduced and all
parameters are stored in an array indexed by the items of this enum.
This will also allow us to automatically set the migration parameters
which directly correspond to libvirt's typed parameters accepted by
virDomainMigrate* APIs.

Signed-off-by: Jiri Denemark 
---
src/qemu/qemu_migration_params.c | 470 ---
src/qemu/qemu_migration_params.h |  20 ++
2 files changed, 328 insertions(+), 162 deletions(-)



Reviewed-by: Ján Tomko 

Jano


signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v2 56/73] qemu: Refactor qemuMigrationParams

[Jiri Denemark]

Adding support for new migration parameter requires a lot of places to
be changed (most likely by copy engineering): new variables to
store the parameter value and the associated *_set bool, JSON formatter
and parser, XML formatter and parser (to be added soon), and the actual
code to set the parameter. It's pretty easy to forget about some of the
places which need to be updated and end up with incorrect support. The
goal of this patch is to let most of the places do their job without any
modifications when new parameters are added.

To achieve the goal, a new qemuMigrationParam enum is introduced and all
parameters are stored in an array indexed by the items of this enum.
This will also allow us to automatically set the migration parameters
which directly correspond to libvirt's typed parameters accepted by
virDomainMigrate* APIs.

Signed-off-by: Jiri Denemark 
---
 src/qemu/qemu_migration_params.c | 470 ---
 src/qemu/qemu_migration_params.h |  20 ++
 2 files changed, 328 insertions(+), 162 deletions(-)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index 8027896c12..77abc7191f 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -39,46 +39,29 @@ VIR_LOG_INIT("qemu.qemu_migration_params");
 
 #define QEMU_MIGRATION_TLS_ALIAS_BASE "libvirt_migrate"
 
-typedef struct _qemuMonitorMigrationParams qemuMonitorMigrationParams;
-typedef qemuMonitorMigrationParams *qemuMonitorMigrationParamsPtr;
-struct _qemuMonitorMigrationParams {
-bool compressLevel_set;
-int compressLevel;
+typedef enum {
+QEMU_MIGRATION_PARAM_TYPE_INT,
+QEMU_MIGRATION_PARAM_TYPE_ULL,
+QEMU_MIGRATION_PARAM_TYPE_BOOL,
+QEMU_MIGRATION_PARAM_TYPE_STRING,
+} qemuMigrationParamType;
 
-bool compressThreads_set;
-int compressThreads;
-
-bool decompressThreads_set;
-int decompressThreads;
-
-bool cpuThrottleInitial_set;
-int cpuThrottleInitial;
-
-bool cpuThrottleIncrement_set;
-int cpuThrottleIncrement;
-
-/* Value is either NULL, "", or some string. NULL indicates no support;
- * whereas, some string value indicates we can support setting/clearing */
-char *tlsCreds;
-char *tlsHostname;
-
-bool maxBandwidth_set;
-unsigned long long maxBandwidth;
-
-bool downtimeLimit_set;
-unsigned long long downtimeLimit;
-
-bool blockIncremental_set;
-bool blockIncremental;
-
-bool xbzrleCacheSize_set;
-unsigned long long xbzrleCacheSize;
+typedef struct _qemuMigrationParamValue qemuMigrationParamValue;
+typedef qemuMigrationParamValue *qemuMigrationParamValuePtr;
+struct _qemuMigrationParamValue {
+bool set;
+union {
+int i; /* exempt from syntax-check */
+unsigned long long ull;
+bool b;
+char *s;
+} value;
 };
 
 struct _qemuMigrationParams {
 unsigned long long compMethods; /* bit-wise OR of 
qemuMigrationCompressMethod */
 virBitmapPtr caps;
-qemuMonitorMigrationParams params;
+qemuMigrationParamValue params[QEMU_MIGRATION_PARAM_LAST];
 };
 
 typedef enum {
@@ -93,6 +76,20 @@ VIR_ENUM_IMPL(qemuMigrationCompressMethod, 
QEMU_MIGRATION_COMPRESS_LAST,
   "mt",
 );
 
+VIR_ENUM_DECL(qemuMigrationParam)
+VIR_ENUM_IMPL(qemuMigrationParam, QEMU_MIGRATION_PARAM_LAST,
+  "compress-level",
+  "compress-threads",
+  "decompress-threads",
+  "cpu-throttle-initial",
+  "cpu-throttle-increment",
+  "tls-creds",
+  "tls-hostname",
+  "max-bandwidth",
+  "downtime-limit",
+  "block-incremental",
+  "xbzrle-cache-size",
+);
 
 typedef struct _qemuMigrationParamsAlwaysOnItem 
qemuMigrationParamsAlwaysOnItem;
 struct _qemuMigrationParamsAlwaysOnItem {
@@ -129,6 +126,21 @@ static const qemuMigrationParamsFlagMapItem 
qemuMigrationParamsFlagMap[] = {
  QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION},
 };
 
+static const qemuMigrationParamType qemuMigrationParamTypes[] = {
+[QEMU_MIGRATION_PARAM_COMPRESS_LEVEL] = QEMU_MIGRATION_PARAM_TYPE_INT,
+[QEMU_MIGRATION_PARAM_COMPRESS_THREADS] = QEMU_MIGRATION_PARAM_TYPE_INT,
+[QEMU_MIGRATION_PARAM_DECOMPRESS_THREADS] = QEMU_MIGRATION_PARAM_TYPE_INT,
+[QEMU_MIGRATION_PARAM_THROTTLE_INITIAL] = QEMU_MIGRATION_PARAM_TYPE_INT,
+[QEMU_MIGRATION_PARAM_THROTTLE_INCREMENT] = QEMU_MIGRATION_PARAM_TYPE_INT,
+[QEMU_MIGRATION_PARAM_TLS_CREDS] = QEMU_MIGRATION_PARAM_TYPE_STRING,
+[QEMU_MIGRATION_PARAM_TLS_HOSTNAME] = QEMU_MIGRATION_PARAM_TYPE_STRING,
+[QEMU_MIGRATION_PARAM_MAX_BANDWIDTH] = QEMU_MIGRATION_PARAM_TYPE_ULL,
+[QEMU_MIGRATION_PARAM_DOWNTIME_LIMIT] = QEMU_MIGRATION_PARAM_TYPE_ULL,
+[QEMU_MIGRATION_PARAM_BLOCK_INCREMENTAL] = QEMU_MIGRATION_PARAM_TYPE_BOOL,
+[QEMU_MIGRATION_PARAM_XBZRLE_CACHE_SIZE] = QEMU_MIGRATION_PARAM_TYPE_ULL,
+};