Re: [Qemu-devel] [PATCH] qdev: Keep global allocation counter per bus

[Markus Armbruster]

Alexander Graf <address@hidden> writes:

> When we have 2 separate qdev devices that both create a qbus of the
> same type without specifying a bus name or device name, we end up
> with two buses of the same name, such as ide.0 on the Mac machines:
>
>   dev: macio-ide, id ""
>     bus: ide.0
>       type IDE
>   dev: macio-ide, id ""
>     bus: ide.0
>       type IDE
>
> If we now spawn a device that connects to a ide.0 the last created
> bus gets the device, with the first created bus inaccessible to the
> command line.

isapc has the same issue: two onboard isa-ide devices, each providing a
bus, both buses named ide.0.

> After some discussion on IRC we concluded that the best quick fix way
> forward for this is to make automated bus-class type based allocation
> count a global counter. That's what this patch implements. With this
> we instead get
>
>   dev: macio-ide, id ""
>     bus: ide.1
>       type IDE
>   dev: macio-ide, id ""
>     bus: ide.0
>       type IDE
>
> on the example mentioned above.

Commit message should explain more clearly how and when this affects bus
names.

Patch breaks isapc:

    $ qemu -nodefaults -S -display none -monitor stdio -M isapc -drive 
if=none,id=drive0 -device ide-cd,drive=drive0
    (qemu) Segmentation fault (core dumped)

Debugging a bit:

    (gdb) bt
    #0  0x000055555572e745 in ide_get_geometry (bus=0x0, unit=0, cyls=
        0x7fffffffdb8a, heads=0x7fffffffdb88 "\210\271qU", secs=
        0x7fffffffdb89 "\271qU") at /home/armbru/work/qemu/hw/ide/qdev.c:129
    #1  0x00005555558f1fed in pc_cmos_init_late (opaque=0x55555628b420 
<arg.29452>)
        at /home/armbru/work/qemu/hw/i386/pc.c:336
    #2  0x0000555555898abc in qemu_devices_reset ()
        at /home/armbru/work/qemu/vl.c:1836
    #3  0x0000555555898b28 in qemu_system_reset (report=false)
        at /home/armbru/work/qemu/vl.c:1845
    #4  0x00005555558a0640 in main (argc=13, argv=0x7fffffffe048, envp=
        0x7fffffffe0b8) at /home/armbru/work/qemu/vl.c:4344
    (gdb) p arg->idebus
    $1 = {0x555556322e10, 0x0}
    (gdb) p i
    $2 = 2

Looks like your patch kills the second isa-ide somehow.

Your commit message doesn't state your command line, so I had to figure
out a PPC example myself:

    $ qemu-system-ppc -M mac99 -nodefaults -S -display none -monitor stdio 
-drive if=none,id=drive0 -device ide-cd,drive=drive0,bus=ide.0

"info qtree" before your patch:

      dev: macio-ide, id ""
        irq 2
        mmio ffffffffffffffff/0000000000001000
        bus: ide.0
          type IDE
          dev: ide-cd, id ""
            drive = drive0
            logical_block_size = 512
            physical_block_size = 512
            min_io_size = 0
            opt_io_size = 0
            bootindex = -1
            discard_granularity = 512
            ver = "1.7.50"
            wwn = 0x0
            serial = "QM00003"
            model = <null>
            unit = 0
      dev: macio-ide, id ""
        irq 2
        mmio ffffffffffffffff/0000000000001000
        bus: ide.0
          type IDE

After:

      dev: macio-ide, id ""
        irq 2
        mmio ffffffffffffffff/0000000000001000
        bus: ide.1
          type IDE
      dev: macio-ide, id ""
        irq 2
        mmio ffffffffffffffff/0000000000001000
        bus: ide.0
          type IDE
          dev: ide-cd, id ""
            drive = drive0
            logical_block_size = 512
            physical_block_size = 512
            min_io_size = 0
            opt_io_size = 0
            bootindex = -1
            discard_granularity = 512
            ver = "1.7.50"
            wwn = 0x0
            serial = "QM00001"
            model = <null>
            unit = 0

Incompatible change: device ide-cd moved to a different controller.
Great fun when you try to live migrate across your patch.

I'd expect isapc to have the same issue once its crash bug is fixed.

First law of QEMU hacking: if your patch looks simple, it's probably
wrong ;)