[Qemu-devel] [PATCH v2 0/2] fdc: fix media handling

[Qemu-devel] [PATCH v2 0/2] fdc: fix media handling

[Pavel Hrdina]

This patch series fixes handling of FDC when you don't have media inserted.
Guest should see floppy drive if you start guest without media and should
detect that there is no media in drive.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>

Pavel Hrdina (2):
  fdc: floppy drive should be visible after start without media
  fdc: fix media detection

 hw/fdc.c |   14 ++++++++------
 hw/pc.c  |    2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

-- 
1.7.7.6

[Qemu-devel] [PATCH v2 1/2] fdc: floppy drive should be visible after start without media

[Pavel Hrdina]

If you start guest with floppy drive but without media inserted, guest
still should see floppy drive pressent.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 hw/pc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 4d34a33..967c17a 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
     if (floppy) {
         fdc_get_bs(fd, floppy);
         for (i = 0; i < 2; i++) {
-            if (fd[i] && bdrv_is_inserted(fd[i])) {
+            if (fd[i]) {
                 bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
                                               &last_sect, FDRIVE_DRV_NONE,
                                               &fd_type[i], &rate);
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v2 1/2] fdc: floppy drive should be visible after start without media

[Kevin Wolf]

Am 22.05.2012 17:00, schrieb Pavel Hrdina:
> If you start guest with floppy drive but without media inserted, guest
> still should see floppy drive pressent.
> 
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hw/pc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 4d34a33..967c17a 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>      if (floppy) {
>          fdc_get_bs(fd, floppy);
>          for (i = 0; i < 2; i++) {
> -            if (fd[i] && bdrv_is_inserted(fd[i])) {
> +            if (fd[i]) {
>                  bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>                                                &last_sect, FDRIVE_DRV_NONE,
>                                                &fd_type[i], &rate);

I'm not completely sure, but we might have to make this change dependent
on the machine type to avoid surprises with live migration.

Kevin

Re: [Qemu-devel] [PATCH v2 1/2] fdc: floppy drive should be visible after start without media

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 22.05.2012 17:00, schrieb Pavel Hrdina:
>> If you start guest with floppy drive but without media inserted, guest
>> still should see floppy drive pressent.
>> 
>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>> ---
>>  hw/pc.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>> 
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 4d34a33..967c17a 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>>      if (floppy) {
>>          fdc_get_bs(fd, floppy);
>>          for (i = 0; i < 2; i++) {
>> -            if (fd[i] && bdrv_is_inserted(fd[i])) {
>> +            if (fd[i]) {
>>                  bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>>                                                &last_sect, FDRIVE_DRV_NONE,
>>                                                &fd_type[i], &rate);
>
> I'm not completely sure, but we might have to make this change dependent
> on the machine type to avoid surprises with live migration.

What kind of surprises do you have in mind?

Re: [Qemu-devel] [PATCH v2 1/2] fdc: floppy drive should be visible after start without media

[Kevin Wolf]

Am 23.05.2012 11:14, schrieb Markus Armbruster:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
>> Am 22.05.2012 17:00, schrieb Pavel Hrdina:
>>> If you start guest with floppy drive but without media inserted, guest
>>> still should see floppy drive pressent.
>>>
>>> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>>> ---
>>>  hw/pc.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/hw/pc.c b/hw/pc.c
>>> index 4d34a33..967c17a 100644
>>> --- a/hw/pc.c
>>> +++ b/hw/pc.c
>>> @@ -382,7 +382,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,
>>>      if (floppy) {
>>>          fdc_get_bs(fd, floppy);
>>>          for (i = 0; i < 2; i++) {
>>> -            if (fd[i] && bdrv_is_inserted(fd[i])) {
>>> +            if (fd[i]) {
>>>                  bdrv_get_floppy_geometry_hint(fd[i], &nb_heads, &max_track,
>>>                                                &last_sect, FDRIVE_DRV_NONE,
>>>                                                &fd_type[i], &rate);
>>
>> I'm not completely sure, but we might have to make this change dependent
>> on the machine type to avoid surprises with live migration.
> 
> What kind of surprises do you have in mind?

A second drive appearing (or appearing in part) after a migration. But
looking closer at the code, this really only changes the CMOS (which is
migrated) and doesn't seem to have any side effects.

Kevin

[Qemu-devel] [PATCH v2 2/2] fdc: fix media detection

[Pavel Hrdina]

We have to set up 'media_changed' after guest start so floppy driver
could detect that there is no media in drive. For this purpose we call
'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'.
'fd_revalidate' is called inside 'fdctrl_change_cb'.

We still have to set default drive geometry in 'fd_revalidate' even
if there is no media in drive. When you try to open (windows) or mount (linux)
floppy the driver tries to seek on track 1. Linux guest hang in loop than crash
and windows guest print error message.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 hw/fdc.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index cb4cd25..b1bc6ed 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
     FDriveRate rate;
 
     FLOPPY_DPRINTF("revalidate\n");
-    if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
+    if (drv->bs != NULL) {
         ro = bdrv_is_read_only(drv->bs);
         bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
                                       &last_sect, drv->drive, &drive, &rate);
-        if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
-            FLOPPY_DPRINTF("User defined disk (%d %d %d)",
+        if (!bdrv_is_inserted(drv->bs)) {
+            FLOPPY_DPRINTF("No disk in drive\n");
+        } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
+            FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
                            nb_heads - 1, max_track, last_sect);
         } else {
             FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
@@ -201,7 +203,7 @@ static void fd_revalidate(FDrive *drv)
         drv->drive = drive;
         drv->media_rate = rate;
     } else {
-        FLOPPY_DPRINTF("No disk in drive\n");
+        FLOPPY_DPRINTF("Invalid disk in drive\n");
         drv->last_sect = 0;
         drv->max_track = 0;
         drv->flags &= ~FDISK_DBL_SIDES;
@@ -709,7 +711,7 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)
         FDrive *cur_drv;
         /* A seek clears the disk change line (if a disk is inserted) */
         cur_drv = get_cur_drv(fdctrl);
-        if (cur_drv->max_track) {
+        if (cur_drv->bs != NULL && bdrv_is_inserted(cur_drv->bs)) {
             cur_drv->media_changed = 0;
         }
     }
@@ -1878,7 +1880,7 @@ static int fdctrl_connect_drives(FDCtrl *fdctrl)
         }
 
         fd_init(drive);
-        fd_revalidate(drive);
+        fdctrl_change_cb(drive, 0);
         if (drive->bs) {
             bdrv_set_dev_ops(drive->bs, &fdctrl_block_ops, drive);
         }
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH v2 2/2] fdc: fix media detection

[Markus Armbruster]

Pavel Hrdina <phrdina@redhat.com> writes:

> We have to set up 'media_changed' after guest start so floppy driver
> could detect that there is no media in drive. For this purpose we call
> 'fdctrl_change_cb' instead of 'fd_revalidate' in 'fdctrl_connect_drives'.
> 'fd_revalidate' is called inside 'fdctrl_change_cb'.
>
> We still have to set default drive geometry in 'fd_revalidate' even
> if there is no media in drive. When you try to open (windows) or mount (linux)
> floppy the driver tries to seek on track 1. Linux guest hang in loop than crash
> and windows guest print error message.

Got one nitpick inline.

> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  hw/fdc.c |   14 ++++++++------
>  1 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index cb4cd25..b1bc6ed 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -179,12 +179,14 @@ static void fd_revalidate(FDrive *drv)
>      FDriveRate rate;
>  
>      FLOPPY_DPRINTF("revalidate\n");
> -    if (drv->bs != NULL && bdrv_is_inserted(drv->bs)) {
> +    if (drv->bs != NULL) {
>          ro = bdrv_is_read_only(drv->bs);
>          bdrv_get_floppy_geometry_hint(drv->bs, &nb_heads, &max_track,
>                                        &last_sect, drv->drive, &drive, &rate);
> -        if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> -            FLOPPY_DPRINTF("User defined disk (%d %d %d)",
> +        if (!bdrv_is_inserted(drv->bs)) {
> +            FLOPPY_DPRINTF("No disk in drive\n");
> +        } else if (nb_heads != 0 && max_track != 0 && last_sect != 0) {
> +            FLOPPY_DPRINTF("User defined disk (%d %d %d)\n",
>                             nb_heads - 1, max_track, last_sect);
>          } else {
>              FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
> @@ -201,7 +203,7 @@ static void fd_revalidate(FDrive *drv)
>          drv->drive = drive;
>          drv->media_rate = rate;
>      } else {
> -        FLOPPY_DPRINTF("No disk in drive\n");
> +        FLOPPY_DPRINTF("Invalid disk in drive\n");

Misleading.  What !drv->bs actually means is "no drive connected".

>          drv->last_sect = 0;
>          drv->max_track = 0;
>          drv->flags &= ~FDISK_DBL_SIDES;
[...]