Re: [PULL v3 0/8] Block layer patches

[Kevin Wolf <kwolf@redhat.com>]

Am 20.11.2024 um 14:19 hat Peter Maydell geschrieben:
> On Wed, 20 Nov 2024 at 10:52, Kevin Wolf <kwolf@redhat.com> wrote:
> >
> > The following changes since commit e6459afb1ff4d86b361b14f4a2fc43f0d2b4d679:
> >
> >   Merge tag 'pull-target-arm-20241119' of https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-11-19 14:23:34 +0000)
> >
> > are available in the Git repository at:
> >
> >   https://repo.or.cz/qemu/kevin.git tags/for-upstream
> >
> > for you to fetch changes up to 83987bf722b6b692bc745b47901be76a1c97140b:
> >
> >   vl: use qmp_device_add() in qemu_create_cli_devices() (2024-11-20 11:47:49 +0100)
> >
> > ----------------------------------------------------------------
> > Block layer patches
> >
> > - Fix qmp_device_add() to not throw non-scalar options away (fixes
> >   iothread-vq-mapping being silently ignored in device_add)
> > - Fix qdev property crash with integer PCI addresses and JSON -device
> > - iotests: Fix mypy failure
> > - parallels: Avoid potential integer overflow
> > - Fix crash in migration_is_running()
> 
> Hi; the hotplug_blk.py:HotPlug.test avocado seems to be failing:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/8423313170
> https://gitlab.com/qemu-project/qemu/-/jobs/8423313162
> 
> [stdlog] 2024-11-20 12:49:35,669 avocado.test test L0740 ERROR| FAIL
> 1-tests/avocado/hotplug_blk.py:HotPlug.test -> AssertionError: 1 != 0
> : Guest command failed: test -e /sys/block/vda
> 
> https://qemu-project.gitlab.io/-/qemu/-/jobs/8423313162/artifacts/build/tests/results/latest/test-results/09-tests_avocado_hotplug_blk.py_HotPlug.test/debug.log
> 
> Looks like the test called device_add, it succeeded, but
> it didn't see the /sys/block/vda node appear in the guest.
> 
> (The test logic of "try the command, if it fails sleep for 1
> second then try a second time and if that fails call it a
> test error" doesn't seem super robust in the face of slow
> CI runners, but OTOH it failed the same way on both jobs,
> so I don't think that is the culprit here.)

This looks like a bug in the test case that was previously cancelled out
by a QEMU bug. :-/

{
  "execute": "device_add",
  "arguments": {
    "driver": "virtio-blk-pci",
    "drive": "disk",
    "id": "virtio-disk0",
    "bus": "pci.1",
    "addr": 1
  },
  "id": "__qmp#00002"
}

What it actually meant is "addr": "1". It's an unfortunate interface,
but string "1" and integer 1 mean different things for PCI address
properties... Going through QemuOpts turned everything into strings, so
that masked the bug in the test case.

Should I just fix the test case and move on, or are we concerned about
other users having a similar bug and want to move the change to 10.0,
keeping device_add with iothread-vq-mapping broken in 9.2?

As far as I can tell, libvirt always uses the string form, so everything
using libvirt should be fine either way. Peter (Krempa), can you
confirm?

Kevin