[Qemu-devel] [PATCH 0/4] block: Improve warnings for doubly-connected drives

[Peter Maydell <peter.maydell@linaro.org>]

This patchset attempts to improve the warning and error messages for
bad user command lines that attempt to connect a drive up to two
devices. The motivation here is patch #4, which changes the default
interface for the virt board to virtio. That will break some existing
command lines which forgot to specify if=none, and so I would like
us to at least diagnose that user error in a helpful way that points
the user towards adding the missing if=none.

The patchset improves some error messages, and makes some previously
undiagnosed mistakes into warnings. The changes (with sample x86
command lines to provoke them) are:

(1) Drive specified as to be auto-connected and also manually connected
(and the board does handle this if= type):

  qemu-system-x86_64 -nodefaults -display none -drive if=scsi,file=tmp.qcow2,id=foo -device ide-hd,drive=foo

Previously: an error:
  qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
  can't take value 'foo', it's in use

Now: a better error:
  qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
  can't be set to drive ID 'foo'; that drive has been automatically
  connected to another device. Use if=none if you do not want that
  automatic connection.

(2) As 1, but the board does not handle this if= type:

Previously: not diagnosed at all

Now: a warning:
  Warning: automatic connection of this drive requested (because if=sd
  was specified) but it was also connected manually to a device:
  id=foo,file=tmp.qcow2,if=sd,bus=0,unit=0
  (If you don't want this drive auto-connected, use if=none.)

[This means we now will always warn one way or another about drives which
have an if= auto-connect specified but which the board didn't pick up: either
they're also manually connected and get this warning, or they're not manually
connected, and get the orphan-drive warning. If the if= was due to the
board default rather than the user typing it specifically, the error message
text is slightly different to reflect that.]

(3) Drive specified to be manually connected in two different ways:

  qemu-system-x86_64 -nodefaults -display none -drive if=sd,file=tmp.qcow2,id=foo -device ide-hd,drive=foo -device ide-hd,drive=foo

Previously: an error:
  qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
  can't take value 'foo', it's in use

Now: a better error:
  qemu-system-x86_64: -device ide-hd,drive=foo: Property 'ide-hd.drive'
  can't be set to drive ID 'foo'; that drive has already been connected
  to another device.



In order to detect when a drive was auto-connected, we need to set a
flag in the DriveInfo when this happens.  we do this by assuming that
all calls to blk_by_legacy_dinfo() imply that we're about to assign
the drive to a device.  This is a slightly ugly place to make the
test, but simpler than trying to locate and change every place in the
code that does automatic drive handling, and the worst case is that
we might print out a spurious warning.


I include patch #4 as the motivation/context but in fact it doesn't
depend on the first 3, so if you want to take the first 3 via
block and have me put the 4th one in target-arm that's OK.

thanks
-- PMM


Peter Maydell (4):
  block: Warn if an if=<something> drive was also connected manually
  qdev-properties-system: Change set_pointer's parse callback to use
    Error
  qdev-properties-system: Improve error message for drive assignment
    conflict
  hw/arm/virt: Make block devices default to virtio

 block/block-backend.c            |  4 ++++
 blockdev.c                       | 39 ++++++++++++++++++++++++++++++++++
 hw/arm/virt.c                    |  2 ++
 hw/core/qdev-properties-system.c | 45 ++++++++++++++++++++++++++++------------
 include/sysemu/blockdev.h        |  2 ++
 5 files changed, 79 insertions(+), 13 deletions(-)

-- 
1.9.1