Re: [PATCH for-5.0 v11 01/20] migration: Support QLIST migration

[Auger Eric]

Hi Juan,

On 1/8/20 2:19 PM, Juan Quintela wrote:
> "Dr. David Alan Gilbert" <address@hidden> wrote:
>> * Eric Auger (address@hidden) wrote:
>>> Support QLIST migration using the same principle as QTAILQ:
>>> 94869d5c52 ("migration: migrate QTAILQ").
>>>
>>> The VMSTATE_QLIST_V macro has the same proto as VMSTATE_QTAILQ_V.
>>> The change mainly resides in QLIST RAW macros: QLIST_RAW_INSERT_HEAD
>>> and QLIST_RAW_REVERSE.
>>>
>>> Tests also are provided.
>>>
>>> Signed-off-by: Eric Auger <address@hidden>
>>>
>>> +    while (qemu_get_byte(f)) {
>>> +        elm = g_malloc(size);
>>> +        ret = vmstate_load_state(f, vmsd, elm, version_id);
>>> +        if (ret) {
>>> +            error_report("%s: failed to load %s (%d)", field->name,
>>> +                         vmsd->name, ret);
>>> +            g_free(elm);
>>> +            return ret;
>>> +        }
>>> +        QLIST_RAW_INSERT_HEAD(pv, elm, entry_offset);
>>> +    }
>>> +    QLIST_RAW_REVERSE(pv, elm, entry_offset);
>>
>> Can you explain why you need to do a REVERSE on the loaded list,
>> rather than using doing a QLIST_INSERT_AFTER to always insert at
>> the end?
>>
>> Other than that it looks good.
> 
> This was my fault (integrated as this is).
> 
> Old code had a "walk to the end of the list" and then insert.
> I told it was way faster just to insert and the beggining and then
> reverse.  I didn't noticed that we had the previous element to know
> where to insert.

Not sure I get your comment. To insert at the end one needs to walk
though the list. The head has no prev pointer pointing to the tail as
opposed to the queue. So I understood Dave's comment as "just explain
why you prefered this solution against the QLIST_INSERT_AFTER alternative.

Thanks

Eric
> 
> Eric, feel free to send a patch on top of this, or I will do it.

> 
> Later, Juan.
> 
>