[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] scsi: esp: fix migration
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH] scsi: esp: fix migration |
Date: |
Wed, 29 Jun 2016 14:00:17 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.0.94 (gnu/linux) |
Paolo Bonzini <address@hidden> wrote:
> On 27/06/2016 09:20, Amit Shah wrote:
>> On (Mon) 20 Jun 2016 [16:33:26], Paolo Bonzini wrote:
>>> Commit 926cde5 ("scsi: esp: make cmdbuf big enough for maximum CDB size",
>>> 2016-06-16) changed the size of a migrated field. Split it in two
>>> parts, and only migrate the second part in a new vmstate version.
>>
>> With this patch, the static checker fails in this way:
>>
>> Section "esp", Description "esp": expected field "cmdlen", got
>> "cmdbuf"; skipping rest
>> Section "dc390", Description "esp": expected field "cmdlen", got
>> "cmdbuf"; skipping rest
>> Section "am53c974", Description "esp": expected field "cmdlen", got
>> "cmdbuf"; skipping rest
>>
>> Note it doesn't complain about the version numbers. That's because:
>>
>>> const VMStateDescription vmstate_esp = {
>>> .name ="esp",
>>> - .version_id = 3,
>>> + .version_id = 4,
>>> .minimum_version_id = 3,
>>
>> this suggests older versions can still be accepted for incoming
>> migration, which isn't true.
>
> Sure they can:
>
> - VMSTATE_BUFFER(cmdbuf, ESPState),
> + VMSTATE_PARTIAL_BUFFER(cmdbuf, ESPState, 16),
> + VMSTATE_BUFFER_START_MIDDLE_V(cmdbuf, ESPState, 16, 4),
Amit, would it help the checker if we do something like:
- VMSTATE_BUFFER(cmdbuf, ESPState),
+ VMSTATE_PARTIAL_BUFFER_TEST(cmdbuf, ESPState, 16, v_is_3),
+ VMSTATE_BUFFER_TEST(cmdbuf, ESPState, from_4),
Yes, VMSTATE_PARTIAL_BUFFER_TEST don't exist, but it is trivial to
define.
Later, Juan.
>
> 2.6 is transmitting version 3 and a 16-byte buffer.
>
> 2.7 is transmitting version 4, a first 16-byte buffer, and a second
> 16-byte buffer that is skipped when receiving version 3.
>
> So it seems like a static checker limitation.
>
> Paolo