On Wed, 22 Jan 2020 22:47:42 +0100
Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> > This test is failing on OSX:
> >
> > TestFail: machine type pc-i440fx-2.0: <class 'TypeError'>
> >
> > Looking at my job-results/job-2020-01-22T17.54-92b7fae/job.log:
> >
> > Unexpected error in object_property_find() at qom/object.c:1201:
> > qemu-system-x86_64: -device virtio-blk-pci,id=scsi0,drive=drive0: can't
> > apply global virtio-blk-device.scsi=true: Property '.scsi' not found
> >
> > Which makes sense looking at hw/block/virtio-blk.c:
> >
> > 1261 static Property virtio_blk_properties[] = {
> > 1262 DEFINE_BLOCK_PROPERTIES(VirtIOBlock, conf.conf),
> > ...
> > 1268 #ifdef __linux__
> > 1269 DEFINE_PROP_BIT64("scsi", VirtIOBlock, host_features,
> > 1270 VIRTIO_BLK_F_SCSI, false),
> > 1271 #endif
> >
> > Except code moved around, origin is:
> >
> > $ git show 1063b8b15
> > commit 1063b8b15fb49fcf88ffa282b19aaaf7ca9c678c
> > Author: Christoph Hellwig <h...@lst.de>
> > Date: Mon Apr 27 10:29:14 2009 +0200
> >
> > virtio-blk: add SGI_IO passthru support
> >
> > Add support for SG_IO passthru (packet commands) to the virtio-blk
> > backend. Conceptually based on an older patch from Hannes Reinecke
> > but largely rewritten to match the code structure and layering in
> > virtio-blk.
> >
> > Note that currently we issue the hose SG_IO synchronously. We could
> > easily switch to async I/O, but that would required either bloating
> > the VirtIOBlockReq by the size of struct sg_io_hdr or an additional
> > memory allocation for each SG_IO request.
> >
> > I'm not sure what is the correct way to fix this.
>
> ... because of:
>
> $ git show ed65fd1a27
> commit ed65fd1a2750d24290354cc7ea49caec7c13e30b
> Author: Cornelia Huck <cornelia.h...@de.ibm.com>
> Date: Fri Oct 16 12:25:54 2015 +0200
>
> virtio-blk: switch off scsi-passthrough by default
>
> Devices that are compliant with virtio-1 do not support scsi
> passthrough any more (and it has not been a recommended setup
> anyway for quite some time). To avoid having to switch it off
> explicitly in newer qemus that turn on virtio-1 by default, let's
> switch the default to scsi=false for 2.5.
>
> Signed-off-by: Cornelia Huck <cornelia.h...@de.ibm.com>
> Message-id: 1444991154-79217-4-git-send-email-cornelia.h...@de.ibm.com
> Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
>
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 095de5d12f..93e71afb4a 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,11 @@
> #define HW_COMPAT_H
>
> #define HW_COMPAT_2_4 \
> - /* empty */
> + {\
> + .driver = "virtio-blk-device",\
> + .property = "scsi",\
> + .value = "true",\
> + },
This code has changed a lot in the meantime...
>
> #define HW_COMPAT_2_3 \
> {\
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 3e230debb8..45a24e4fa6 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -972,7 +972,7 @@ static Property virtio_blk_properties[] = {
> DEFINE_PROP_STRING("serial", VirtIOBlock, conf.serial),
> DEFINE_PROP_BIT("config-wce", VirtIOBlock, conf.config_wce, 0, true),
> #ifdef __linux__
> - DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, true),
> + DEFINE_PROP_BIT("scsi", VirtIOBlock, conf.scsi, 0, false),
> #endif
> DEFINE_PROP_BIT("request-merging", VirtIOBlock,
> conf.request_merging, 0,
> true),
>
> Should this HW_COMPAT_2_4 entry be guarded with ifdef __linux__?
... so something like the following might do the trick:
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 3e288bfceb7f..d8e30e4895d8 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -148,7 +148,8 @@ GlobalProperty hw_compat_2_5[] = {
const size_t hw_compat_2_5_len = G_N_ELEMENTS(hw_compat_2_5);
GlobalProperty hw_compat_2_4[] = {
- { "virtio-blk-device", "scsi", "true" },
+ /* Optional because the 'scsi' property is Linux-only */
+ { "virtio-blk-device", "scsi", "true", .optional = true },
{ "e1000", "extra_mac_registers", "off" },
{ "virtio-pci", "x-disable-pcie", "on" },
{ "virtio-pci", "migrate-extra", "off" },
>
> Probably nobody ran a pre-2.4 machine out of Linux =)
>
Yeah. I'm wondering if there's more compat stuff in there that should
be optional. Devices that simply do not exist are not a problem, but
properties that not always exist are.
