balloon: error if guest driver is not initialised
cover balloon: error if guest driver is not initialised
Hello,
These patches make qemu generate an error on failure in setting a balloon value:
(qemu) balloon 400
Device 'balloon' has not been activated
This can happen when the guest hasn't yet loaded a balloon driver.
Previously, such a ballooning attempt didn't produce an error, but the virtio-balloon driver remembered the value nevertheless, causing a module load in the guest using the attempted balloon value originally. This may not be the desired course of action.
I've re-used the QERR_DEVICE_NOT_ACTIVE error message, I think it fits in this case too. If not, let me know and I'll change it to something else.
Please review and apply.
Amit Shah (3):
balloon: Make functions return 0 on OK, -1 on error.
balloon: report error if ballooning operation fails
virtio-balloon: report error if balloon driver in guest not available
balloon.c | 19 ++++++++++++------- balloon.h | 2 +- hw/virtio-balloon.c | 6 +++++- 3 files changed, 18 insertions(+), 9 deletions(-)
Signature · 1 line
1.7.7.3
PATCH 1/3 balloon: Make functions return 0 on OK, -1 on error.
Current semantics of 1 on OK and 0 on error are slightly weird. qemu_balloon() and qemu_balloon_stats() do this. Other functions in the file use the standard 0 and -1 return values. This commit makes the file consistent in returning such values.
--- balloon.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
balloon.c +8 -6
@@ -64,25 +64,26 @@ void qemu_remove_balloon_handler(void *opaque)static int qemu_balloon (ram_addr_t target) {if (!balloon_event_fn) { -return 0 ; +return -1 ; }trace_balloon_event (balloon_opaque, target);balloon_event_fn (balloon_opaque, target); -return 1 ; +return 0 ; }static int qemu_balloon_status (BalloonInfo *info) {if (!balloon_stat_fn) { -return 0 ; +return -1 ; }balloon_stat_fn (balloon_opaque, info); -return 1 ; +return 0 ; }BalloonInfo *qmp_query_balloon (Error **errp) {BalloonInfo *info; +int ret;if (kvm_enabled () && !kvm_has_sync_mmu ()) {error_set (errp,QERR_KVM_MISSING_CAP ,"synchronous MMU" ,"balloon" );
@@ -91,7 +92,8 @@ BalloonInfo *qmp_query_balloon(Error **errp) info =g_malloc0 (sizeof (*info)); -if (qemu_balloon_status (info) ==0 ) { + ret =qemu_balloon_status (info); +if (ret <0 ) {error_set (errp,QERR_DEVICE_NOT_ACTIVE ,"balloon" );qapi_free_BalloonInfo (info);return NULL ;
@@ -120,7 +122,7 @@ int do_balloon(Monitor *mon, const QDict *params,return -1 ; } ret =qemu_balloon (target); -if (ret ==0 ) { +if (ret <0 ) {qerror_report (QERR_DEVICE_NOT_ACTIVE ,"balloon" );return -1 ; }
Signature · 1 line
1.7.7.3
PATCH 2/3 balloon: report error if ballooning operation fails
In reply to Amit ShahBallooning operations can fail (e.g. driver in guest not available). Let the user know of such an error condition instead of silently ignoring errors.
--- balloon.c | 7 +++++-- balloon.h | 2 +- hw/virtio-balloon.c | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-)
balloon.c +5 -2
@@ -63,12 +63,15 @@ void qemu_remove_balloon_handler(void *opaque)static int qemu_balloon (ram_addr_t target) { +int ret; +if (!balloon_event_fn) {return -1 ; }trace_balloon_event (balloon_opaque, target); -balloon_event_fn (balloon_opaque, target); -return 0 ; + ret =balloon_event_fn (balloon_opaque, target); + +return ret <0 ?-1 :0 ; }static int qemu_balloon_status (BalloonInfo *info)
balloon.h +1 -1
@@ -17,7 +17,7 @@#include "monitor.h" #include "qapi-types.h" -typedef void (QEMUBalloonEvent )(void *opaque,ram_addr_t target); +typedef int (QEMUBalloonEvent )(void *opaque,ram_addr_t target);typedef void (QEMUBalloonStatus )(void *opaque,BalloonInfo *info); int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
hw/virtio-balloon.c +2 -1
@@ -180,7 +180,7 @@ static void virtio_balloon_stat(void *opaque, BalloonInfo *info) VIRTIO_BALLOON_PFN_SHIFT); } -static void virtio_balloon_to_target (void *opaque,ram_addr_t target) +static int virtio_balloon_to_target (void *opaque,ram_addr_t target) {VirtIOBalloon *dev = opaque;
@@ -191,6 +191,7 @@ static void virtio_balloon_to_target(void *opaque, ram_addr_t target) dev->num_pages = (ram_size - target) >>VIRTIO_BALLOON_PFN_SHIFT ;virtio_notify_config (&dev->vdev); } +return 0 ; }static void virtio_balloon_save (QEMUFile *f,void *opaque)
Signature · 1 line
1.7.7.3
PATCH 3/3 virtio-balloon: report error if balloon driver in guest not available
In reply to Amit ShahThe guest may not have the balloon driver initialised or may have it disabled. Report an error to the user in such a case when a request for ballooning arrives.
This also solves another issue where a previous request for ballooning failed (e.g., for the reason mentioned above), but the config was still updated, resulting in the ballooning value being applied to the guest after the guest loads the balloon module. This may not be the desirable thing to do at this later stage.
--- hw/virtio-balloon.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
hw/virtio-balloon.c +3 -0
@@ -184,6 +184,9 @@ static int virtio_balloon_to_target(void *opaque, ram_addr_t target) {VirtIOBalloon *dev = opaque; +if (!(dev->vdev.status &VIRTIO_CONFIG_S_DRIVER_OK )) { +return -ENODEV ; + }if (target > ram_size) { target = ram_size; }
Signature · 1 line
1.7.7.3
Quoted 9 lines · Amit Shah
Hello,
These patches make qemu generate an error on failure in setting a balloon value:
(qemu) balloon 400
Device 'balloon' has not been activatedThis can happen when the guest hasn't yet loaded a balloon driver.
This is a pretty significant change in behaviour/semantics of the balloon driver interface.
eg, when libvirt starts a guest, it launches QEMU paused, and immediately sets a balloon target, which may be lower than the memory limit, and then starts CPUS to let the guest boot. The guest will utilize this target when it loads the balloon driver during bootup.
With this proposed change, libvirt will immediately get an error, and have to guess when the balloon driver might be loaded by the guest, in order to set the limit post-boot.
Quoted 4 lines
Previously, such a ballooning attempt didn't produce an error, but the virtio-balloon driver remembered the value nevertheless, causing a module load in the guest using the attempted balloon value originally. This may not be the desired course of action.
This is *exactly* desired behaviour. Please don't change this.
Regards,
Daniel
Signature · 4 lines
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Quoted 24 lines · Daniel P. Berrange
Quoted 9 lines · Amit Shah
Hello,
These patches make qemu generate an error on failure in setting a balloon value:
(qemu) balloon 400
Device 'balloon' has not been activatedThis can happen when the guest hasn't yet loaded a balloon driver.
This is a pretty significant change in behaviour/semantics of the balloon driver interface.
eg, when libvirt starts a guest, it launches QEMU paused, and immediately sets a balloon target, which may be lower than the memory limit, and then starts CPUS to let the guest boot. The guest will utilize this target when it loads the balloon driver during bootup.
With this proposed change, libvirt will immediately get an error, and have to guess when the balloon driver might be loaded by the guest, in order to set the limit post-boot.
Quoted 6 lines
Quoted 4 lines
Previously, such a ballooning attempt didn't produce an error, but the virtio-balloon driver remembered the value nevertheless, causing a module load in the guest using the attempted balloon value originally. This may not be the desired course of action.
This is *exactly* desired behaviour. Please don't change this.
OK, thanks.
I'll retract this patch series.
Amit