Re: [PATCH v2 01/29] hw/core: Allow setting 'virtio-blk-device.scsi' property on OSX host

[Philippe Mathieu-Daudé]

On 1/30/20 1:20 PM, Cornelia Huck wrote:
> On Wed, 29 Jan 2020 22:23:17 +0100
> Philippe Mathieu-Daudé <address@hidden> wrote:
>
> > Commit ed65fd1a27 restricted the 'scsi' property to Linux hosts:
>
> It was restricted to Linux hosts before that commit; that only became
> problematic because we set it unconditionally in the compat code.
>
> >    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
>
> What about:
>
> Commit ed65fd1a2750 ("virtio-blk: switch off scsi-passthrough by
> default") changed the default value of the 'scsi' property of
> virtio-blk, which is only available on Linux hosts. It also added an
> unconditional compat entry for 2.4 or earlier machines.

OK!

> > When we try to set this property on a pre-2.4 machine on OSX,
>
> Pre-2.5, I think?

Yes :S

> > we get:
> >
> >     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
> >
> > Fix this error by marking the property optional.
> >
> > Fixes: ed65fd1a27
>
> Fixes: ed65fd1a2750 ("virtio-blk: switch off scsi-passthrough by default")

OK.

> > Suggested-by: Cornelia Huck <address@hidden>
> > Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> > ---
> >   hw/core/machine.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 3e288bfceb..d8e30e4895 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" },
>
> Reviewed-by: Cornelia Huck <address@hidden>

Thanks!