Re: [Qemu-devel] [QEMU PATCH v10 3/3] tests/migration: Add test for QTAILQ migration

[Halil Pasic <pasic@linux.vnet.ibm.com>]

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 <duanj@linux.vnet.ibm.com> wrote:
>>>> >>> Add a test for QTAILQ migration to tests/test-vmstate.c.
>>>> >>>
>>>> >>> Signed-off-by: Jianjun Duan <duanj@linux.vnet.ibm.com>
>>> >>
>>> >> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>> >>
>> > 
>> > 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
>