| Message ID | 20200129212345.20547-2-philmd@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/acceptance/virtio_seg_max_adjust: Restrict it to Linux/X86 | expand |
On Wed, 29 Jan 2020 22:23:17 +0100 Philippe Mathieu-Daudé <philmd@redhat.com> 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. > > When we try to set this property on a pre-2.4 machine on OSX, Pre-2.5, I think? > 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") > Suggested-by: Cornelia Huck <cohuck@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > 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 <cohuck@redhat.com>
On 1/30/20 1:20 PM, Cornelia Huck wrote: > On Wed, 29 Jan 2020 22:23:17 +0100 > Philippe Mathieu-Daudé <philmd@redhat.com> 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 <cohuck@redhat.com> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> >> --- >> 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 <cohuck@redhat.com> Thanks!
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" },
Commit ed65fd1a27 restricted the 'scsi' property to Linux hosts: 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 When we try to set this property on a pre-2.4 machine on OSX, 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 Suggested-by: Cornelia Huck <cohuck@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> --- hw/core/machine.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)