Re: [Qemu-devel] [PATCH 2/5] exec: qemu_ram_alloc_device, qemu_ram_resize

[Michael S. Tsirkin]

On Wed, Nov 19, 2014 at 11:16:57AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <address@hidden> writes:
> 
> > On Wed, Nov 19, 2014 at 10:19:22AM +0100, Juan Quintela wrote:
> >> "Michael S. Tsirkin" <address@hidden> wrote:
> >> > On Tue, Nov 18, 2014 at 07:03:58AM +0100, Paolo Bonzini wrote:
> >> >> 
> >> >> 
> >> >> On 17/11/2014 21:08, Michael S. Tsirkin wrote:
> >> >> > Add API to manage on-device RAM.
> >> >> > This looks just like regular RAM from migration POV,
> >> >> > but has two special properties internally:
> >> >> > 
> >> >> >     - block is sized on migration, making it easier to extend
> >> >> >       without breaking migration compatibility or wasting
> >> >> >       virtual memory
> >> >> >     - callers must specify an upper bound on size
> >> >> 
> >> 
> >> 
> >> >> Also, I am afraid that this design could make it easier to introduce
> >> >> backwards-incompatible changes.
> >> >
> >> >
> >> > Well the point is exactly to make it easy to make *compatible*
> >> > changes.
> >> >
> >> > As I mentioned in the cover letter, it's not just ACPI.
> >> > For example, we now change boot index dynamically.
> >> > People using large fw cfg blobs, e.g. -initrd, would benefit from
> >> > ability to change the blob dynamically.
> >> > There could be other examples.
> >> 
> >> changing the size of the initrd, on the fly and wanting to migrate?  Is
> >> that a real use case?  One that we should really care?
> >
> > I'm not sure.
> >
> > At the moment one can swap kernels by doing halt in guest and
> > restarting with the new one.
> >
> > If we wanted to allow reboot in guest to bring a new kernel instead,
> > that would be one way to implement it.
> >
> > I was merely pointing out that the capability might find other uses.
> >
> >
> >> >>  I very much prefer to have
> >> >> user-controlled ACPI information (coming from the command-line)
> >> >> byte-for-byte identical for a given machine type.  Patches for that have
> >> >> been on the list for almost two months, and it's not nice.
> >> >> 
> >> >> Paolo
> >> >
> >> > I guess we just disagree on whether these patches will effectively 
> >> > achieve
> >> > this goal.  For example, some people want to rewrite iasl bits,
> >> > generating everything in C. This will affect static bits too.
> >> > I don't want to make every single change in code conditional
> >> > on a machine type.
> >> 
> >> You can't migrate with a different BIOS on destination, period.
> >
> > This claim is very wrong.
> > This would make is impossible to change BIOS bus without breaking
> > migration.  Look at history of qemu, we change BIOS every release.
> 
> Since migration doesn't transport configuration, we require a compatibly
> configured target, and that includes identical memory sizes.  RAM size
> is explicit and the user's problem.  ROM size is generally implicit, and
> we use machine type compatibility machinery to keep it fixed.  BIOS
> changes can break migration only when we screw up or forget the
> compatibility machinery.  Same as for lots of other stuff.  No big deal,
> really, just a consequence of not migrating configuration.

You don't get to maintain it, so it's no big deal for you.

I see pain every single release and code is becoming spaghetty-like very
quickly.  We thought it would work. It does not.  We do need a solution.

And the pain is completely self-inflicted: we already migrate
all necessary information!
It's just a question of adjusting our datastructures to it.



> >>  That is
> >> what is making this whole issue complicated.  We have two clear options:
> >> 
> >> a- require BIOS & memory regions to be exactly the same in both sides.
> >>    No need to add compat machinery.
> >> b- trying to accomodate any potential change that could appear and use
> >>    the same BIOS.
> >> 
> >> IMHO, b) is just asking for trouble.  Being able to go from random
> >> changes to random changes look strange.
> >
> > Yes, it is hard to support.
> > But it's a required feature, and in fact, it's an existing one.
> >
> >> Just think about it for a second.  We are sending more data for some
> >> regions that it was allocated.  And we just grow the regions and expect
> >> that everything is going to be ok.  It is me, or this goes against every
> >> security discipline that I can think of?
> >> 
> >> Later, Juan.
> >
> > We have many devices that just get N from stream, do malloc(N), then read
> > data from stream into it.
> > You think it's unsafe? Go ahead and fix them all.
> >
> > However, my patch does address your concern: callers specify the upper
> > limit on the region size.
> > Trying to migrate in a 1Gbyte region
> 
> Are you proposing to make incoming migration adjust some or all memory
> sizes on the target from "whatever was configured during startup" to
> "whatever is configured on the source"?

Yes.

At the moment, I only propose this for internal on-device RAM,
for the simple reason that users don't know or care about it.
So migrating it just removes maintainance pain.

It wouldn't be hard to extend it for user-specified RAM,
but I don't know whether that is useful.

> Possibly with some limitations,
> such as "can only adjust downwards"?

Yes.

"Can adjust downwards" is too limiting, since especially downstreams
want two-way migration to work.
So I just have devices specify an upper limit.


-- 
MST