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

[Markus Armbruster]

"Michael S. Tsirkin" <address@hidden> writes:

> 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.

Okay, I now have a better understanding of what you're trying to do.

The general nice-to-have feature is "I don't want to repeat the source
configuration on the target manually, I want the target get it from the
source".

Since that's a tough nut to crack, you're proposing to do a special
case: get a few selected memory sizes "that users don't know or care
about" from the source.  We get their contents anyway, so why not the
size.  Implementation detail: involves resizing memory on the target.

The problem I have with this: in the physical world, memory sizes don't
change.  You can reflash your BIOS, but to get a bigger flash chip, you
have to replace the motherboard.  Translated to virtual, this means the
size of the flash chip is tied to the machine type.

Likewise for other devices containing memory.

Furthermore, I'm struggling to see why coping memory sizes tied to
machine type is hard.  Pick a sane flash size, leaving ample space for
bug fixes.  Next machine type can pick a bigger size if it really needs
so much more space for new features.  If we pick our sizes stingily, we
may end up with a number of sizes tied to machine types.  Not exactly
pretty, but it's what we've been doing for a while, and it works.  If
you want fewer sizes, start picking sizes more generously.

What am I missing here that can justify the complexity of partially
overriding target configuration in the migration stream plus
infrastructure for resizing memory?