Re: [Qemu-devel] [PATCH v2 5/6] e1000: Choose which set of props to migrate

[Jason Wang <jasowang@redhat.com>]

On 2018年03月29日 16:44, Dr. David Alan Gilbert wrote:
> * Jason Wang (jasowang@redhat.com) wrote:
>>
>> On 2018年03月29日 16:08, Dr. David Alan Gilbert wrote:
>>> * Jason Wang (jasowang@redhat.com) wrote:
>>>> On 2018年03月29日 00:36, Dr. David Alan Gilbert (git) wrote:
>>>>> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>>>>>
>>>>> When we're using the subsection we migrate both
>>>>> the 'props' and 'tso_props' data; when we're not using
>>>>> the subsection (to migrate to 2.11 or old machine types) we've
>>>>> got to choose what to migrate in the main structure.
>>>>>
>>>>> If we're using the subsection migrate 'props' in the main structure.
>>>>> If we're not using the subsection then migrate the last one
>>>>> that changed, which gives behaviour similar to the old behaviour.
>>>>>
>>>>>
>>>> But only after migration. Why not simply switch back to the old behavior if
>>>> migrate_tso_props if false?
>>> Because:
>>>     1) We know it's a broken behaviour so it's better not to unfix it
>>>     2) The fix doesn't change guest visible behaviour other than actually
>>>        sending the right packets; so there's no reason to make the fix
>>>        itself dependent on the machine type.
>>>     3) Gating the fix itself on the flag is actually more complex and
>>>        would need checking the flag in lots of places that are already
>>>        pretty complex, rather than what this does which is just check it
>>>        in one place at migration.
>> It looks to me it was just something like:
>>
>>      struct e1000x_txd_props *props = tp->cptse && chkflag(TSO) ?
>> &tp->tso_props : &tp->props;
> is that the only thing that would change?

Looks like and we can avoid using tricks like patch 4.

>
>> Btw, did this patch work when:
>>
>> migrate A to B
>> migrate B to C
>>
>> But there's no tx during B?
> Hmm, good question; I only tried it keeping the stream alive during
> migration.
> Lets see what happens.
>
> For this code to be used we have to be running with an old machine
> type/property.
> That means A->B will have either come from 2.11 or a 2.12 with this same
> patch.
> But given patch 2 that duplicates on loading; that means A->B should
> end up with B having the same data in both sets of props, and
> thus it doesn't matter which set this patch picks.

Yes it is. I think I would agree with your idea now consider it was 
slightly better than using the old behavior.

Thanks

>
> Dave
>
>> Thanks
>>
>>> Dave
>>>> Thanks
>>>>
>>> --
>>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>