[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 10:24:19 +0100 |
> Am 19.02.2020 um 21:47 schrieb Peter Xu <address@hidden>:
>
> 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.
Very right, will drop this patch! Thanks!
>
> 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.
I test all code I share. This survives „make check“. I assume all tests send
small pages where „matches_target_page_size==true“, so the tests did not catch
this.
I even spent the last day getting avocado-vt to work and ran multiple
(obviously not all) migration tests, including postcopy, so your suggestions
have already been considered ...
Could have mentioned that in the cover letter, yes.
- [PATCH v1 05/13] migrate/ram: Handle RAM block resizes during precopy, (continued)
- [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 <=
- 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