Re: [Qemu-devel] RAMBlock's named ""

[Juan Quintela <quintela@redhat.com>]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Igor Mammedov (imammedo@redhat.com) wrote:
>> On Tue, 7 Mar 2017 19:46:56 +0000
>> "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
>> 
>> > We seem to have a couple of weird cases where we end up with
>> > RAMBlocks with no name;  I think they'll badly confuse
>> > the migration code, but I don't quite understand how they're
>> > happening.
>> > 
>> > 1) device_del e1000e
>> > 2) -object memory-backend-file  without wiring it up
>> > 
>> > I added some debug into migration/ram.c ram_save_setup to
>> > dump the names it was seeing in it's FOREACH.
>> > 
>> > 1)
>> >   (from https://bugzilla.redhat.com/show_bug.cgi?id=1425273)
>> >   The simplest reproducer of this is:
>> > 
>> > ./qemu-system-x86_64 -nographic -device e1000e,id=foo -m 1G -M
>> > pc,accel=kvm my.img
>> > 
>> >   with a Linux image and after it's booted do a device_del foo
>> > 
>> >   migration at that point sees an empty RAMBlock that was the ROM
>> >   associated with the device.  This doesn't happen on an e1000 device,
>> >   so I've not figured out what the difference is.
>> > 
>> >   This gives a : Unknown ramblock "", cannot accept migration
>> >   on the destination (correctly).
>> > 
>> >   (This happens on 2.7.0 as well, so it's nothing new)
>> > 
>> > 2)
>> >   ./qemu-system-x86_64 -nographic -object
>> > memory-backend-file,id=mem,size=512M,mem-path=/tmp
>> > 
>> >   Note I've not wired that memory to a NUMA node or a DIMM or anything, so
>> >   it's just hanging around.
>> >   Again that RAMBlock does exist and shows up in the migration code,
>> >   I think it'll try and migrate it.
>> it has to be registered with vmstate_register_ram() which
>> doesn't happen for non used hostmem object.
>> See:
>>   pc_dimm_memory_plug()
>> and
>>   memory_region_allocate_system_memory()
>> 
>> > The real fun is that there doesn't seem to be anything that stops
>> > two blocks having the same name!
>> code doesn't permit duplicate ids for -object created objects
>> but memory region api doesn't care about it as long as
>> memory_region name is unique child name within its parent object
>> children namespace.
>> 
>> you can do a check for empty / duplicate names at ram_block_add()
>> time and fail gracefully, but that probably will break things as
>> it hasn't been expected behavior before.
>
> There's actually code to check for setting duplicate RAMBlock names;
> what isn't caught is where two RAMBlocks have never had their names
> set or where they've been unset.
>
> I'm tempted to add a check at the start of migration; if one of these
> blocks exists during a migration it'll probably succeed; two of them
> however will cause chaos.

This is the best approach for the short term as far as I can see.

Later, Juan.