[libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Depending on the memballoon model, the corresponding QOM node will have a different type and we need to account for this when searching for it in the QOM tree. https://bugzilla.redhat.com/show_bug.cgi?id=1911786 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b5c0364652..c97dadd11e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1017,7 +1017,27 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon, switch (balloon->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - name = "virtio-balloon-pci"; + switch (balloon->model) { + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: + name = "virtio-balloon-pci"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL: + name = "virtio-balloon-pci-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL: + name = "virtio-balloon-pci-non-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: + case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid model for virtio-balloon-pci")); + return; + case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return; + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: name = "virtio-balloon-ccw"; -- 2.26.2
On 1/12/21 2:47 PM, Andrea Bolognani wrote:
Depending on the memballoon model, the corresponding QOM node will have a different type and we need to account for this when searching for it in the QOM tree.
https://bugzilla.redhat.com/show_bug.cgi?id=1911786 Signed-off-by: Andrea Bolognani <abologna@redhat.com> ---
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b5c0364652..c97dadd11e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1017,7 +1017,27 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon,
switch (balloon->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - name = "virtio-balloon-pci"; + switch (balloon->model) { + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: + name = "virtio-balloon-pci"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL: + name = "virtio-balloon-pci-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL: + name = "virtio-balloon-pci-non-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: + case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid model for virtio-balloon-pci")); + return; + case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return; + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: name = "virtio-balloon-ccw";
On Tue, Jan 12, 2021 at 18:47:36 +0100, Andrea Bolognani wrote:
Depending on the memballoon model, the corresponding QOM node will have a different type and we need to account for this when searching for it in the QOM tree.
https://bugzilla.redhat.com/show_bug.cgi?id=1911786 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b5c0364652..c97dadd11e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1017,7 +1017,27 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon,
switch (balloon->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - name = "virtio-balloon-pci"; + switch (balloon->model) {
This is an 'int'. Please typecast it appropriately ...
+ case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: + name = "virtio-balloon-pci"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL: + name = "virtio-balloon-pci-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL: + name = "virtio-balloon-pci-non-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: + case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid model for virtio-balloon-pci")); + return; + case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return;
... so that this actually makes sense and is kept up to date. Also, reporting errors in a function which doesn't abort execution on said error is dubious.
+ } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW: name = "virtio-balloon-ccw"; -- 2.26.2
On Tue, Jan 12, 2021 at 19:56:11 +0100, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 18:47:36 +0100, Andrea Bolognani wrote:
Depending on the memballoon model, the corresponding QOM node will have a different type and we need to account for this when searching for it in the QOM tree.
https://bugzilla.redhat.com/show_bug.cgi?id=1911786 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
[...]
+ case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return;
[...]
Also, reporting errors in a function which doesn't abort execution on said error is dubious.
Disregard this. Multiple places in this function have this dubious error reporting.
On Tue, 2021-01-12 at 19:56 +0100, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 18:47:36 +0100, Andrea Bolognani wrote:
+ switch (balloon->model) {
This is an 'int'. Please typecast it appropriately ...
+ case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO: + name = "virtio-balloon-pci"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_TRANSITIONAL: + name = "virtio-balloon-pci-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO_NON_TRANSITIONAL: + name = "virtio-balloon-pci-non-transitional"; + break; + case VIR_DOMAIN_MEMBALLOON_MODEL_XEN: + case VIR_DOMAIN_MEMBALLOON_MODEL_NONE: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("invalid model for virtio-balloon-pci")); + return; + case VIR_DOMAIN_MEMBALLOON_MODEL_LAST: + default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return;
... so that this actually makes sense and is kept up to date.
Right you are! Very good catch, thank you :) Do you want me to respin, or can I just add the virDomainMemballoonModel cast before pushing? -- Andrea Bolognani / Red Hat / Virtualization
On Wed, Jan 13, 2021 at 10:18:35 +0100, Andrea Bolognani wrote:
On Tue, 2021-01-12 at 19:56 +0100, Peter Krempa wrote:
On Tue, Jan 12, 2021 at 18:47:36 +0100, Andrea Bolognani wrote:
+ switch (balloon->model) {
[...]
+ default: + virReportEnumRangeError(virDomainMemballoonModel, + balloon->model); + return;
... so that this actually makes sense and is kept up to date.
Right you are! Very good catch, thank you :)
Do you want me to respin, or can I just add the virDomainMemballoonModel cast before pushing?
No need to respin for this.
On 1/12/21 6:47 PM, Andrea Bolognani wrote:
Depending on the memballoon model, the corresponding QOM node will have a different type and we need to account for this when searching for it in the QOM tree.
https://bugzilla.redhat.com/show_bug.cgi?id=1911786 Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index b5c0364652..c97dadd11e 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1017,7 +1017,27 @@ qemuMonitorInitBalloonObjectPath(qemuMonitorPtr mon,
switch (balloon->info.type) { case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: - name = "virtio-balloon-pci"; + switch (balloon->model) {
If you typecast this as Peter suggests then: Reviewed-by: Michal Privoznik <mprivozn@redhat.com> and safe for freeze. Michal
participants (4)
-
Andrea Bolognani -
Daniel Henrique Barboza -
Michal Privoznik -
Peter Krempa