Re: [Qemu-devel] [PATCH] migratioin/ram.c: reset complete_round when we gets a queued page

[Wei Yang]

On Wed, Jun 05, 2019 at 12:33:39PM +0200, Juan Quintela wrote:
>Peter Xu <address@hidden> wrote:
>> On Wed, Jun 05, 2019 at 04:52:07PM +0800, Wei Yang wrote:
>>> On Wed, Jun 05, 2019 at 02:41:08PM +0800, Peter Xu wrote:
>>> >On Wed, Jun 05, 2019 at 09:08:28AM +0800, Wei Yang wrote:
>>> >> In case we gets a queued page, the order of block is interrupted. We may
>>> >> not rely on the complete_round flag to say we have already searched the
>>> >> whole blocks on the list.
>>> >> 
>>> >> Signed-off-by: Wei Yang <address@hidden>
>>> >> ---
>>> >>  migration/ram.c | 6 ++++++
>>> >>  1 file changed, 6 insertions(+)
>>> >> 
>>> >> diff --git a/migration/ram.c b/migration/ram.c
>>> >> index d881981876..e9b40d636d 100644
>>> >> --- a/migration/ram.c
>>> >> +++ b/migration/ram.c
>>> >> @@ -2290,6 +2290,12 @@ static bool get_queued_page(RAMState *rs, 
>>> >> PageSearchStatus *pss)
>>> >>           */
>>> >>          pss->block = block;
>>> >>          pss->page = offset >> TARGET_PAGE_BITS;
>>> >> +
>>> >> +        /*
>>> >> +         * This unqueued page would break the "one round" check, even is
>>> >> +         * really rare.
>>> >
>
>
>> Ah I see your point, but I don't think there is a problem - note that
>> complete_round will be reset for each ram_find_and_save_block(), so
>> even if we have that iteration of ram_find_and_save_block() to return
>> we'll still know we have dirty pages to migrate and in the next call
>> we'll be fine, no?
>
>Reviewed-by: Juan Quintela <address@hidden>
>
>I *think* that peter is perhaps right, but it is not clear at all, and
>it is easier to be safe.  I think that the only case that this could
>matter is if:
>- all pages are clean (so complete_round will get as true)
>- we went a queue_page request
>
>Is that possible?  I am not completely sure after looking at the code.
>It *could* be if the page that got queued is the last page remaining,
>but ......  I fully agree that the case that _almost all_ pages are
>clean and we get a request for a queued page is really rare, so it
>should not matter in real life, but ....
>

Agree

>Later, Juan.

-- 
Wei Yang
Help you, Help me