| Message ID | 1393164341-7196-1-git-send-email-kroosec@gmail.com |
|---|---|
| State | New |
| Headers | show |
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"
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.
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"); } }
Also fix trailing whitespace. Signed-off-by: Hani Benhabiles <hani@linux.com> --- balloon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)