[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAIL
From: |
Halil Pasic |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ |
Date: |
Thu, 27 Oct 2016 13:16:04 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 |
On 10/26/2016 07:33 PM, Jianjun Duan wrote:
>>>>>>> +#define QTAILQ_FIRST_OFFSET 0
>>>>>>> >>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *))
>>>>>> >>>>>
>>>>>> >>>>> OK, you might want to add some asserts somewhere in a .c just to
>>>>>> >>>>> catch if
>>>>>> >>>>> any of these offsets change.
>>>>>> >>>>>
>>>>> >>>> if the layout of QTAILQ changes, the version id for the vmsd should
>>>>> >>>> also
>>>>> >>>> be changed. It will break migration between different versions.
>>>>> >>>> However
>>>>> >>>> the operation doesn't depend on these values.
>>>> >>>
>>>> >>> No, changing layout of QTAILQ doesn't need to change the version id of
>>>> >>> vmsd;
>>>> >>> it's an internal change. But if someone does make the change and
>>>> >>> forgets
>>>> >>> to update your OFFSET macros it'll cause memory corruption.
>>>> >>> You could catch that with an assert (possibly build time).
>>>> >>>
>>> >> If we use these constant I would agree an assertion is necessary. By
>>> >> using a macro we leave the door open for change. and if someone changes
>>> >> the layout, he or she better change the macros and the version id. If an
>>> >> assertion is added, then that assertion needs to be changed to reflect
>>> >> the change, then in the unlikely situations we could have several
>>> >> version of layout/macro/assertions floating around. That is too much. SO
>>> >> version id is the best guard here.
>> >
>> > Version id's are irrelevant here.
>> > The macros are irrelevant here.
>> > I'm talking about a potential mismatch between the definition of
>> > QTAILQ_LAST_OFFSET
>> > and the definition of Q_TAILQ_HEAD.
>> >
>> > Dave
>> >
> I suppose anyone who changes the layout should also change the macro and
> version ID of the relevant vmsd. Similar issue was discussed before as
> the early version tried to generate all these offsets based on the
> element and head type. You can see in version 2 discussion.
>
>
> Thanks,
> Jianjun
IMHO Dave is right, although a change in head/entry does not seem too
likely to me. Just want to point out that Dave's proposal (discussed here
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03770.html)
does not have these issues and it appears more elegant to me. Jianjun
why do you prefer doing the offsets manually?
Halil
- [Qemu-ppc] [QEMU PATCH v8 0/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/25
- [Qemu-ppc] [QEMU PATCH v8 1/3] migration: extend VMStateInfo, Jianjun Duan, 2016/10/25
- [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/25
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Dr. David Alan Gilbert, 2016/10/26
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/26
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Dr. David Alan Gilbert, 2016/10/26
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/26
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Dr. David Alan Gilbert, 2016/10/26
- Re: [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/26
- Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ,
Halil Pasic <=
- Re: [Qemu-ppc] [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ, Jianjun Duan, 2016/10/27
[Qemu-ppc] [QEMU PATCH v8 3/3] tests/migration: Add test for QTAILQ migration, Jianjun Duan, 2016/10/25