Re: [kvm-devel] [PATCH] KVM simplified virtio balloon driver

On Thursday 17 January 2008 13:14:58 Anthony Liguori wrote:

Rusty Russell wrote:

quoted

+static struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_BALLOON, VIRTIO_DEV_ANY_ID},

Could use a space after VIRTIO_DEV_ANY_ID

Thanks, fixed.

quoted

+		__free_page(page);
+		vb->num_pages--;
+		totalram_pages++;

Do we really want to modify totalram_pages in this driver?  The only
other place that I see that modifies it is in mm/memory_hotplug and it
also modifies other things (like num_physpages).  The cmm driver doesn't
touch totalram_pages.

I don't think there's a standard here, they're all ad-hoc (eg. no locking)  
Modifying totalram_pages has the nice effect of showing up in "free" in the 
guest.

We should probably not modify num_physpages, because some places seem to use 
it as an address space limit.  But we should probably fix all those 
networking size heuristics to use totalram_pages instead of num_physpages.

It would be very useful too to write vb->num_pages into the config space
whenever it was updated.  This way, the host can easily keep track of
where the guest is at in terms of ballooning.

OTOH it's currently pretty obvious (and usually fatal) if the guest has 
trouble meeting the balloon requirements.  A serious host needs a way of 
detecting stress in the guest anyway, which this doesn't offer until it's too 
late...

Rusty.