diff mbox

[v3,2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

Message ID 1429613471-7944-3-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao April 21, 2015, 10:51 a.m. UTC 
From: Shannon Zhao <shannon.zhao@linaro.org>

Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
The transports just sync the host features from backend.

Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
 hw/s390x/s390-virtio-bus.c      | 1 -
 hw/s390x/virtio-ccw.c           | 1 -
 hw/scsi/virtio-scsi.c           | 5 +++++
 hw/virtio/virtio-pci.c          | 1 -
 include/hw/virtio/virtio-scsi.h | 1 +
 5 files changed, 6 insertions(+), 3 deletions(-)

Comments

Shannon Zhao April 28, 2015, 12:32 a.m. UTC | #1 
Ping?

On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
> The transports just sync the host features from backend.
> 
> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
>  hw/s390x/s390-virtio-bus.c      | 1 -
>  hw/s390x/virtio-ccw.c           | 1 -
>  hw/scsi/virtio-scsi.c           | 5 +++++
>  hw/virtio/virtio-pci.c          | 1 -
>  include/hw/virtio/virtio-scsi.h | 1 +
>  5 files changed, 6 insertions(+), 3 deletions(-)
Michael S. Tsirkin April 28, 2015, 5:55 a.m. UTC | #2 
Pong.
You responded to Cornelia's comments on patch 1/2
with "ok will add".
I expected v4.

On Tue, Apr 28, 2015 at 08:32:21AM +0800, Shannon Zhao wrote:
> Ping?
> 
> On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
> > From: Shannon Zhao <shannon.zhao@linaro.org>
> > 
> > Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
> > The transports just sync the host features from backend.
> > 
> > Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> > Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> > ---
> >  hw/s390x/s390-virtio-bus.c      | 1 -
> >  hw/s390x/virtio-ccw.c           | 1 -
> >  hw/scsi/virtio-scsi.c           | 5 +++++
> >  hw/virtio/virtio-pci.c          | 1 -
> >  include/hw/virtio/virtio-scsi.h | 1 +
> >  5 files changed, 6 insertions(+), 3 deletions(-)
> 
> -- 
> Shannon
Shannon Zhao April 28, 2015, 6:30 a.m. UTC | #3 
On 2015/4/28 13:55, Michael S. Tsirkin wrote:
> Pong.
> You responded to Cornelia's comments on patch 1/2
> with "ok will add".
> I expected v4.
> 

Yeah, but few comments on this patch, so I just want to confirm this
modification is acceptable to virtio-scsi, then will modify the commit
log and send v4.

> On Tue, Apr 28, 2015 at 08:32:21AM +0800, Shannon Zhao wrote:
>> Ping?
>>
>> On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>
>>> Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
>>> The transports just sync the host features from backend.
>>>
>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>> ---
>>>  hw/s390x/s390-virtio-bus.c      | 1 -
>>>  hw/s390x/virtio-ccw.c           | 1 -
>>>  hw/scsi/virtio-scsi.c           | 5 +++++
>>>  hw/virtio/virtio-pci.c          | 1 -
>>>  include/hw/virtio/virtio-scsi.h | 1 +
>>>  5 files changed, 6 insertions(+), 3 deletions(-)
>>
>> -- 
>> Shannon
> 
> .
>
Michael S. Tsirkin April 28, 2015, 7:15 a.m. UTC | #4 
On Tue, Apr 28, 2015 at 02:30:17PM +0800, Shannon Zhao wrote:
> On 2015/4/28 13:55, Michael S. Tsirkin wrote:
> > Pong.
> > You responded to Cornelia's comments on patch 1/2
> > with "ok will add".
> > I expected v4.
> > 
> 
> Yeah, but few comments on this patch, so I just want to confirm this
> modification is acceptable to virtio-scsi, then will modify the commit
> log and send v4.

My question would be, why just net and scsi?
Does not the same apply to all devices?

> > On Tue, Apr 28, 2015 at 08:32:21AM +0800, Shannon Zhao wrote:
> >> Ping?
> >>
> >> On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
> >>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>
> >>> Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
> >>> The transports just sync the host features from backend.
> >>>
> >>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>> ---
> >>>  hw/s390x/s390-virtio-bus.c      | 1 -
> >>>  hw/s390x/virtio-ccw.c           | 1 -
> >>>  hw/scsi/virtio-scsi.c           | 5 +++++
> >>>  hw/virtio/virtio-pci.c          | 1 -
> >>>  include/hw/virtio/virtio-scsi.h | 1 +
> >>>  5 files changed, 6 insertions(+), 3 deletions(-)
> >>
> >> -- 
> >> Shannon
> > 
> > .
> > 
>
Shannon Zhao April 28, 2015, 7:28 a.m. UTC | #5 
On 2015/4/28 15:15, Michael S. Tsirkin wrote:
> On Tue, Apr 28, 2015 at 02:30:17PM +0800, Shannon Zhao wrote:
>> On 2015/4/28 13:55, Michael S. Tsirkin wrote:
>>> Pong.
>>> You responded to Cornelia's comments on patch 1/2
>>> with "ok will add".
>>> I expected v4.
>>>
>>
>> Yeah, but few comments on this patch, so I just want to confirm this
>> modification is acceptable to virtio-scsi, then will modify the commit
>> log and send v4.
> 
> My question would be, why just net and scsi?
> Does not the same apply to all devices?
> 

The virtio-blk has been changed before, while leaving net and scsi.
And the other devices don't set host features in the wrappers.

>>> On Tue, Apr 28, 2015 at 08:32:21AM +0800, Shannon Zhao wrote:
>>>> Ping?
>>>>
>>>> On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
>>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>>>>
>>>>> Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
>>>>> The transports just sync the host features from backend.
>>>>>
>>>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>>>>> ---
>>>>>  hw/s390x/s390-virtio-bus.c      | 1 -
>>>>>  hw/s390x/virtio-ccw.c           | 1 -
>>>>>  hw/scsi/virtio-scsi.c           | 5 +++++
>>>>>  hw/virtio/virtio-pci.c          | 1 -
>>>>>  include/hw/virtio/virtio-scsi.h | 1 +
>>>>>  5 files changed, 6 insertions(+), 3 deletions(-)
>>>>
>>>> -- 
>>>> Shannon
>>>
>>> .
>>>
>>
> 
> .
>
Michael S. Tsirkin April 28, 2015, 8:18 a.m. UTC | #6 
On Tue, Apr 28, 2015 at 03:28:51PM +0800, Shannon Zhao wrote:
> On 2015/4/28 15:15, Michael S. Tsirkin wrote:
> > On Tue, Apr 28, 2015 at 02:30:17PM +0800, Shannon Zhao wrote:
> >> On 2015/4/28 13:55, Michael S. Tsirkin wrote:
> >>> Pong.
> >>> You responded to Cornelia's comments on patch 1/2
> >>> with "ok will add".
> >>> I expected v4.
> >>>
> >>
> >> Yeah, but few comments on this patch, so I just want to confirm this
> >> modification is acceptable to virtio-scsi, then will modify the commit
> >> log and send v4.
> > 
> > My question would be, why just net and scsi?
> > Does not the same apply to all devices?
> > 
> 
> The virtio-blk has been changed before, while leaving net and scsi.
> And the other devices don't set host features in the wrappers.


They just use DEFINE_VIRTIO_COMMON_FEATURES but it's the
same issue.

> >>> On Tue, Apr 28, 2015 at 08:32:21AM +0800, Shannon Zhao wrote:
> >>>> Ping?
> >>>>
> >>>> On 2015/4/21 18:51, shannon.zhao@linaro.org wrote:
> >>>>> From: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>>
> >>>>> Move DEFINE_VIRTIO_SCSI_FEATURES to the backend virtio-scsi.
> >>>>> The transports just sync the host features from backend.
> >>>>>
> >>>>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
> >>>>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> >>>>> ---
> >>>>>  hw/s390x/s390-virtio-bus.c      | 1 -
> >>>>>  hw/s390x/virtio-ccw.c           | 1 -
> >>>>>  hw/scsi/virtio-scsi.c           | 5 +++++
> >>>>>  hw/virtio/virtio-pci.c          | 1 -
> >>>>>  include/hw/virtio/virtio-scsi.h | 1 +
> >>>>>  5 files changed, 6 insertions(+), 3 deletions(-)
> >>>>
> >>>> -- 
> >>>> Shannon
> >>>
> >>> .
> >>>
> >>
> > 
> > .
> > 
> 
> 
> -- 
> Thanks,
> Shannon
Peter Maydell April 28, 2015, 9:29 a.m. UTC | #7 
On 28 April 2015 at 09:18, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Apr 28, 2015 at 03:28:51PM +0800, Shannon Zhao wrote:
>> On 2015/4/28 15:15, Michael S. Tsirkin wrote:
>> > My question would be, why just net and scsi?
>> > Does not the same apply to all devices?
>> >
>>
>> The virtio-blk has been changed before, while leaving net and scsi.
>> And the other devices don't set host features in the wrappers.
>
>
> They just use DEFINE_VIRTIO_COMMON_FEATURES but it's the
> same issue.

No, DEFINE_VIRTIO_COMMON_FEATURES are all transport features,
not backend features. These are already correctly placed
in the transports.

-- PMM
diff mbox

Patch

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index b893e02..c8a78ba 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -622,7 +622,6 @@  static const TypeInfo virtio_s390_device_info = {
 
 static Property s390_virtio_scsi_properties[] = {
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VIRTIO_SCSI_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 1252162..ef97fe9 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1506,7 +1506,6 @@  static const TypeInfo virtio_ccw_balloon = {
 
 static Property virtio_ccw_scsi_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VIRTIO_SCSI_FEATURES(VirtioCcwDevice, host_features[0]),
     DEFINE_PROP_BIT("ioeventfd", VirtioCcwDevice, flags,
                     VIRTIO_CCW_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index c9bea06..e242fef 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -631,6 +631,10 @@  static void virtio_scsi_set_config(VirtIODevice *vdev,
 static uint32_t virtio_scsi_get_features(VirtIODevice *vdev,
                                          uint32_t requested_features)
 {
+    VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+
+    /* Firstly sync all virtio-scsi possible supported features */
+    requested_features |= s->host_features;
     return requested_features;
 }
 
@@ -945,6 +949,7 @@  static void virtio_scsi_device_unrealize(DeviceState *dev, Error **errp)
 
 static Property virtio_scsi_properties[] = {
     DEFINE_VIRTIO_SCSI_PROPERTIES(VirtIOSCSI, parent_obj.conf),
+    DEFINE_VIRTIO_SCSI_FEATURES(VirtIOSCSI, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c6b99f9..5c173c4 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1078,7 +1078,6 @@  static Property virtio_scsi_pci_properties[] = {
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors,
                        DEV_NVECTORS_UNSPECIFIED),
-    DEFINE_VIRTIO_SCSI_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index f93b57d..b42e7f1 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -98,6 +98,7 @@  typedef struct VirtIOSCSI {
     bool dataplane_fenced;
     Error *blocker;
     Notifier migration_state_notifier;
+    uint32_t host_features;
 } VirtIOSCSI;
 
 typedef struct VirtIOSCSIReq {