[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 v11 01/20] migration: Support QLIST migration
From: |
Juan Quintela |
Subject: |
Re: [PATCH for-5.0 v11 01/20] migration: Support QLIST migration |
Date: |
Wed, 08 Jan 2020 14:51:01 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Auger Eric <address@hidden> wrote:
> 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.
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.
But as said, we can move that to another patch.
Later, Juan.