qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default
@ 2015-10-16  8:40 Cornelia Huck
  2015-10-16  8:41 ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Cornelia Huck @ 2015-10-16  8:40 UTC (permalink / raw)
  To: qemu-devel
  Cc: kwolf, qemu-block, mst, jasowang, stefanha, Cornelia Huck,
	pbonzini

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.huck@de.ibm.com>
---

Note: this is based upon my s390x series from 10/14, as I otherwise
could not test compat on s390x.

---
 hw/block/virtio-blk.c      | 2 +-
 hw/s390x/s390-virtio-ccw.c | 4 ++++
 include/hw/compat.h        | 6 +++++-
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 8beb26b..999dbd7 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -975,7 +975,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),
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 20883ff..2c72358 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -272,6 +272,10 @@ static const TypeInfo ccw_machine_info = {
             .driver   = "vhost-scsi-ccw",\
             .property = "max_revision",\
             .value    = "0",\
+        },{\
+            .driver   = "virtio-blk-ccw",\
+            .property = "scsi",\
+            .value    = "true",\
         },
 
 static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 095de5d..bbf1ab2 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-pci",\
+            .property = "scsi",\
+            .value    = "true",\
+        },
 
 #define HW_COMPAT_2_3 \
         {\
-- 
2.3.9

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default
  2015-10-16  8:40 [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default Cornelia Huck
@ 2015-10-16  8:41 ` Paolo Bonzini
  2015-10-16  8:46   ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2015-10-16  8:41 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: kwolf, jasowang, stefanha, qemu-block, mst



On 16/10/2015 10:40, Cornelia Huck wrote:
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -272,6 +272,10 @@ static const TypeInfo ccw_machine_info = {
>              .driver   = "vhost-scsi-ccw",\
>              .property = "max_revision",\
>              .value    = "0",\
> +        },{\
> +            .driver   = "virtio-blk-ccw",\
> +            .property = "scsi",\
> +            .value    = "true",\
>          },
>  
>  static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 095de5d..bbf1ab2 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-pci",\
> +            .property = "scsi",\
> +            .value    = "true",\
> +        },
>  
>  #define HW_COMPAT_2_3 \
>          {\
> 

s390 should use HW_COMPAT_2_4 as well.  Otherwise looks good.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default
  2015-10-16  8:41 ` Paolo Bonzini
@ 2015-10-16  8:46   ` Paolo Bonzini
  2015-10-16  8:54     ` Cornelia Huck
  0 siblings, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2015-10-16  8:46 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: kwolf, jasowang, qemu-block, stefanha, mst



On 16/10/2015 10:41, Paolo Bonzini wrote:
> 
> 
> On 16/10/2015 10:40, Cornelia Huck wrote:
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -272,6 +272,10 @@ static const TypeInfo ccw_machine_info = {
>>              .driver   = "vhost-scsi-ccw",\
>>              .property = "max_revision",\
>>              .value    = "0",\
>> +        },{\
>> +            .driver   = "virtio-blk-ccw",\
>> +            .property = "scsi",\
>> +            .value    = "true",\
>>          },
>>  
>>  static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
>> diff --git a/include/hw/compat.h b/include/hw/compat.h
>> index 095de5d..bbf1ab2 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-pci",\
>> +            .property = "scsi",\
>> +            .value    = "true",\
>> +        },
>>  
>>  #define HW_COMPAT_2_3 \
>>          {\
>>
> 
> s390 should use HW_COMPAT_2_4 as well.  Otherwise looks good.

Hmm, ECONCISE probably.  Sorry.

I mean that virtio-blk-ccw's scsi property should IMO go in
HW_COMPAT_2_4 as well.

But I noticed now that:

* if it works it would be even better if the compat property used
virtio-blk-device;

* a new pseries-2.4 machine also needs to be created.

Thanks,

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default
  2015-10-16  8:46   ` Paolo Bonzini
@ 2015-10-16  8:54     ` Cornelia Huck
  2015-10-16  9:02       ` Paolo Bonzini
  0 siblings, 1 reply; 5+ messages in thread
From: Cornelia Huck @ 2015-10-16  8:54 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kwolf, qemu-block, mst, jasowang, qemu-devel, stefanha

On Fri, 16 Oct 2015 10:46:31 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> 
> 
> On 16/10/2015 10:41, Paolo Bonzini wrote:
> > 
> > 
> > On 16/10/2015 10:40, Cornelia Huck wrote:
> >> --- a/hw/s390x/s390-virtio-ccw.c
> >> +++ b/hw/s390x/s390-virtio-ccw.c
> >> @@ -272,6 +272,10 @@ static const TypeInfo ccw_machine_info = {
> >>              .driver   = "vhost-scsi-ccw",\
> >>              .property = "max_revision",\
> >>              .value    = "0",\
> >> +        },{\
> >> +            .driver   = "virtio-blk-ccw",\
> >> +            .property = "scsi",\
> >> +            .value    = "true",\
> >>          },
> >>  
> >>  static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
> >> diff --git a/include/hw/compat.h b/include/hw/compat.h
> >> index 095de5d..bbf1ab2 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-pci",\
> >> +            .property = "scsi",\
> >> +            .value    = "true",\
> >> +        },
> >>  
> >>  #define HW_COMPAT_2_3 \
> >>          {\
> >>
> > 
> > s390 should use HW_COMPAT_2_4 as well.  Otherwise looks good.
> 
> Hmm, ECONCISE probably.  Sorry.
> 
> I mean that virtio-blk-ccw's scsi property should IMO go in
> HW_COMPAT_2_4 as well.

I was wondering about the semantics of HW_COMPAT_*: Does any hw-related
compat stuff go in there, even if it is architecture specific (like
ccw)?
> 
> But I noticed now that:
> 
> * if it works it would be even better if the compat property used
> virtio-blk-device;

Hm. Previous virtio-compat always treated -pci explicitly, but we only
gained s390x compat handling with 2.4, so it didn't really matter. But
if it works, this is the saner approach.

> 
> * a new pseries-2.4 machine also needs to be created.

Ah, wasn't aware they do compat as well.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default
  2015-10-16  8:54     ` Cornelia Huck
@ 2015-10-16  9:02       ` Paolo Bonzini
  0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2015-10-16  9:02 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: kwolf, qemu-block, mst, jasowang, qemu-devel, stefanha



On 16/10/2015 10:54, Cornelia Huck wrote:
> On Fri, 16 Oct 2015 10:46:31 +0200
> Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
>>
>>
>> On 16/10/2015 10:41, Paolo Bonzini wrote:
>>>
>>>
>>> On 16/10/2015 10:40, Cornelia Huck wrote:
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -272,6 +272,10 @@ static const TypeInfo ccw_machine_info = {
>>>>              .driver   = "vhost-scsi-ccw",\
>>>>              .property = "max_revision",\
>>>>              .value    = "0",\
>>>> +        },{\
>>>> +            .driver   = "virtio-blk-ccw",\
>>>> +            .property = "scsi",\
>>>> +            .value    = "true",\
>>>>          },
>>>>  
>>>>  static void ccw_machine_2_4_class_init(ObjectClass *oc, void *data)
>>>> diff --git a/include/hw/compat.h b/include/hw/compat.h
>>>> index 095de5d..bbf1ab2 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-pci",\
>>>> +            .property = "scsi",\
>>>> +            .value    = "true",\
>>>> +        },
>>>>  
>>>>  #define HW_COMPAT_2_3 \
>>>>          {\
>>>>
>>>
>>> s390 should use HW_COMPAT_2_4 as well.  Otherwise looks good.
>>
>> Hmm, ECONCISE probably.  Sorry.
>>
>> I mean that virtio-blk-ccw's scsi property should IMO go in
>> HW_COMPAT_2_4 as well.
> 
> I was wondering about the semantics of HW_COMPAT_*: Does any hw-related
> compat stuff go in there, even if it is architecture specific (like
> ccw)?

It depends.  For stuff like your max_revision I guess it makes sense to
keep it in the board.  Similarly, x86 CPU flags go in PC_COMPAT_*.  But
for stuff like virtio-blk-ccw, it makes some sense to keep it close to
virtio-blk-pci.

>> But I noticed now that:
>>
>> * if it works it would be even better if the compat property used
>> virtio-blk-device;
> 
> Hm. Previous virtio-compat always treated -pci explicitly, but we only
> gained s390x compat handling with 2.4, so it didn't really matter. But
> if it works, this is the saner approach.

Yes, I agree.

Paolo

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-10-16  9:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-16  8:40 [Qemu-devel] [PATCH] virtio-blk: switch off scsi-passthrough by default Cornelia Huck
2015-10-16  8:41 ` Paolo Bonzini
2015-10-16  8:46   ` Paolo Bonzini
2015-10-16  8:54     ` Cornelia Huck
2015-10-16  9:02       ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).