Re: [Qemu-devel] [PATCH] scsi: esp: fix migration

[Juan Quintela <quintela@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> 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