Re: [Qemu-devel] [PATCH 2/2] atapi migration: Throw recoverable error to avoid recovery

[John Snow <jsnow@redhat.com>]

On 12/18/2014 02:39 PM, Dr. David Alan Gilbert wrote:
> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
>> * Paolo Bonzini (pbonzini@redhat.com) wrote:
>>>
>>>
>>> On 09/12/2014 19:15, Dr. David Alan Gilbert (git) wrote:
>>>> (With the previous atapi_dma flag recovery)
>>>> If migration happens between the ATAPI command being written and the
>>>> bmdma being started, the DMA is dropped.  Eventually the guest times
>>>> out and recovers, but that can take many seconds.
>>>> (This is rare, on a pingpong reading the CD continuously I hit
>>>> this about ~1/30-1/50 migrates)
>>>>
>>>> I don't think we've got enough state to be able to recover safely
>>>> at this point, so I throw a 'medium error, no seek complete'
>>>> that I'm assuming guests will try and recover from an apparently
>>>> dirty CD.
>>>
>>
>>> Also, what is the ATAPI command that is being run?
>>
>> The command in my test case is:
>> 28 00 00 06 df b8 00 00 40 00 00 00
>> so just a standard READ (10); I guess it's possible that other OSs/operations
>> maybe doing different READ variants.
>>
>>> What is the part of the state that is not being migrated?
>>
>> I can't see lba, io_buffer_index, io_buffer_size or cd_sector_size being passed
>> to a VMSTATE_ macro; and having noticed that those are missing I didn't dig much
>> deeper.
>> (I'll admit to not really understanding the interaction between the core.c, atapi.c
>> and pci.c dma code properly to know if it can be reconstructed).
>
> Actually, another argument for this is that even if we come along and add
> the extra state as a subsection, we can still do this as a fallback when receiving
> older streams.
>

Fair enough. I vote we give this hack a shot for now, then. I can always 
put this in my basket of things to look at ($later) after I have 
finished ($other_things).