RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: David Hildenbrand [mailto:address@hidden]
> Sent: 06 February 2020 14:55
> To: Shameerali Kolothum Thodi <address@hidden>;
> Igor Mammedov <address@hidden>
> Cc: address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> xuwei (O) <address@hidden>; Linuxarm <address@hidden>;
> address@hidden; address@hidden; address@hidden
> Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
> 
> On 06.02.20 12:28, Shameerali Kolothum Thodi wrote:
> >
> >
> >> -----Original Message-----
> >> From: David Hildenbrand [mailto:address@hidden]
> >> Sent: 06 February 2020 10:56
> >> To: Shameerali Kolothum Thodi <address@hidden>;
> >> Igor Mammedov <address@hidden>
> >> Cc: address@hidden; address@hidden;
> >> address@hidden; address@hidden; address@hidden;
> >> xuwei (O) <address@hidden>; Linuxarm <address@hidden>;
> >> address@hidden; address@hidden; address@hidden
> >> Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
> >
> > [...]
> >
> >>> root@ubuntu:/# cat /dev/pmem
> >>> pmem0  pmem1
> >>>
> >>> From the logs, it looks like the ram_load_precopy() --> qemu_ram_resize()
> is
> >> not
> >>> called as length == used_length and both seems to be page aligned values.
> >>> And from
> >> https://github.com/qemu/qemu/blob/master/migration/ram.c#L3421
> >>> qemu_ram_resize() is called with length if length != used_length.
> >>
> >> Assume on your source, the old size is 12345 bytes. So 16384 aligned up
> >> (4 pages).
> >>
> >> Assume on your target, the new size is 123456 bytes, so 126976 aligned
> >> up (31 pages).
> >>
> >> If you migrate from source to destination, the migration code would
> >> resize to 16384, although the "actual size" is 12345. The callback will
> >> be called with the aligned size, not the actual size. Same the other way
> >> around. That's what's inconsistent IMHO.
> >
> > Thanks. You are right. I didn’t consider the case where the target can be
> > configured with a larger number of devices than the source. I can replicate
> > the scenario now,
> >
> > Source:
> >
> > fw_cfg_add_file_callback: filename etc/boot-fail-wait size 0x4
> > fw_cfg_add_file_callback: filename etc/acpi/nvdimm-mem size 0x1000
> > fw_cfg_add_file_callback: filename etc/acpi/tables size 0x6210
> >
> > Target:
> > ram_load_precopy: Ram blk mem1 length 0x40000000 used_length
> 0x40000000
> > ram_load_precopy: Ram blk virt.flash0 length 0x4000000 used_length
> 0x4000000
> > ram_load_precopy: Ram blk virt.flash1 length 0x4000000 used_length
> 0x4000000
> > ram_load_precopy: Ram blk /rom@etc/acpi/tables length 0x7000
> used_length 0x8000
> > fw_cfg_modify_file: filename etc/acpi/tables len 0x7000
> >
> > Target updates FWCfgEntry with a page aligned size :(. I will look into 
> > this and
> see how
> > we can solve this. Any pointers welcome.
> 
> Can you look the original value up somehow and us the resize callback
> only as a notification that something changed? (that value would have to
> be stored somewhere and migrated I assume - maybe that's already being
> done)

Ok. I will take a look at that. But can we instead pass the block->used_length 
to
fw_cfg_add_file_callback(). That way we don’t have to change the 
qemu_ram_resize()
as well. I think Igor has suggested this before[1] and I had a go at it before 
coming up
with the "req_length" proposal here.

Thanks,
Shameer

[1] https://lore.kernel.org/qemu-devel/address@hidden/