[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
From: |
David Hildenbrand |
Subject: |
Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback |
Date: |
Fri, 28 Feb 2020 18:59:57 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 28.02.20 17:49, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: David Hildenbrand [mailto:address@hidden]
>> Sent: 13 February 2020 17:09
>> 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;
>> address@hidden; Juan Jose Quintela Carreira <address@hidden>
>> Subject: Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback
>
> [...]
>
>>>> Thanks for that. I had a go with the below patch and it indeed fixes the
>>>> issue
>>>> of callback not being called on resize. But the migration fails with the
>>>> below
>>>> error,
>>>>
>>>> For x86
>>>> ---------
>>>> qemu-system-x86_64: Unknown combination of migration flags: 0x14
>>>> qemu-system-x86_64: error while loading state for instance 0x0 of device
>> 'ram'
>>>> qemu-system-x86_64: load of migration failed: Invalid argument
>>>>
>>>> For arm64
>>>> --------------
>>>> qemu-system-aarch64: Received an unexpected compressed page
>>>> qemu-system-aarch64: error while loading state for instance 0x0 of device
>> 'ram'
>>>> qemu-system-aarch64: load of migration failed: Invalid argument
>>>>
>>>> I haven’t debugged this further but looks like there is a corruption
>> happening.
>>>> Please let me know if you have any clue.
>>>
>>> The issue is
>>>
>>> qemu_put_be64(f, ram_bytes_total_common(true) |
>> RAM_SAVE_FLAG_MEM_SIZE)
>>>
>>> The total ram size we store must be page aligned, otherwise it will be
>>> detected as flags. Hm ... maybe we can round it up ...
>>>
>>
>> I'm afraid we can't otherwise we will run into issues in
>> ram_load_precopy(). Hm ...
>
> Sorry, took a while to get back on this. Yes, round up indeed breaks in
> ram_load_precopy() . I had the below on top of your patch and that
> seems to do the job (sanity tested on arm/virt).
>
> Please take a look and let me know if you see any issues with this approach.
>
> Thanks,
> Shameer
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 2acc4b85ca..7447f0cefa 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1782,7 +1782,7 @@ static uint64_t ram_bytes_total_migration(void)
> RCU_READ_LOCK_GUARD();
>
> RAMBLOCK_FOREACH_MIGRATABLE(block) {
> - total += ramblock_ram_bytes_migration(block);
> + total += block->used_length;
> }
> return total;
> }
> @@ -3479,7 +3479,7 @@ static int ram_load_precopy(QEMUFile *f)
> ret = -EINVAL;
> }
>
> - total_ram_bytes -= length;
> + total_ram_bytes -= block->used_length;
> }
> break;
>
>
>
What you mean is the following:
commit 702f4325086c3a8d6083787f8bc8503f7523bac8 (HEAD -> master)
Author: David Hildenbrand <address@hidden>
Date: Wed Feb 12 19:16:34 2020 +0100
tmp
Signed-off-by: David Hildenbrand <address@hidden>
diff --git a/exec.c b/exec.c
index 67e520d18e..cec643b914 100644
--- a/exec.c
+++ b/exec.c
@@ -2125,11 +2125,21 @@ static int memory_try_enable_merging(void *addr, size_t
len)
*/
int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize, Error **errp)
{
+ const ram_addr_t unaligned_size = newsize;
+
assert(block);
newsize = HOST_PAGE_ALIGN(newsize);
if (block->used_length == newsize) {
+ /*
+ * We don't have to resize the ram block (which only knows aligned
+ * sizes), however, we have to notify if the unaligned size changed.
+ */
+ if (block->resized && unaligned_size != memory_region_size(block->mr))
{
+ block->resized(block->idstr, unaligned_size, block->host);
+ memory_region_set_size(block->mr, unaligned_size);
+ }
return 0;
}
@@ -2153,9 +2163,9 @@ int qemu_ram_resize(RAMBlock *block, ram_addr_t newsize,
Error **errp)
block->used_length = newsize;
cpu_physical_memory_set_dirty_range(block->offset, block->used_length,
DIRTY_CLIENTS_ALL);
- memory_region_set_size(block->mr, newsize);
+ memory_region_set_size(block->mr, unaligned_size);
if (block->resized) {
- block->resized(block->idstr, newsize, block->host);
+ block->resized(block->idstr, unaligned_size, block->host);
}
return 0;
}
diff --git a/migration/ram.c b/migration/ram.c
index d2208b5534..249d3edede 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3412,7 +3412,15 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
RAMBLOCK_FOREACH_MIGRATABLE(block) {
qemu_put_byte(f, strlen(block->idstr));
qemu_put_buffer(f, (uint8_t *)block->idstr, strlen(block->idstr));
- qemu_put_be64(f, block->used_length);
+ /*
+ * When resizing on the target, we need the unaligned size,
+ * otherwise we lose the unaligned sise during migration.
+ *
+ * Note: The sum of all ram blocks will differ to
+ * ram_bytes_total_common(true) stored above.
+ */
+ qemu_put_be64(f, ramblock_ram_bytes_migration(block));
+
if (migrate_postcopy_ram() && block->page_size !=
qemu_host_page_size) {
qemu_put_be64(f, block->page_size);
@@ -4429,7 +4437,7 @@ static int ram_load_precopy(QEMUFile *f)
ret = -EINVAL;
}
- total_ram_bytes -= length;
+ total_ram_bytes -= block->used_length;
}
break;
Please note that this will *for sure* break migration between QEMU versions.
So I don't think this will work.
We should instead think about
1. Migrating the actual size of the 3 memory regions separately and setting
them via
memory_region_ram_resize() when loading the vmstate. This will trigger another
FW cfg
fixup and should be fine (with the same qemu_ram_resize() above).
2. Introduce a new RAM_SAVE_FLAG_MEM_SIZE_2, that e.g., stores the number of
ramblocks,
not the total amount of memory of the ram blocks. But it's hacky, because we
migrate
something for RAM blocks, that is not a RAM block concept (sub-block sizes).
I think you should look into 1. Shouldn't be too hard I think.
--
Thanks,
David / dhildenb
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, (continued)
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/07
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/10
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/10
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/10
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/12
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/12
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/13
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/13
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, David Hildenbrand, 2020/02/13
- RE: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback, Shameerali Kolothum Thodi, 2020/02/28
- Re: [PATCH v2 1/7] exec: Fix for qemu_ram_resize() callback,
David Hildenbrand <=