Re: [PATCH] Add a page cache-backed balloon device driver.

On Tue, Jun 26, 2012 at 04:45:36PM -0700, Frank Swiderski wrote:

On Tue, Jun 26, 2012 at 2:45 PM, Rik van Riel [off-list ref] wrote:

quoted

On 06/26/2012 05:31 PM, Frank Swiderski wrote:

quoted

On Tue, Jun 26, 2012 at 1:40 PM, Rik van Riel[off-list ref]  wrote:

quoted

quoted

The code looks good to me, my only worry is the
code duplication. We now have 5 balloon drivers,
for 4 hypervisors, all implementing everything
from scratch...

Do you have any recommendations on this?  I could (I think reasonably
so) modify the existing virtio_balloon.c and have it change behavior
based on a feature bit or other configuration.  I'm not sure that
really addresses the root of what you're pointing out--it's still
adding a different implementation, but doing so as an extension of an
existing one.

Ideally, I believe we would have two balloon
top parts in a guest (one classical balloon,
one on the LRU), and four bottom parts (kvm,
xen, vmware & s390).

That way the virt specific bits of a balloon
driver would be essentially a ->balloon_page
and ->release_page callback for pages, as well
as methods to communicate with the host.

All the management of pages, including stuff
like putting them on the LRU, or isolating
them for migration, would be done with the
same common code, regardless of what virt
software we are running on.

Of course, that is a substantial amount of
work and I feel it would be unreasonable to
block anyone's code on that kind of thing
(especially considering that your code is good),
but I do believe the explosion of balloon
code is a little worrying.

Hm, that makes a lot of sense.  That would be a few patches definitely
worth doing, IMHO.  I'm not entirely sure how I feel about inflating
the balloon drivers in the meantime.  Sigh, and I didn't even mean
that as a pun.

fes

Actually I'm not 100% sure the num_pages interface
of the classical balloon is a good fit for the LRU
balloon. Let's figure that out first: if we fork the interface
there might not be all that much common code ...

-- 
MST