Patchwork fdc: fix drive property handling.

login
register
mail settings
Submitter Gerd Hoffmann
Date Dec. 15, 2009, 3:09 p.m.
Message ID <1260889756-26864-1-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/41191/
State New
Headers show

Comments

Gerd Hoffmann - Dec. 15, 2009, 3:09 p.m.
Fix the floppy controller init wrappers to set the drive properties
only in case the DriveInfo pointers passed in are non NULL.  This allows
to set the properties using -global.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/fdc.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)
Markus Armbruster - Dec. 16, 2009, 10:03 a.m.
Gerd Hoffmann <kraxel@redhat.com> writes:

> Fix the floppy controller init wrappers to set the drive properties
> only in case the DriveInfo pointers passed in are non NULL.  This allows
> to set the properties using -global.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

This fixes "-global isa-fdc,driveB=DRIVE-ID" for me, but problems
remain:

* Default:

          dev: isa-fdc, id ""
            dev-prop: driveA = floppy0
            dev-prop: driveB = <null>
            isa irq 6

* -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd

          dev: isa-fdc, id ""
            dev-prop: driveA = floppy0
            dev-prop: driveB = <null>
            isa irq 6

  No effect.  It works with -nodefaults:

          dev: isa-fdc, id ""
            dev-prop: driveA = fd
            dev-prop: driveB = <null>
            isa irq 6

* -drive if=none,id=fd,file=fd.img -global isa-fdc.driveB=fd

          dev: isa-fdc, id ""
            dev-prop: driveA = floppy0
            dev-prop: driveB = fd
            isa irq 6

  Works.  -nodefaults works as well:

          dev: isa-fdc, id ""
            dev-prop: driveA = <null>
            dev-prop: driveB = fd
            isa irq 6

* -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd,driveB=fd

  Dies, relevant part of stack backtrace:

    #2  0x006a9092 in abort () from /lib/libc.so.6
    #3  0x08128e79 in qdev_prop_set_globals (dev=0x84a6e88)
        at /home/armbru/work/qemu/hw/qdev-properties.c:622
    #4  0x08126632 in qdev_create (bus=0x849a8d0, name=0x81d4354 "isa-fdc")
        at /home/armbru/work/qemu/hw/qdev.c:106
    #5  0x081bf3d9 in isa_create (name=0x81d4354 "isa-fdc")
        at /home/armbru/work/qemu/hw/isa-bus.c:120
    #6  0x08092d83 in fdctrl_init_isa (fds=0xbfffefa0)
        at /home/armbru/work/qemu/hw/fdc.c:1887

  Same with -nodefaults.
Gerd Hoffmann - Dec. 16, 2009, 1:29 p.m.
On 12/16/09 11:03, Markus Armbruster wrote:
> This fixes "-global isa-fdc,driveB=DRIVE-ID" for me, but problems
> remain:
>
> * -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd
>
>            dev: isa-fdc, id ""
>              dev-prop: driveA = floppy0
>              dev-prop: driveB =<null>
>              isa irq 6

Patch is on the list.

> * -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd,driveB=fd

Oh, -global works like -set, i.e. you can't set multiple properties in 
one go.  Should be '-global isa-fdc.driveA=fd -global isa-fdc.driveB=fd'.

The error reporting is *ahem*, well, not perfect.  Patch is on the list.

cheers,
   Gerd
Markus Armbruster - Dec. 16, 2009, 2:45 p.m.
Gerd Hoffmann <kraxel@redhat.com> writes:

> On 12/16/09 11:03, Markus Armbruster wrote:
>> This fixes "-global isa-fdc,driveB=DRIVE-ID" for me, but problems
>> remain:
>>
>> * -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd
>>
>>            dev: isa-fdc, id ""
>>              dev-prop: driveA = floppy0
>>              dev-prop: driveB =<null>
>>              isa irq 6
>
> Patch is on the list.

Subject: [Qemu-devel] [FOR 0.12 PATCH 1/2] defaults: split default_drive
Subject: [Qemu-devel] [FOR 0.12 PATCH 2/2] defaults: update device_list[]

Works fine for me, just one oddity: setting any of the fdc's two drives
kills the default, i.e. setting driveB makes driveA empty.

>> * -drive if=none,id=fd,file=fd.img -global isa-fdc.driveA=fd,driveB=fd
>
> Oh, -global works like -set, i.e. you can't set multiple properties in
> one go.  Should be '-global isa-fdc.driveA=fd -global
> isa-fdc.driveB=fd'.
>
> The error reporting is *ahem*, well, not perfect.  Patch is on the list.

Subject: [Qemu-devel] [FOR 0.12 PATCH] qdev: improve property error reporting.

Works fine now.

Patch

diff --git a/hw/fdc.c b/hw/fdc.c
index 11ea439..2e5c7a1 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1885,8 +1885,10 @@  fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
     ISADevice *dev;
 
     dev = isa_create("isa-fdc");
-    qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
-    qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
+    if (fds[0])
+        qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
+    if (fds[1])
+        qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
     if (qdev_init(&dev->qdev) < 0)
         return NULL;
     return &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
@@ -1904,8 +1906,10 @@  fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
     fdctrl = &sys->state;
     fdctrl->dma_chann = dma_chann; /* FIXME */
-    qdev_prop_set_drive(dev, "driveA", fds[0]);
-    qdev_prop_set_drive(dev, "driveB", fds[1]);
+    if (fds[0])
+        qdev_prop_set_drive(dev, "driveA", fds[0]);
+    if (fds[1])
+        qdev_prop_set_drive(dev, "driveB", fds[1]);
     qdev_init_nofail(dev);
     sysbus_connect_irq(&sys->busdev, 0, irq);
     sysbus_mmio_map(&sys->busdev, 0, mmio_base);
@@ -1921,7 +1925,8 @@  fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
     fdctrl_t *fdctrl;
 
     dev = qdev_create(NULL, "SUNW,fdtwo");
-    qdev_prop_set_drive(dev, "drive", fds[0]);
+    if (fds[0])
+        qdev_prop_set_drive(dev, "drive", fds[0]);
     qdev_init_nofail(dev);
     sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
     fdctrl = &sys->state;