<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Feb 27, 2023 at 2:04 PM Michal Prívozník <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/27/23 13:49, Kristina Hanicova wrote:<br>
> <br>
> <br>
> On Mon, Feb 27, 2023 at 12:35 PM Michal Privoznik <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a><br>
> <mailto:<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>>> wrote:<br>
> <br>
> When processing memory devices (as a reply from QEMU), a bunch of<br>
> STREQ()-s is used. Fortunately, the set of strings we process is<br>
> the same as virDomainMemoryModel enum. Therefore, we can use<br>
> virDomainMemoryModelTypeFromString() and when use integer<br></blockquote><div><br></div><div>*then<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> comparison (well, switch()). This has an up side, that<br>
> introducing a new memory model let's us see immediately at<br>
> compile time, what places need adjusting.<br></blockquote><div><br></div><div>I would rewrite the last sentence into:</div><div><span class="gmail-mark_underline_27_3|h%0 gmail-alerts-clarity gmail-alerts-markBase gmail-fuoslpi gmail-fc6fojp gmail-alerts-markHeightStandard gmail-alerts-markSelected gmail-alerts-markSelectedFocused gmail-alerts-markVisible gmail-fysyo2 gmail-fdltgmf">"This</span> has an upside: introducing a new memory model lets us see what places need adjusting immediately at compile time."</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
> <br>
> NB, this is in contrast with cmd line generator<br>
> (qemuBuildMemoryDeviceProps()), where more specific models are<br>
> generated (e.g. "pc-dimm", "virtio-mem-pci", etc.). But QEMU<br>
> reports back the parent model, instead of specific child<br>
> instance.<br>
> <br>
> Signed-off-by: Michal Privoznik <<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a><br>
> <mailto:<a href="mailto:mprivozn@redhat.com" target="_blank">mprivozn@redhat.com</a>>><br>
> ---<br>
> src/qemu/qemu_monitor_json.c | 52 +++++++++++++++++++++++++-----------<br>
> 1 file changed, 37 insertions(+), 15 deletions(-)<br>
> <br>
> <br>
> <br>
> Seems that I can't compile this patch - compiler is sad that devalias<br>
> may be used uninitialized:<br>
> <br>
> ../src/qemu/qemu_monitor_json.c: In function<br>
> ‘qemuMonitorJSONGetMemoryDeviceInfo’:<br>
> ../src/qemu/qemu_monitor_json.c:7333:13: error: ‘devalias’ may be used<br>
> uninitialized [-Werror=maybe-uninitialized]<br>
> 7333 | if (virHashAddEntry(info, devalias, meminfo) < 0)<br>
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<br>
> ../src/qemu/qemu_monitor_json.c:7213:21: note: ‘devalias’ was declared here<br>
> 7213 | const char *devalias;<br>
> <br>
> Using gcc 12.2.1<br>
<br>
Oh right. Thank you for catching this. I'm building with -O0 by default<br>
(because I want to be able to debug libvirtd), which apparently enables<br>
gcc understand the code. Without it, (maybe) it enables some<br>
optimizations which make it incapable of understanding the code. Pity.<br>
Anyway, consider this squashed in:<br>
<br>
diff --git i/src/qemu/qemu_monitor_json.c w/src/qemu/qemu_monitor_json.c<br>
index a5e525b7ce..5ed1f9442e 100644<br>
--- i/src/qemu/qemu_monitor_json.c<br>
+++ w/src/qemu/qemu_monitor_json.c<br>
@@ -7213 +7213 @@ qemuMonitorJSONGetMemoryDeviceInfo(qemuMonitor *mon,<br>
- const char *devalias;<br>
+ const char *devalias = NULL;<br>
<br>
<br></blockquote><div><br></div><div>In that case,</div><div><br></div><div>Reviewed-by: Kristina Hanicova <<a href="mailto:khanicov@redhat.com" target="_blank">khanicov@redhat.com</a>> <br></div><div><br></div><div>Kristina<br></div></div></div>