[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 07/13] migrate/ram: Get rid of "place_source" in ram_load_
From: |
David Hildenbrand |
Subject: |
Re: [PATCH v1 07/13] migrate/ram: Get rid of "place_source" in ram_load_postcopy() |
Date: |
Thu, 20 Feb 2020 14:22:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 19.02.20 21:55, Peter Xu wrote:
> On Wed, Feb 19, 2020 at 03:47:30PM -0500, Peter Xu wrote:
>> On Wed, Feb 19, 2020 at 05:17:19PM +0100, David Hildenbrand wrote:
>>> It's always the same value.
>>
>> I guess not, because...
>>
>>>
>>> Cc: "Dr. David Alan Gilbert" <address@hidden>
>>> Cc: Juan Quintela <address@hidden>
>>> Cc: Peter Xu <address@hidden>
>>> Signed-off-by: David Hildenbrand <address@hidden>
>>> ---
>>> migration/ram.c | 8 +++-----
>>> 1 file changed, 3 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/migration/ram.c b/migration/ram.c
>>> index cbd54947fb..75014717f6 100644
>>> --- a/migration/ram.c
>>> +++ b/migration/ram.c
>>> @@ -3119,7 +3119,6 @@ static int ram_load_postcopy(QEMUFile *f)
>>> ram_addr_t addr;
>>> void *host = NULL;
>>> void *page_buffer = NULL;
>>> - void *place_source = NULL;
>>> RAMBlock *block = NULL;
>>> uint8_t ch;
>>> int len;
>>> @@ -3188,7 +3187,6 @@ static int ram_load_postcopy(QEMUFile *f)
>>> place_needed = true;
>>> target_pages = 0;
>>> }
>>> - place_source = postcopy_host_page;
>>> }
>>>
>>> switch (flags & ~RAM_SAVE_FLAG_CONTINUE) {
>>> @@ -3220,7 +3218,7 @@ static int ram_load_postcopy(QEMUFile *f)
>>> * buffer to make sure the buffer is valid when
>>> * placing the page.
>>> */
>>> - qemu_get_buffer_in_place(f, (uint8_t **)&place_source,
>>
>> ... it can be modified inside the call.
>>
>> I feel like this patch could even fail the QEMU unit test. It would
>> be good to mention what tests have been carried out in the cover
>> letter or with RFC tag if no test is done yet.
>>
>> For a series like this, I'll try at least the unit tests and smoke on
>> both precopy and postcopy. The resizing test would be even better but
>> seems untrivial, so maybe optional.
>
> For resizing test, an easy way (I can think of) is to temporarily
> remove the size check below in your test branch:
Yeah, it's especially hard to have a reliable test one can materialize.
I played with manual tests like this myself.
I'm thinking about testing this with a device that can trigger resizes
on demand, e.g., virtio-mem, for now on my private branch. But I'd
really like to have some test one can automate at one point ... however,
there seems to be no easy way to achieve that right now.
Thanks!
--
Thanks,
David / dhildenb
- Re: [PATCH v1 04/13] numa: Make all callbacks of ram block notifiers optional, (continued)
- [PATCH v1 06/13] migrate/ram: Discard new RAM when growing RAM blocks and the VM is stopped, David Hildenbrand, 2020/02/19
- [PATCH v1 05/13] migrate/ram: Handle RAM block resizes during precopy, David Hildenbrand, 2020/02/19
- [PATCH v1 08/13] migrate/ram: Simplify host page handling in ram_load_postcopy(), David Hildenbrand, 2020/02/19
- [PATCH v1 09/13] migrate/ram: Consolidate variable reset after placement in ram_load_postcopy(), David Hildenbrand, 2020/02/19
- [PATCH v1 07/13] migrate/ram: Get rid of "place_source" in ram_load_postcopy(), David Hildenbrand, 2020/02/19
- Re: [PATCH v1 07/13] migrate/ram: Get rid of "place_source" in ram_load_postcopy(), David Hildenbrand, 2020/02/20
- Re: [PATCH v1 07/13] migrate/ram: Get rid of "place_source" in ram_load_postcopy(), Dr. David Alan Gilbert, 2020/02/20
[PATCH v1 10/13] migrate/ram: Handle RAM block resizes during postcopy, David Hildenbrand, 2020/02/19
[PATCH v1 11/13] migrate/multifd: Print used_length of memory block, David Hildenbrand, 2020/02/19
[PATCH v1 12/13] migrate/ram: Use offset_in_ramblock() in range checks, David Hildenbrand, 2020/02/19
[PATCH v1 13/13] migrate/ram: Tolerate partially changed mappings in postcopy code, David Hildenbrand, 2020/02/19