[Qemu-devel] [PATCH v3 0/2] virtio: Move host features to backends

[Qemu-devel] [PATCH v3 0/2] virtio: Move host features to backends

[shannon.zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

The reason to do this is that the virtio-net-device can't expose host
features to guest while using virtio-mmio. So the performance is low.

The virtio-*-pci, virtio-*-s390, and virtio-*-ccw already have the ability
to forward property accesses to the backend child, by calling 
*_virtio_*_instance_init -> qdev_alias_all_properties. So if we move the
host features to backends, it doesn't break the backwards compatibility
for virtio-*-pci, virtio-*-s390, and virtio-*-ccw.

Here we move the host features to backends, involving
DEFINE_VIRTIO_NET_FEATURES, DEFINE_VIRTIO_SCSI_FEATURES. So the virtio-mmio
devices could have the host freatures, and this has a great performance
improvement to virtio-mmio, especially to virtio-net-device.

changes since v2:
  * move virtio_net_set_config_size to virtio-net (Cornelia)

changes since v1:
  * drop unnecessary change of adding device_plugged hook for
    virtio-ccw and s390-virtio-bus (Cornelia)

Shannon Zhao (2):
  hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net
  virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

 hw/net/virtio-net.c             | 5 +++++
 hw/s390x/s390-virtio-bus.c      | 3 ---
 hw/s390x/virtio-ccw.c           | 3 ---
 hw/scsi/virtio-scsi.c           | 5 +++++
 hw/virtio/virtio-pci.c          | 3 ---
 include/hw/virtio/virtio-net.h  | 1 +
 include/hw/virtio/virtio-scsi.h | 1 +
 7 files changed, 12 insertions(+), 9 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v3 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net

[shannon.zhao]

From: Shannon Zhao <shannon.zhao@linaro.org>

Move DEFINE_VIRTIO_NET_FEATURES to the backend virtio-net.
The transports just sync the host features from backend.
Meanwhile move virtio_net_set_config_size to virtio-net.

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

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 59f76bc..8034869 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -446,6 +446,9 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features)
     VirtIONet *n = VIRTIO_NET(vdev);
     NetClientState *nc = qemu_get_queue(n->nic);
 
+    /* Firstly sync all virtio-net possible supported features */
+    features |= n->host_features;
+
     virtio_add_feature(&features, VIRTIO_NET_F_MAC);
 
     if (!peer_has_vnet_hdr(n)) {
@@ -1585,6 +1588,7 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     NetClientState *nc;
     int i;
 
+    virtio_net_set_config_size(n, n->host_features);
     virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
     n->max_queues = MAX(n->nic_conf.peers.queues, 1);
@@ -1721,6 +1725,7 @@ static void virtio_net_instance_init(Object *obj)
 }
 
 static Property virtio_net_properties[] = {
+    DEFINE_VIRTIO_NET_FEATURES(VirtIONet, host_features),
     DEFINE_NIC_PROPERTIES(VirtIONet, nic_conf),
     DEFINE_PROP_UINT32("x-txtimer", VirtIONet, net_conf.txtimer,
                                                TX_TIMER_INTERVAL),
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 047c963..b893e02 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -145,7 +145,6 @@ static void s390_virtio_net_realize(VirtIOS390Device *s390_dev, Error **errp)
     DeviceState *vdev = DEVICE(&dev->vdev);
     Error *err = NULL;
 
-    virtio_net_set_config_size(&dev->vdev, s390_dev->host_features);
     virtio_net_set_netclient_name(&dev->vdev, qdev->id,
                                   object_get_typename(OBJECT(qdev)));
     qdev_set_parent_bus(vdev, BUS(&s390_dev->bus));
@@ -508,7 +507,6 @@ static unsigned virtio_s390_get_features(DeviceState *d)
 
 static Property s390_virtio_net_properties[] = {
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features),
-    DEFINE_VIRTIO_NET_FEATURES(VirtIOS390Device, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d32ecaf..1252162 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -779,7 +779,6 @@ static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
     DeviceState *vdev = DEVICE(&dev->vdev);
     Error *err = NULL;
 
-    virtio_net_set_config_size(&dev->vdev, ccw_dev->host_features[0]);
     virtio_net_set_netclient_name(&dev->vdev, qdev->id,
                                   object_get_typename(OBJECT(qdev)));
     qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
@@ -1403,7 +1402,6 @@ static int virtio_ccw_load_config(DeviceState *d, QEMUFile *f)
 
 static Property virtio_ccw_net_properties[] = {
     DEFINE_PROP_STRING("devno", VirtioCcwDevice, bus_id),
-    DEFINE_VIRTIO_NET_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/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index c7c3f72..c6b99f9 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1360,7 +1360,6 @@ static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
-    DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1370,7 +1369,6 @@ static void virtio_net_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     VirtIONetPCI *dev = VIRTIO_NET_PCI(vpci_dev);
     DeviceState *vdev = DEVICE(&dev->vdev);
 
-    virtio_net_set_config_size(&dev->vdev, vpci_dev->host_features);
     virtio_net_set_netclient_name(&dev->vdev, qdev->id,
                                   object_get_typename(OBJECT(qdev)));
     qdev_set_parent_bus(vdev, BUS(&vpci_dev->bus));
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 4c2fe83..5bee4df 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -68,6 +68,7 @@ typedef struct VirtIONet {
     uint32_t has_vnet_hdr;
     size_t host_hdr_len;
     size_t guest_hdr_len;
+    uint32_t host_features;
     uint8_t has_ufo;
     int mergeable_rx_bufs;
     uint8_t promisc;
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net

[Cornelia Huck]

On Tue, 21 Apr 2015 18:51:10 +0800
shannon.zhao@linaro.org wrote:

I'd drop the leading "hw/net/" from the subject.

> From: Shannon Zhao <shannon.zhao@linaro.org>
> 
> Move DEFINE_VIRTIO_NET_FEATURES to the backend virtio-net.
> The transports just sync the host features from backend.
> Meanwhile move virtio_net_set_config_size to virtio-net.

Could you elaborate on why this makes sense?

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

FWIW, this survives basic sanity checks on virtio-ccw - also in
combination with my realization sequence reordering patches I sent
yesterday.

Haven't gotten around to s390-virtio or virtio-scsi yet.

Re: [Qemu-devel] [PATCH v3 1/2] hw/net/virtio-net: Move DEFINE_VIRTIO_NET_FEATURES to virtio-net

[Shannon Zhao]

On 2015/4/22 22:24, Cornelia Huck wrote:
> On Tue, 21 Apr 2015 18:51:10 +0800
> shannon.zhao@linaro.org wrote:
> 
> I'd drop the leading "hw/net/" from the subject.
> 

Ok, thanks.

>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> Move DEFINE_VIRTIO_NET_FEATURES to the backend virtio-net.
>> The transports just sync the host features from backend.
>> Meanwhile move virtio_net_set_config_size to virtio-net.
> 
> Could you elaborate on why this makes sense?
> 

Ok, will add.

>>
>> Signed-off-by: Shannon Zhao <zhaoshenglong@huawei.com>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> ---
>>  hw/net/virtio-net.c            | 5 +++++
>>  hw/s390x/s390-virtio-bus.c     | 2 --
>>  hw/s390x/virtio-ccw.c          | 2 --
>>  hw/virtio/virtio-pci.c         | 2 --
>>  include/hw/virtio/virtio-net.h | 1 +
>>  5 files changed, 6 insertions(+), 6 deletions(-)
> 
> FWIW, this survives basic sanity checks on virtio-ccw - also in
> combination with my realization sequence reordering patches I sent
> yesterday.
> 
> Haven't gotten around to s390-virtio or virtio-scsi yet.
> 

-- 
Shannon

[Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[shannon.zhao]

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(-)

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 {
-- 
2.1.0

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Shannon Zhao]

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

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Shannon Zhao]

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
> 
> .
> 

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Michael S. Tsirkin]

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
> > 
> > .
> > 
> 

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Shannon Zhao]

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
>>>
>>> .
>>>
>>
> 
> .
> 


-- 
Thanks,
Shannon

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Michael S. Tsirkin]

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

Re: [Qemu-devel] [PATCH v3 2/2] virtio-scsi: Move DEFINE_VIRTIO_SCSI_FEATURES to virtio-scsi

[Peter Maydell]

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