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

[Kristina Hanicova]

On Mon, Feb 27, 2023 at 2:04 PM Michal Prívozník <mprivozn at redhat.com>
wrote:

> On 2/27/23 13:49, Kristina Hanicova wrote:
> >
> >
> > On Mon, Feb 27, 2023 at 12:35 PM Michal Privoznik <mprivozn at redhat.com
> > <mailto:mprivozn at redhat.com>> 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 at redhat.com
> >     <mailto:mprivozn at redhat.com>>
> >     ---
> >      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 at redhat.com>

Kristina
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230228/efcc908b/attachment-0001.htm>