Re: [PATCH 3/4] qemu_monitor: Switch to virDomainMemoryModel enum in qemuMonitorJSONGetMemoryDeviceInfo()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On Mon, Feb 27, 2023 at 2:04 PM Michal Prívozník <mprivozn@xxxxxxxxxx> wrote:
On 2/27/23 13:49, Kristina Hanicova wrote:
>
>
> On Mon, Feb 27, 2023 at 12:35 PM Michal Privoznik <mprivozn@xxxxxxxxxx
> <mailto:mprivozn@xxxxxxxxxx>> wrote:
>
>     When processing memory devices (as a reply from QEMU), a bunch of
>     STREQ()-s is used. Fortunately, the set of strings we process is
>     the same as virDomainMemoryModel enum. Therefore, we can use
>     virDomainMemoryModelTypeFromString() and when use integer

*then
 
>     comparison (well, switch()). This has an up side, that
>     introducing a new memory model let's us see immediately at
>     compile time, what places need adjusting.

I would rewrite the last sentence into:
"This has an upside: introducing a new memory model lets us see what places need adjusting immediately at compile time."
 
>
>     NB, this is in contrast with cmd line generator
>     (qemuBuildMemoryDeviceProps()), where more specific models are
>     generated (e.g. "pc-dimm", "virtio-mem-pci", etc.). But QEMU
>     reports back the parent model, instead of specific child
>     instance.
>
>     Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx
>     <mailto:mprivozn@xxxxxxxxxx>>
>     ---
>      src/qemu/qemu_monitor_json.c | 52 +++++++++++++++++++++++++-----------
>      1 file changed, 37 insertions(+), 15 deletions(-)
>
>
>
> Seems that  I can't compile this patch - compiler is sad that devalias
> may be used uninitialized:
>
> ../src/qemu/qemu_monitor_json.c: In function
> ‘qemuMonitorJSONGetMemoryDeviceInfo’:
> ../src/qemu/qemu_monitor_json.c:7333:13: error: ‘devalias’ may be used
> uninitialized [-Werror=maybe-uninitialized]
>  7333 |         if (virHashAddEntry(info, devalias, meminfo) < 0)
>       |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../src/qemu/qemu_monitor_json.c:7213:21: note: ‘devalias’ was declared here
>  7213 |         const char *devalias;
>
> Using gcc 12.2.1

Oh right. Thank you for catching this. I'm building with -O0 by default
(because I want to be able to debug libvirtd), which apparently enables
gcc understand the code. Without it, (maybe) it enables some
optimizations which make it incapable of understanding the code. Pity.
Anyway, consider this squashed in:

diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c
index a5e525b7ce..5ed1f9442e 100644
--- i/src/qemu/qemu_monitor_json.c
+++ w/src/qemu/qemu_monitor_json.c
@@ -7213 +7213 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon,
-        const char *devalias;
+        const char *devalias = NULL;



In that case,

Reviewed-by: Kristina Hanicova <khanicov@xxxxxxxxxx>

Kristina

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux