diff mbox

[V2,2/5] virtio-blk: advertise scsi only when scsi is set

Message ID 1436938201-16766-3-git-send-email-jasowang@redhat.com
State New
Headers show

Commit Message

Jason Wang July 15, 2015, 5:29 a.m. UTC
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/block/virtio-blk.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini July 15, 2015, 7:57 a.m. UTC | #1
On 15/07/2015 07:29, Jason Wang wrote:
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: qemu-block@nongnu.org
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/block/virtio-blk.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index 4c27974..761d763 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -731,7 +731,9 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
>      virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);
>      virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
>      virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
> -    virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> +    if (s->conf.scsi) {
> +        virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
> +    }

This must only be done for newer machine types only, or you change guest
ABI for scsi=off.  Effectively you have to split it in two properties,
"scsi" and "always_set_f_scsi".

Paolo
Jason Wang July 15, 2015, 8:31 a.m. UTC | #2
On 07/15/2015 03:57 PM, Paolo Bonzini wrote:
>
> On 15/07/2015 07:29, Jason Wang wrote:
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>> Cc: qemu-block@nongnu.org
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> ---
>>  hw/block/virtio-blk.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
>> index 4c27974..761d763 100644
>> --- a/hw/block/virtio-blk.c
>> +++ b/hw/block/virtio-blk.c
>> @@ -731,7 +731,9 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
>>      virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);
>>      virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
>>      virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
>> -    virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
>> +    if (s->conf.scsi) {
>> +        virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
>> +    }
> This must only be done for newer machine types only, or you change guest
> ABI for scsi=off.  Effectively you have to split it in two properties,
> "scsi" and "always_set_f_scsi".
>
> Paolo

And always_set_f_scsi is true only for legacy machine types?
Paolo Bonzini July 15, 2015, 8:33 a.m. UTC | #3
On 15/07/2015 10:31, Jason Wang wrote:
> > This must only be done for newer machine types only, or you change guest
> > ABI for scsi=off.  Effectively you have to split it in two properties,
> > "scsi" and "always_set_f_scsi".
>
> And always_set_f_scsi is true only for legacy machine types?

s/legacy/older/ :)

It's also ignored for modern devices.  So perhaps you could name it to
transitional_force_f_scsi or something like that.

Paolo
diff mbox

Patch

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 4c27974..761d763 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -731,7 +731,9 @@  static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features,
     virtio_add_feature(&features, VIRTIO_BLK_F_GEOMETRY);
     virtio_add_feature(&features, VIRTIO_BLK_F_TOPOLOGY);
     virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
-    virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
+    if (s->conf.scsi) {
+        virtio_add_feature(&features, VIRTIO_BLK_F_SCSI);
+    }
 
     if (s->conf.config_wce) {
         virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);