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

>>  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"?  Possibly with some limitations,
such as "can only adjust downwards"?