[5/5] ide: fix migration in the middle of a bmdma transfer

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ide/pci.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

On 06/15/2010 04:31 PM, Juan Quintela wrote:
> +static bool ide_bmdma_current_needed(void *opaque)
> +{
> +    BMDMAState *bm = opaque;
> +
> +    return (bm->cur_prd_len != 0);
> +}
> +
> +static const VMStateDescription vmstate_bmdma_current = {
> +    .name = "ide bmdma_current",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
>    

Can we allow these to default to 0?  Most subsections (and most new 
sections) won't need version numbers.

> +    .fields      = (VMStateField []) {
> +        VMSTATE_UINT32(cur_addr, BMDMAState),
> +        VMSTATE_UINT32(cur_prd_last, BMDMAState),
> +        VMSTATE_UINT32(cur_prd_addr, BMDMAState),
> +        VMSTATE_UINT32(cur_prd_len, BMDMAState),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +
>   static const VMStateDescription vmstate_bmdma = {
>       .name = "ide bmdma",
>       .version_id = 3,
> @@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = {
>           VMSTATE_UINT32(nsector, BMDMAState),
>           VMSTATE_UINT8(unit, BMDMAState),
>           VMSTATE_END_OF_LIST()
> +    },
> +    .subsections = (VMStateSubsection []) {
> +        {
> +            .vmsd =&vmstate_bmdma_current,
> +            .needed = ide_bmdma_current_needed,
> +        }, {
> +            /* empty */
> +        }
>       }
>   };
>    

Looks concise and simple.

Avi Kivity <avi@redhat.com> wrote:
> On 06/15/2010 04:31 PM, Juan Quintela wrote:
>> +static bool ide_bmdma_current_needed(void *opaque)
>> +{
>> +    BMDMAState *bm = opaque;
>> +
>> +    return (bm->cur_prd_len != 0);
>> +}
>> +
>> +static const VMStateDescription vmstate_bmdma_current = {
>> +    .name = "ide bmdma_current",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .minimum_version_id_old = 1,
>>    
>
> Can we allow these to default to 0?  Most subsections (and most new
> sections) won't need version numbers.

It is not clear for all if default value is 0 or 1. (not consistent).
We can put it as 0.

>> +    .fields      = (VMStateField []) {
>> +        VMSTATE_UINT32(cur_addr, BMDMAState),
>> +        VMSTATE_UINT32(cur_prd_last, BMDMAState),
>> +        VMSTATE_UINT32(cur_prd_addr, BMDMAState),
>> +        VMSTATE_UINT32(cur_prd_len, BMDMAState),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>> +
>>   static const VMStateDescription vmstate_bmdma = {
>>       .name = "ide bmdma",
>>       .version_id = 3,
>> @@ -134,6 +156,14 @@ static const VMStateDescription vmstate_bmdma = {
>>           VMSTATE_UINT32(nsector, BMDMAState),
>>           VMSTATE_UINT8(unit, BMDMAState),
>>           VMSTATE_END_OF_LIST()
>> +    },
>> +    .subsections = (VMStateSubsection []) {
>> +        {
>> +            .vmsd =&vmstate_bmdma_current,
>> +            .needed = ide_bmdma_current_needed,
>> +        }, {
>> +            /* empty */
>> +        }
>>       }
>>   };
>>    
>
> Looks concise and simple.

On 06/15/2010 03:31 PM, Juan Quintela wrote:
> Signed-off-by: Juan Quintela<quintela@redhat.com>

Sorry if this has been discussed to death before (if so I must have 
missed it...).

With subsections available, what about taking advantage of the new 
protocol extension and add to the subsection info about the size of the 
subsection?

Also, with the size information, would it make sense to specify optional 
subsections that the receiver could choose to ignore?  Mandatory 
subsections are something such that round-trip A->B->A fail unless B 
understands the subsection, while optional subsections are such that A 
can provide a default.  IDE subsections would be optional, for example.

Paolo

Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 06/15/2010 03:31 PM, Juan Quintela wrote:
>> Signed-off-by: Juan Quintela<quintela@redhat.com>
>
> Sorry if this has been discussed to death before (if so I must have
> missed it...).
>
> With subsections available, what about taking advantage of the new
> protocol extension and add to the subsection info about the size of
> the subsection?

Not trivial with current infrastructure :(

> Also, with the size information, would it make sense to specify
> optional subsections that the receiver could choose to ignore?

We agreed that this was going to be forbidden.  If sender send data ->
it needs to be received.  Sender can decide to not send a subsection if
its data is not needed.

> Mandatory subsections are something such that round-trip A->B->A fail
> unless B understands the subsection, while optional subsections are
> such that A can provide a default.  IDE subsections would be optional,
> for example.

This is the whole point of the .needed() function.  if
neeeded(foo_subsection) returns false -> subsection is not needed.

If it returns true -> it is needed, destination has to understand it.

Later, Juan.