[Qemu-devel] RAMBlock's named ""

[Qemu-devel] RAMBlock's named ""

[Dr. David Alan Gilbert]

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.

The real fun is that there doesn't seem to be anything that stops
two blocks having the same name!

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Peter Maydell]

On 7 March 2017 at 20:46, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> The real fun is that there doesn't seem to be anything that stops
> two blocks having the same name!

The memory region API says that the name is for debugging only,
so the problem is in code which relies on them being unique :-)

thanks
-- PMM

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

[Dr. David Alan Gilbert]

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 7 March 2017 at 20:46, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote:
> > The real fun is that there doesn't seem to be anything that stops
> > two blocks having the same name!
> 
> The memory region API says that the name is for debugging only,
> so the problem is in code which relies on them being unique :-)

I'll fix that comment!
However, the problem here is slightly different; I think the Memory regions
do have names - the problem in this case is that the RAMBlock's aren't
attached to any memory regions.

Dave

> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Igor Mammedov]

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.


> 
> Dave
> 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Dr. David Alan Gilbert]

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

Dave

> 
> > 
> > Dave
> > 
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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

[Juan Quintela]

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

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

[Paolo Bonzini]

On 07/03/2017 20:46, Dr. David Alan Gilbert wrote:
>   (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 is a leak of the e1000e object.  The cause is a simple typo.

Paolo