| Message ID | 1418304322-7546-20-git-send-email-cornelia.huck@de.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote: > Wire up virtio-blk to provide different feature bit sets depending > on whether legacy or v1.0 has been requested. > > Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> So we need some way for devices to tell transports not to negotiate rev 1. Does clearing VERSION_1 have this effect? > --- > hw/block/virtio-blk.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c > index 9cfae66..fdc236a 100644 > --- a/hw/block/virtio-blk.c > +++ b/hw/block/virtio-blk.c > @@ -587,6 +587,24 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features) > return features; > } > > +static uint64_t virtio_blk_get_features_rev(VirtIODevice *vdev, > + uint64_t features, > + unsigned int revision) > +{ > + if (revision == 0) { > + /* legacy */ > + virtio_clear_feature(&features, VIRTIO_F_VERSION_1); > + return virtio_blk_get_features(vdev, features); > + } > + /* virtio 1.0 or later */ > + virtio_clear_feature(&features, VIRTIO_BLK_F_SCSI); > + virtio_clear_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); > + virtio_clear_feature(&features, VIRTIO_BLK_F_WCE); > + /* we're still missing ANY_LAYOUT */ > + /* virtio_add_feature(&features, VIRTIO_F_VERSION_1); */ > + return features; > +} > + > static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) > { > VirtIOBlock *s = VIRTIO_BLK(vdev); > @@ -821,6 +839,7 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data) > vdc->get_config = virtio_blk_update_config; > vdc->set_config = virtio_blk_set_config; > vdc->get_features = virtio_blk_get_features; > + vdc->get_features_rev = virtio_blk_get_features_rev; > vdc->set_status = virtio_blk_set_status; > vdc->reset = virtio_blk_reset; > vdc->save = virtio_blk_save_device; > -- > 1.7.9.5
On Sun, 28 Dec 2014 12:24:46 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote: > > Wire up virtio-blk to provide different feature bit sets depending > > on whether legacy or v1.0 has been requested. > > > > Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > So we need some way for devices to tell transports > not to negotiate rev 1. > Does clearing VERSION_1 have this effect? > I just noticed that my patch is running in circles here. What we need is probably the transport-dependent maximum revision checker (which at least for ccw is acting on a device) pass in the requested revision and check if the feature bits for the revision include VERSION_1. Does that make sense?
On Wed, Jan 07, 2015 at 05:29:49PM +0100, Cornelia Huck wrote: > On Sun, 28 Dec 2014 12:24:46 +0200 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote: > > > Wire up virtio-blk to provide different feature bit sets depending > > > on whether legacy or v1.0 has been requested. > > > > > > Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. > > > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > > > So we need some way for devices to tell transports > > not to negotiate rev 1. > > Does clearing VERSION_1 have this effect? > > > I just noticed that my patch is running in circles here. > > What we need is probably the transport-dependent maximum revision > checker (which at least for ccw is acting on a device) pass in the > requested revision and check if the feature bits for the revision > include VERSION_1. Does that make sense? Just make devices set 'rev 1 supported' flag?
On Wed, 7 Jan 2015 21:11:44 +0200 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Wed, Jan 07, 2015 at 05:29:49PM +0100, Cornelia Huck wrote: > > On Sun, 28 Dec 2014 12:24:46 +0200 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Thu, Dec 11, 2014 at 02:25:21PM +0100, Cornelia Huck wrote: > > > > Wire up virtio-blk to provide different feature bit sets depending > > > > on whether legacy or v1.0 has been requested. > > > > > > > > Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. > > > > > > > > Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> > > > > > > So we need some way for devices to tell transports > > > not to negotiate rev 1. > > > Does clearing VERSION_1 have this effect? > > > > > I just noticed that my patch is running in circles here. > > > > What we need is probably the transport-dependent maximum revision > > checker (which at least for ccw is acting on a device) pass in the > > requested revision and check if the feature bits for the revision > > include VERSION_1. Does that make sense? > > Just make devices set 'rev 1 supported' flag? I'm now using the ->get_features() callback to check for VERSION_1 (assuming every device that supports it adds the bit in its callback) and only allow rev 1 if it is present. Will play with this a bit as well.
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9cfae66..fdc236a 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -587,6 +587,24 @@ static uint64_t virtio_blk_get_features(VirtIODevice *vdev, uint64_t features) return features; } +static uint64_t virtio_blk_get_features_rev(VirtIODevice *vdev, + uint64_t features, + unsigned int revision) +{ + if (revision == 0) { + /* legacy */ + virtio_clear_feature(&features, VIRTIO_F_VERSION_1); + return virtio_blk_get_features(vdev, features); + } + /* virtio 1.0 or later */ + virtio_clear_feature(&features, VIRTIO_BLK_F_SCSI); + virtio_clear_feature(&features, VIRTIO_BLK_F_CONFIG_WCE); + virtio_clear_feature(&features, VIRTIO_BLK_F_WCE); + /* we're still missing ANY_LAYOUT */ + /* virtio_add_feature(&features, VIRTIO_F_VERSION_1); */ + return features; +} + static void virtio_blk_set_status(VirtIODevice *vdev, uint8_t status) { VirtIOBlock *s = VIRTIO_BLK(vdev); @@ -821,6 +839,7 @@ static void virtio_blk_class_init(ObjectClass *klass, void *data) vdc->get_config = virtio_blk_update_config; vdc->set_config = virtio_blk_set_config; vdc->get_features = virtio_blk_get_features; + vdc->get_features_rev = virtio_blk_get_features_rev; vdc->set_status = virtio_blk_set_status; vdc->reset = virtio_blk_reset; vdc->save = virtio_blk_save_device;
Wire up virtio-blk to provide different feature bit sets depending on whether legacy or v1.0 has been requested. Note that VERSION_1 is still disabled due to missing ANY_LAYOUT support. Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com> --- hw/block/virtio-blk.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)