1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon

Andrea Bolognani posted 1 patch 3 years, 3 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210112174736.120308-1-abologna@redhat.com
src/qemu/qemu_monitor.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
[libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Andrea Bolognani 3 years, 3 months ago 
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
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Daniel Henrique Barboza 3 years, 3 months ago 

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";
>
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Michal Privoznik 3 years, 3 months ago 
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
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Peter Krempa 3 years, 3 months ago 
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
>
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Peter Krempa 3 years, 3 months ago 
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.
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Andrea Bolognani 3 years, 3 months ago 
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
Re: [libvirt PATCH] qemu: Fix memstat for (non-)transitional memballoon
Posted by Peter Krempa 3 years, 3 months ago 
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.

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.