[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v10 3/3] tests/migration: Add te
From: |
Halil Pasic |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v10 3/3] tests/migration: Add test for QTAILQ migration |
Date: |
Thu, 3 Nov 2016 18:17:42 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 11/03/2016 05:47 PM, Jianjun Duan wrote:
>
> On 11/03/2016 05:22 AM, Halil Pasic wrote:
>> >
>> >
>> > On 11/02/2016 11:47 AM, Juan Quintela wrote:
>>> >> Jianjun Duan <address@hidden> wrote:
>>>> >>> Add a test for QTAILQ migration to tests/test-vmstate.c.
>>>> >>>
>>>> >>> Signed-off-by: Jianjun Duan <address@hidden>
>>> >>
>>> >> Reviewed-by: Juan Quintela <address@hidden>
>>> >>
>> >
>> > Empty QTAILQ seems to be broken. Have written a small
>> > test to prove my point. It May even make sense to have such
>> > a test in the test-suite (some prettyfication might be
>> > necessary though).
>> >
> It is working as intended.
>
My train of thought was that the object holding the queue might
be dynamically allocated by the migration code or otherwise
uninitialized. I was unaware these scenarios are prohibited.
> The current design is to append the qtailq from source to the
> corresponding one on target.
I do not see this documented. I'm used to vmstate_load overwriting
values and following pointers, so IMHO it is not obvious that
qtailq load does append.
> It works well for the task in hard
> such as migrating ccs_list and pending_events for DRC objects.
>
Because target head is always properly initialized to empty queue?
> I suspect in most cases the qtailqs on target are empty.
If I think about migration having no queues populated with
elements on a target site sounds very reasonable since IFAIU
the target should not do any work which would populate these
data structures.
> If not,
> appending to them is a good choice. Clearing them is tricky since
> each queue probably require a specialized routine to clean. If they
> are not empty there are must be good reasons for that.
Have you some code or a scenario in mind where this is legit? I
mean creating a mix of the state(?) we found at the target and
the state captured at the source does not sound right. I would
argue that the target should not have any state which is subject
to migration.
You are right a non-empty queue is trouble, and frankly I never
considered it as a valid scenario.
Sorry if I'm bothering you with nonsense.
Greetings,
Halil
>
> Thanks,
> Jianjun
>