[Qemu-devel] [PATCH] balloon: Fix device name in error message.

[Qemu-devel] [PATCH] balloon: Fix device name in error message.

[Hani Benhabiles]

Also fix trailing whitespace.

Signed-off-by: Hani Benhabiles <hani@linux.com>
---
 balloon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/balloon.c b/balloon.c
index e321f2c..b66710a 100644
--- a/balloon.c
+++ b/balloon.c
@@ -125,8 +125,8 @@ void qmp_balloon(int64_t value, Error **errp)
         error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
         return;
     }
-    
+
     if (qemu_balloon(value) == 0) {
-        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
+        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "virtio-balloon-pci");
     }
 }
-- 
1.8.3.2

Re: [Qemu-devel] [PATCH] balloon: Fix device name in error message.

[Markus Armbruster]

Hani Benhabiles <kroosec@gmail.com> writes:

> Also fix trailing whitespace.
>
> Signed-off-by: Hani Benhabiles <hani@linux.com>
> ---
>  balloon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/balloon.c b/balloon.c
> index e321f2c..b66710a 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -125,8 +125,8 @@ void qmp_balloon(int64_t value, Error **errp)
>          error_set(errp, QERR_INVALID_PARAMETER_VALUE, "target", "a size");
>          return;
>      }
> -    
> +
>      if (qemu_balloon(value) == 0) {
> -        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "balloon");
> +        error_set(errp, QERR_DEVICE_NOT_ACTIVE, "virtio-balloon-pci");
>      }
>  }

I agree the error message is bad, but I'm afraid your patch makes it
worse :)

qemu_balloon() returns zero when balloon_event_fn has not been set with
qemu_add_balloon_handler().

Right now, the only device that calls qemu_add_balloon_handler() is
virtio-balloon-device.  In theory, there could be others in the future.

virtio-balloon-device is a virtio-bus device.  The bus can be provided
by virtio-balloon-pci or virtio-balloon-ccw.

Your error message is misleading when it's provided by the latter.

Moreover, you missed qmp_query_balloon().

QERR_DEVICE_NOT_ACTIVE is also used with argument "spice", and the
resulting error message is similarly bad.  Perhaps something like this
would do:

#define QERR_DEVICE_NOT_ACTIVE \
    ERROR_CLASS_DEVICE_NOT_ACTIVE, "No %s device has been activated"

Re: [Qemu-devel] [PATCH] balloon: Fix device name in error message.

[Hani Benhabiles]

On Mon, Feb 24, 2014 at 10:04:05AM +0100, Markus Armbruster wrote:
> I agree the error message is bad, but I'm afraid your patch makes it
> worse :)
> 
> qemu_balloon() returns zero when balloon_event_fn has not been set with
> qemu_add_balloon_handler().
> 
> Right now, the only device that calls qemu_add_balloon_handler() is
> virtio-balloon-device.  In theory, there could be others in the future.
> 
> virtio-balloon-device is a virtio-bus device.  The bus can be provided
> by virtio-balloon-pci or virtio-balloon-ccw.
> 
> Your error message is misleading when it's provided by the latter.
> 

I see. Sorry for not throughly understanding the whole chain yet!

I somehow missed virtio-balloon-ccw, thus my false belief that
virtio-balloon-pci would be the only one to be ever needed/used.

> Moreover, you missed qmp_query_balloon().
> 
> QERR_DEVICE_NOT_ACTIVE is also used with argument "spice", and the
> resulting error message is similarly bad.  Perhaps something like this
> would do:
> 
> #define QERR_DEVICE_NOT_ACTIVE \
>     ERROR_CLASS_DEVICE_NOT_ACTIVE, "No %s device has been activated"

Sounds good to me. Will send a patch shortly.