Re: [PATCH for 8.0] igb: Save the entire Tx context descriptor

[Juan Quintela]

Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> On 16/3/23 13:40, Akihiko Odaki wrote:
>> On 2023/03/16 21:36, Philippe Mathieu-Daudé wrote:
>>> On 16/3/23 13:28, Akihiko Odaki wrote:
>>>> The current implementation of igb uses only part of a advanced Tx
>>>> context descriptor because it misses some features and sniffs the trait
>>>> of the packet instead of respecting the packet type specified in the
>>>> descriptor. However, we will certainly need the entire Tx context
>>>> descriptor when we update igb to respect these ignored fields. Save the
>>>> entire Tx context descriptor to prepare for such a change.
>>>>
>>>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> ---
>>>>   hw/net/igb.c      |  6 ++++--
>>>>   hw/net/igb_core.c | 17 ++++++++++-------
>>>>   hw/net/igb_core.h |  3 +--
>>>>   3 files changed, 15 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/hw/net/igb.c b/hw/net/igb.c
>>>> index 0792626322..50239a7cb1 100644
>>>> --- a/hw/net/igb.c
>>>> +++ b/hw/net/igb.c
>>>> @@ -499,8 +499,10 @@ static const VMStateDescription igb_vmstate_tx = {
>>>>       .version_id = 1,
>>>
>>> Don't we need to increment the vmstate version? See
>>> https://qemu-project.gitlab.io/qemu/devel/migration.html#versions
>> This device is added only a week ago so it shouldn't need version
>> bump. That is also why I tagged this change "for 8.0".
>
> Well it is cheaper than dealing with partially backported commits...
> Also could be a better example for future developers IMHO. My 2 cents.

You can't have everything O:-)

I would just bump the version and not do the "dance" where you can
migrate from v1 and v2.  I.e. don't add tests at all.

This way bisect will fail with the correct message.

Later, Juan.