Re: [Qemu-devel] [PATCH v2] fdc: fix media change detection for windows

[Pavel Hrdina <phrdina@redhat.com>]

On 06/05/2012 05:56 PM, Kevin Wolf wrote:
> Am 05.06.2012 17:50, schrieb Pavel Hrdina:
>> Sorry, I forget the qtest. I'll create it and send it again.
>>
>> Pavel
>>
>> On 06/05/2012 05:46 PM, Pavel Hrdina wrote:
>>> The Windows uses 'READ' command at the start of an instalation
>>> without checking the 'dir' register. We have to abort the transfer
>>> with an abnormal termination if there is no media in the drive.
>>>
>>> We have to also check the 'media_change' bit in the 'fd_seek'. This
>>> internal seek clears the 'media_change' bit, too, if there is
>>> a media inserted.
>>>
>>> Signed-off-by: Pavel Hrdina<phrdina@redhat.com>
>>> ---
>>>    hw/fdc.c |    7 ++++++-
>>>    1 files changed, 6 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/fdc.c b/hw/fdc.c
>>> index 30d34e3..2d6dd30 100644
>>> --- a/hw/fdc.c
>>> +++ b/hw/fdc.c
>>> @@ -127,8 +127,13 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,
>>>        uint32_t sector;
>>>        int ret;
>>>
>>> +    if (drv->bs != NULL&&   bdrv_is_inserted(drv->bs)) {
>>> +        drv->media_changed = 0;
>>> +    }
>>> +
>>>        if (track>   drv->max_track ||
>>> -        (head != 0&&   (drv->flags&   FDISK_DBL_SIDES) == 0)) {
>>> +        (head != 0&&   (drv->flags&   FDISK_DBL_SIDES) == 0) ||
>>> +        drv->media_changed) {
> Why not directly use bdrv_is_inserted() here?
>
> Fiddling around with media_changed feels rather hacky and is strictly
> speaking incorrect because a step pulse is not guaranteed to happen in
> the following code. The floppy code doesn't get it quite right today
> anyway, but a hack like this would contribute to the problem.
>
>
>>>            FLOPPY_DPRINTF("try to read %d %02x %02x (max=%d %d %02x %02x)\n",
>>>                           head, track, sect, 1,
>>>                           (drv->flags&   FDISK_DBL_SIDES) == 0 ? 0 : 1,
> Kevin
>
I'll move the code into the end of 'fd_seek' function, where a step is 
guaranteed. Internal seek should also reset the 'media_changed' bit if 
there is a media in the drive. I'll create the qtest tomorrow then I'll 
send another version of this patch.

Pavel