Linux Virtualization - Apr 2015 - [PATCH 2/2] virtio-balloon: virtio 1 support

On Mon, 13 Apr 2015 13:26:31 +0200
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> On Mon, Apr 13, 2015 at 10:02:58AM +0200, Cornelia Huck wrote:
> > Also, doesn't get_features need to be modified as well so that
> > VERSION_1 is advertised?
> 
> virtio_pci_device_plugged seems to set it ATM. I'll re-test to confirm.
Indeed, pci sets it, but ccw does not. And virtio-net, for example,
sets it explicitly as well. We need to unify this :)

On Mon, Apr 13, 2015 at 01:35:21PM +0200, Cornelia Huck
wrote:
> On Mon, 13 Apr 2015 13:26:31 +0200
> "Michael S. Tsirkin" <mst at redhat.com> wrote:
> 
> > On Mon, Apr 13, 2015 at 10:02:58AM +0200, Cornelia Huck wrote:
> 
> > > Also, doesn't get_features need to be modified as well so
that
> > > VERSION_1 is advertised?
> > 
> > virtio_pci_device_plugged seems to set it ATM. I'll re-test to
confirm.
> 
> Indeed, pci sets it, but ccw does not. And virtio-net, for example,
> sets it explicitly as well. We need to unify this :)
I'm inclined to set it in transport and black-list in specific devices.

-- 
MST

On Mon, 13 Apr 2015 13:49:42 +0200
"Michael S. Tsirkin" <mst at redhat.com> wrote:
> On Mon, Apr 13, 2015 at 01:35:21PM +0200, Cornelia Huck wrote:
> > On Mon, 13 Apr 2015 13:26:31 +0200
> > "Michael S. Tsirkin" <mst at redhat.com> wrote:
> > 
> > > On Mon, Apr 13, 2015 at 10:02:58AM +0200, Cornelia Huck wrote:
> > 
> > > > Also, doesn't get_features need to be modified as well
so that
> > > > VERSION_1 is advertised?
> > > 
> > > virtio_pci_device_plugged seems to set it ATM. I'll re-test
to confirm.
> > 
> > Indeed, pci sets it, but ccw does not. And virtio-net, for example,
> > sets it explicitly as well. We need to unify this :)
> 
> I'm inclined to set it in transport and black-list in specific devices.
> 
Agreed, let's set it in the transports.

For ccw, I'd probably only want to offer it once the driver has
negotiated a revision >= 1, so something like the following (untested):

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 8b6b2ab..4d8cc24 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -703,6 +703,9 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw)
         dev->host_features             
virtio_bus_get_vdev_features_rev(&dev->bus, dev->host_features,
                                              dev->revision >= 1 ? 1 : 0);
+        if (dev->revision >= 1) {
+            virtio_add_feature(&dev->host_features, VIRTIO_F_VERSION_1);
+        }
         break;
     default:
         ret = -ENOSYS;