Linux Virtualization - Apr 2012 - [PATCH 0/3] virtio: balloon: cleanups and a fix

Hello,

The main fix is to update the host with the current balloon value on
module removal after deflating the balloon.  Without the fix, the host
has the wrong idea of the ballooned memory in the guest.  This is
patch 2.

Patches 1 and 3 are cleanups with no effective code change.

Please apply,

Amit Shah (3):
  virtio: balloon: drop restore_common()
  virtio: balloon: let host know of updated balloon size before module
    removal
  virtio: balloon: separate out common code between remove and freeze
    functions

 drivers/virtio/virtio_balloon.c |   34 ++++++++++++++--------------------
 1 files changed, 14 insertions(+), 20 deletions(-)

-- 
1.7.7.6

restore_common() was used when there were different thaw and freeze PM
callbacks implemented.  We removed thaw in commit
f38f8387cbdc4138a492ce9f2a5f04fd3cd3cf33.

restore_common() can be removed and virtballoon_restore() can itself do
the restore ops.

Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/virtio/virtio_balloon.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index c2d05a8..6921326 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -418,7 +418,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 	return 0;
 }
 
-static int restore_common(struct virtio_device *vdev)
+static int virtballoon_restore(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb = vdev->priv;
 	int ret;
@@ -431,11 +431,6 @@ static int restore_common(struct virtio_device *vdev)
 	update_balloon_size(vb);
 	return 0;
 }
-
-static int virtballoon_restore(struct virtio_device *vdev)
-{
-	return restore_common(vdev);
-}
 #endif
 
 static unsigned int features[] = {
-- 
1.7.7.6

When the balloon module is removed, we deflate the balloon, reclaiming
all the pages that were given to the host.  However, we don't update the
config values for the new balloon size, resulting in the host showing
outdated balloon values.

The size update is done after each leak and fill operation, only the
module removal case was left out.

Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/virtio/virtio_balloon.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 6921326..04baad6 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -390,6 +390,7 @@ static void __devexit virtballoon_remove(struct
virtio_device *vdev)
 	/* There might be pages left in the balloon: free them. */
 	while (vb->num_pages)
 		leak_balloon(vb, vb->num_pages);
+	update_balloon_size(vb);
 
 	/* Now we reset the device so we can clean up the queues. */
 	vdev->config->reset(vdev);
-- 
1.7.7.6

The remove and freeze functions have a lot of shared code; put it into a
common function that gets called by both.

Signed-off-by: Amit Shah <amit.shah at redhat.com>
---
 drivers/virtio/virtio_balloon.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 04baad6..bfbc15c 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -381,21 +381,25 @@ out:
 	return err;
 }
 
-static void __devexit virtballoon_remove(struct virtio_device *vdev)
+static void remove_common(struct virtio_balloon *vb)
 {
-	struct virtio_balloon *vb = vdev->priv;
-
-	kthread_stop(vb->thread);
-
 	/* There might be pages left in the balloon: free them. */
 	while (vb->num_pages)
 		leak_balloon(vb, vb->num_pages);
 	update_balloon_size(vb);
 
 	/* Now we reset the device so we can clean up the queues. */
-	vdev->config->reset(vdev);
+	vb->vdev->config->reset(vb->vdev);
 
-	vdev->config->del_vqs(vdev);
+	vb->vdev->config->del_vqs(vb->vdev);
+}
+
+static void __devexit virtballoon_remove(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = vdev->priv;
+
+	kthread_stop(vb->thread);
+	remove_common(vb);
 	kfree(vb);
 }
 
@@ -409,13 +413,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 	 * function is called.
 	 */
 
-	while (vb->num_pages)
-		leak_balloon(vb, vb->num_pages);
-	update_balloon_size(vb);
-
-	/* Ensure we don't get any more requests from the host */
-	vdev->config->reset(vdev);
-	vdev->config->del_vqs(vdev);
+	remove_common(vb);
 	return 0;
 }
 
-- 
1.7.7.6

On Fri, Apr 27, 2012 at 12:45:54AM +0530, Amit Shah
wrote:
> Hello,
> 
> The main fix is to update the host with the current balloon value on
> module removal after deflating the balloon.  Without the fix, the host
> has the wrong idea of the ballooned memory in the guest.  This is
> patch 2.
> 
> Patches 1 and 3 are cleanups with no effective code change.

better to just do fixes for 3.4. can you reorder pls?
> Please apply,
> 
> Amit Shah (3):
>   virtio: balloon: drop restore_common()
>   virtio: balloon: let host know of updated balloon size before module
>     removal
>   virtio: balloon: separate out common code between remove and freeze
>     functions
> 
>  drivers/virtio/virtio_balloon.c |   34 ++++++++++++++--------------------
>  1 files changed, 14 insertions(+), 20 deletions(-)
> 
> -- 
> 1.7.7.6