On 7 July 2016 at 17:15, Andrew Jones <[email protected]> wrote: > On Tue, Jul 05, 2016 at 07:23:14PM +0200, Laszlo Ersek wrote: >> At the moment the following QEMU command line triggers an assertion >> failure (minimal reproducer by Cole): >> >> qemu-system-aarch64 \ >> -machine virt-2.6,accel=tcg \ >> -nodefaults \ >> -no-user-config \ >> -nographic -monitor stdio \ >> -device virtio-scsi-device,id=scsi0 \ >> -device virtio-scsi-device,id=scsi1 \ >> -drive file=foo.img,format=raw,if=none,id=d0 \ >> -device scsi-hd,bus=scsi0.0,drive=d0 \ >> -drive file=foo.img,format=raw,if=none,id=d1 \ >> -device scsi-hd,bus=scsi1.0,drive=d1 >> >> qemu-system-aarch64: migration/savevm.c:615: >> vmstate_register_with_alias_id: >> Assertion `!se->compat || se->instance_id == 0' failed. >> >> The reason is that the vmstate sections for the two scsi-hd devices are >> not uniquely identifiable by name. >> >> The direct parent buses of the scsi-hd devices -- scsi0.0 and scsi1.0 -- >> support the BusClass.get_dev_path member function. scsibus_get_dev_path() >> formats a device path prefix with the help of its topologically parent >> bus, and then appends the chan:id:lun triplet to it. For both scsi-hd >> devices, this triplet is 0:0:0. >> >> (Here we use "device path" in the QEMU migration sense, for vmstate >> section identification, not in the OFW or UEFI device path senses.) >> >> The virtio-scsi HBA is plugged into the virtio-mmio bus (implemented by >> the internal VirtIOMMIOProxy device). This bus class >> (TYPE_VIRTIO_MMIO_BUS) inherits, as its get_dev_path() member function, >> the virtio_bus_get_dev_path() method from its parent class >> (TYPE_VIRTIO_BUS). >> >> virtio_bus_get_dev_path() does not format any kind of device address on >> its own; "virtio addresses" are transport-specific. Therefore >> virtio_bus_get_dev_path() asks the topologically parent bus of the proxy >> object (implementing the specific virtio transport) to format the address >> of the proxy object. >> >> (For virtio-pci devices (where the proxy is an instance of VirtIOPCIProxy, >> plugged into a PCI bus), this ends up in pcibus_get_dev_path().) >> >> However, VirtIOMMIOProxy is usually (in practice: always) plugged into >> "main-system-bus", the singleton TYPE_SYSTEM_BUS object. This BusClass >> does not support formatting QEMU vmstate device paths at all (as >> SysBusDevice objects can have zero or more IO ports and zero or more MMIO >> regions). Hence the formatting request delegated from >> virtio_bus_get_dev_path() gets answered with NULL. >> >> The end result is that the two scsi-hd devices end up with the same device >> path "0:0:0", which triggers the assert. >> >> We can solve this by recognizing that virtio-mmio transports are >> distinguished from each other by their base addresses in MMIO address >> space. Implement virtio_mmio_bus_get_dev_path() as follows: >> >> (1) The virtio device whose devpath is to be formatted resides on a >> virtio-mmio bus that is implemented by a VirtIOMMIOProxy object. Ask >> the parent bus of VirtIOMMIOProxy to format the device path of >> VirtIOMMIOProxy, as a path prefix. (This is identical to what >> virtio_bus_get_dev_path() does.) >> >> (2) Append the base address of VirtIOMMIOProxy to the device path, such >> as: >> - virtio-mmio@000000000a003e00, >> - virtio-mmio@000000000a003c00. >> >> Given that these device paths are placed in the migration stream, step (2) >> above, if done unconditionally, would break migration. So make that step >> conditional on a new VirtIOMMIOProxy property, which is enabled for 2.7 >> machine types and later. >> >> Cc: "Michael S. Tsirkin" <[email protected]> >> Cc: Cole Robinson <[email protected]> >> Cc: Dr. David Alan Gilbert <[email protected]> >> Cc: Kevin Zhao <[email protected]> >> Cc: Peter Maydell <[email protected]> >> Cc: Tom Hanson <[email protected]> >> Reported-by: Kevin Zhao <[email protected]> >> Fixes: https://bugs.launchpad.net/qemu/+bug/1594239 >> Signed-off-by: Laszlo Ersek <[email protected]>
> FWIW, this patch looks good to me. > > Reviewed-by: Andrew Jones <[email protected]> Thanks; applied to target-arm.next. -- PMM
