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

[Paolo Bonzini <pbonzini@redhat.com>]

Il 08/01/2014 15:35, Markus Armbruster ha scritto:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
>> Leaving only those that will be affected by the patch:
> 
> You omitted akita, borzoi, connex, mainstone, nuri, smdkc210, spitz,
> terrier, tosa, verdex, z2, s390-virtio.  Why won't they be affected?

Because the dup bus names are hardcoded in the board:

    i2cbus = i2c_init_bus(dev, "dummy");

or in the device:

    s->spi = g_new(SSIBus *, s->num_busses);
    for (i = 0; i < s->num_busses; ++i) {
        char bus_name[16];
        snprintf(bus_name, 16, "spi%d", i);
        s->spi[i] = ssi_create_bus(dev, bus_name);
    }

Only dups of the "xyzzy.NN" form have their bus names created by qdev
core.  For other buses this patch changes nothing (neither for better,
nor for worse).

> You also omitted the machines that I can't get to start, but I'm not
> overly worried by them, because they're all either Xen, where I don't
> expect differences to plain x86, or ppcemb, where Alex gets to clean up
> any mess he might make.

Right.  In particular xenfv is just a PIIX PC, plus the (non qdev) Xen
PV bus.  And xenpv is just the Xen PV bus.

>> Il 07/01/2014 18:34, Markus Armbruster ha scritto:
>>>     target      machine         bus id              times
>>>     aarch64     n800            i2c-bus.0           2
>>>     aarch64     n810            i2c-bus.0           2
>>>     arm         n800            i2c-bus.0           2
>>>     arm         n810            i2c-bus.0           2
>>
>> Devices are created explicitly on one of the two buses, using
>> s->mpu->i2c[0], so no change to the guest.
>>
>>>     aarch64     vexpress-a15    virtio-mmio-bus.0   4
>>>     aarch64     vexpress-a9     virtio-mmio-bus.0   4
>>>     aarch64     virt            virtio-mmio-bus.0   32
>>>     arm         vexpress-a15    virtio-mmio-bus.0   4
>>>     arm         vexpress-a9     virtio-mmio-bus.0   4
>>>     arm         virt            virtio-mmio-bus.0   32
>>
>> With Alex's patch we get the ability to plug the device in a particular
>> slot.  If anyone was using virtio-mmio-bus.0 explicitly, they get the
>> first slot instead of the 4th or 32nd.  Bugfix.
> 
> Doesn't this break migration?  If yes, do we care?

I don't know for sure, but probably not.  sysbus doesn't implement
get_dev_path, so it relies on the old instance_id mechanism to
distinguish devices.  instance_id is unreliable in general (e.g. with
hotplug), but for command-lines and no hot-plug/hot-unplug it should
work.  You do have to be careful and specify bus=virtio-mmio-bus.31 on
the destination if you used bus=virtio-mmio-bus.0 on the source.

BTW if you didn't use bus=virtio-mmio-bus.0, nothing changes because the
logic in qbus_find_recursive is unaffected.

>>>     mips        mips            ide.0               2
>>>     mips64      mips            ide.0               2
>>>     mips64el    mips            ide.0               2
>>>     mipsel      mips            ide.0               2
>>
>> Not affected, the bus is not stored anywhere.
> 
> Isn't command line use and migration affected, just like everywhere
> else?

Right, command-line use of ide.0.  Bugfix as in Alex's PPC case, because
makes everything else consistent with PCI IDE which is the only place
where bus=ide.N worked.

Migration is not affected unless you used ide.0 on the command line.  In
other words, migration from old "-drive if=ide,bus=N" to new "-drive
if=none ... -device ...,bus=ide.N" should work.

>>>     ppc         g3beige         ide.0               2
>>>     ppc         mac99           ide.0               2
>>>     ppc         prep            ide.0               2
>>>     ppc64       g3beige         ide.0               2
>>>     ppc64       mac99           ide.0               2
>>>     ppc64       prep            ide.0               2
>>
>> Trusting Alex's tests here.
> 
> Our analysis should be recorded in the commit message.  With that done,
> I could R-by the patch.

Alex, can you spin v3 with a new commit message?

Paolo