Hi Juan,
On 1/8/20 2:51 PM, Juan Quintela wrote:
> Auger Eric <eric.au...@redhat.com> wrote:
>> Hi Juan,
>>
>> On 1/8/20 2:19 PM, Juan Quintela wrote:
>>> "Dr. David Alan Gilbert" <dgilb...@redhat.com> wrote:
>>>> * Eric Auger (eric.au...@redhat.com) 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 <eric.au...@redhat.com>
>>>>>
>>>>> + 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.
>
> You have the previous inserted element, so it is kind of easy O:-)
>
> prev = NULL;
> 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;
> }
> if (!prev) {
> QLIST_RAW_INSERT_HEAD(pv, elm, entry_offset);
> } else {
> QLIST_RAW_INSERT_AFTER(prev, elm, entry_offset);
> }
> prev = elm;
> }
>
> And yes, I realize that there is no QLIST_RAW_INSTERT_AFTER() (it is
> QLIST_INSERT_AFTER). And no, I haven't took the time to understand the
> different between QLIST and QLIST_RAW. From a quick look, it seems that
> QLIST_RAW is embededed inside other structure.
Ah OK I get it now. Yes indeed that looks simpler.
>
> But as said, we can move that to another patch.
OK.
Thanks
Eric
>
> Later, Juan.
>
>
