[PATCH] qemu: Move some enums impl to qemu_monitor.c

Michal Privoznik posted 1 patch 2 years, 2 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/0ca1f4384a9664e1b22722a0602b166c09a512ce.1645456214.git.mprivozn@redhat.com
src/qemu/qemu_monitor.c      | 40 ++++++++++++++++++++++++++++++++++++
src/qemu/qemu_monitor_json.c | 34 ------------------------------
2 files changed, 40 insertions(+), 34 deletions(-)
[PATCH] qemu: Move some enums impl to qemu_monitor.c
Posted by Michal Privoznik 2 years, 2 months ago
There are some enums that are declared in qemu_monitor.h but
implemented in qemu_monitor_json.c. While from compiler and
linker POV it doesn't matter, the code is cleaner if an enum is
implemented in .c file that corresponds to .h file which declared
the enum.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_monitor.c      | 40 ++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_monitor_json.c | 34 ------------------------------
 2 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0ff938a577..8fc2a49abf 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -111,6 +111,38 @@ static int qemuMonitorOnceInit(void)
 
 VIR_ONCE_GLOBAL_INIT(qemuMonitor);
 
+VIR_ENUM_IMPL(qemuMonitorJob,
+              QEMU_MONITOR_JOB_TYPE_LAST,
+              "",
+              "commit",
+              "stream",
+              "mirror",
+              "backup",
+              "create",
+);
+
+VIR_ENUM_IMPL(qemuMonitorJobStatus,
+              QEMU_MONITOR_JOB_STATUS_LAST,
+              "",
+              "created",
+              "running",
+              "paused",
+              "ready",
+              "standby",
+              "waiting",
+              "pending",
+              "aborting",
+              "concluded",
+              "undefined",
+              "null",
+);
+
+VIR_ENUM_IMPL(qemuMonitorCPUProperty,
+              QEMU_MONITOR_CPU_PROPERTY_LAST,
+              "boolean",
+              "string",
+              "number",
+);
 
 VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
               QEMU_MONITOR_MIGRATION_STATUS_LAST,
@@ -4473,6 +4505,14 @@ qemuMonitorTransactionBackup(virJSONValue *actions,
 }
 
 
+VIR_ENUM_IMPL(qemuMonitorDirtyRateCalcMode,
+              QEMU_MONITOR_DIRTYRATE_CALC_MODE_LAST,
+              "page-sampling",
+              "dirty-bitmap",
+              "dirty-ring",
+);
+
+
 int
 qemuMonitorStartDirtyRateCalc(qemuMonitor *mon,
                               int seconds,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 345b81cd12..4d339f29b8 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -53,30 +53,6 @@ VIR_LOG_INIT("qemu.qemu_monitor_json");
 
 #define LINE_ENDING "\r\n"
 
-VIR_ENUM_IMPL(qemuMonitorJob,
-              QEMU_MONITOR_JOB_TYPE_LAST,
-              "",
-              "commit",
-              "stream",
-              "mirror",
-              "backup",
-              "create");
-
-VIR_ENUM_IMPL(qemuMonitorJobStatus,
-              QEMU_MONITOR_JOB_STATUS_LAST,
-              "",
-              "created",
-              "running",
-              "paused",
-              "ready",
-              "standby",
-              "waiting",
-              "pending",
-              "aborting",
-              "concluded",
-              "undefined",
-              "null");
-
 static void qemuMonitorJSONHandleShutdown(qemuMonitor *mon, virJSONValue *data);
 static void qemuMonitorJSONHandleReset(qemuMonitor *mon, virJSONValue *data);
 static void qemuMonitorJSONHandleStop(qemuMonitor *mon, virJSONValue *data);
@@ -5347,11 +5323,6 @@ qemuMonitorJSONGetCPUDefinitions(qemuMonitor *mon,
 }
 
 
-VIR_ENUM_IMPL(qemuMonitorCPUProperty,
-              QEMU_MONITOR_CPU_PROPERTY_LAST,
-              "boolean", "string", "number",
-);
-
 static int
 qemuMonitorJSONParseCPUModelProperty(const char *key,
                                      virJSONValue *value,
@@ -8740,11 +8711,6 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitor *mon,
                                   migratable);
 }
 
-VIR_ENUM_IMPL(qemuMonitorDirtyRateCalcMode,
-              QEMU_MONITOR_DIRTYRATE_CALC_MODE_LAST,
-              "page-sampling",
-              "dirty-bitmap",
-              "dirty-ring");
 
 int
 qemuMonitorJSONStartDirtyRateCalc(qemuMonitor *mon,
-- 
2.34.1

Re: [PATCH] qemu: Move some enums impl to qemu_monitor.c
Posted by Andrea Bolognani 2 years, 2 months ago
On Mon, Feb 21, 2022 at 04:10:25PM +0100, Michal Privoznik wrote:
> +VIR_ENUM_IMPL(qemuMonitorCPUProperty,
> +              QEMU_MONITOR_CPU_PROPERTY_LAST,
> +              "boolean",
> +              "string",
> +              "number",
> +);
>
>  VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
>                QEMU_MONITOR_MIGRATION_STATUS_LAST,
> @@ -4473,6 +4505,14 @@ qemuMonitorTransactionBackup(virJSONValue *actions,
>  }
>
>
> +VIR_ENUM_IMPL(qemuMonitorDirtyRateCalcMode,
> +              QEMU_MONITOR_DIRTYRATE_CALC_MODE_LAST,
> +              "page-sampling",
> +              "dirty-bitmap",
> +              "dirty-ring",
> +);
> +
> +

Kinda weird that this one ends up in the middle of the file instead
of being grouped with the rest. I'd keep them together, unless
there's a good reason for doing things this way that I missed.

Either way,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

Re: [PATCH] qemu: Move some enums impl to qemu_monitor.c
Posted by Michal Prívozník 2 years, 2 months ago
On 2/21/22 17:17, Andrea Bolognani wrote:
> On Mon, Feb 21, 2022 at 04:10:25PM +0100, Michal Privoznik wrote:
>> +VIR_ENUM_IMPL(qemuMonitorCPUProperty,
>> +              QEMU_MONITOR_CPU_PROPERTY_LAST,
>> +              "boolean",
>> +              "string",
>> +              "number",
>> +);
>>
>>  VIR_ENUM_IMPL(qemuMonitorMigrationStatus,
>>                QEMU_MONITOR_MIGRATION_STATUS_LAST,
>> @@ -4473,6 +4505,14 @@ qemuMonitorTransactionBackup(virJSONValue *actions,
>>  }
>>
>>
>> +VIR_ENUM_IMPL(qemuMonitorDirtyRateCalcMode,
>> +              QEMU_MONITOR_DIRTYRATE_CALC_MODE_LAST,
>> +              "page-sampling",
>> +              "dirty-bitmap",
>> +              "dirty-ring",
>> +);
>> +
>> +
> 
> Kinda weird that this one ends up in the middle of the file instead
> of being grouped with the rest. I'd keep them together, unless
> there's a good reason for doing things this way that I missed.

My idea was to keep it close to the rest of dirty rate related
functions. But I can move it next to the rest. I don't have strong
preference.

> 
> Either way,
> 
>   Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> 

Thanks, I'll wait for your reply before pushing.

Michal

Re: [PATCH] qemu: Move some enums impl to qemu_monitor.c
Posted by Andrea Bolognani 2 years, 2 months ago
On Tue, Feb 22, 2022 at 08:38:10AM +0100, Michal Prívozník wrote:
> On 2/21/22 17:17, Andrea Bolognani wrote:
> > On Mon, Feb 21, 2022 at 04:10:25PM +0100, Michal Privoznik wrote:
> >> +VIR_ENUM_IMPL(qemuMonitorDirtyRateCalcMode,
> >> +              QEMU_MONITOR_DIRTYRATE_CALC_MODE_LAST,
> >> +              "page-sampling",
> >> +              "dirty-bitmap",
> >> +              "dirty-ring",
> >> +);
> >
> > Kinda weird that this one ends up in the middle of the file instead
> > of being grouped with the rest. I'd keep them together, unless
> > there's a good reason for doing things this way that I missed.
>
> My idea was to keep it close to the rest of dirty rate related
> functions. But I can move it next to the rest. I don't have strong
> preference.

I don't have a strong preference either. Just push it as is :)

-- 
Andrea Bolognani / Red Hat / Virtualization