Re: [Qemu-devel] [PATCH 2/5] virtio-blk: disable scsi passthrough for 1.0 device

["Michael S. Tsirkin" <mst@redhat.com>]

On Wed, Jul 15, 2015 at 05:38:53PM +0200, Cornelia Huck wrote:
> On Wed, 15 Jul 2015 17:39:18 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Jul 15, 2015 at 04:30:51PM +0200, Cornelia Huck wrote:
> > > On Wed, 15 Jul 2015 17:11:57 +0300
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > > > > > Fine, but revision is negotiated way before features are
> > > > > > > > probed so why does it make a practical difference?
> > > > > > > 
> > > > > > > Legacy drivers (that don't know about the set-revision command) will
> > > > > > > read features without revision negotiation - we need to offer them the
> > > > > > > legacy feature set.
> > > > > > 
> > > > > > Right. So simply do if (revision < 1) return features & 0xffffffff
> > > > > > and that will do this, will it not?
> > > > > 
> > > > > Not for bits that we want to offer for legacy but not for modern.
> > > > 
> > > > I don't think this selective offering works at least for scsi.
> > > > scsi is a backend feature, if you connect a modern device
> > > > in front the device simply does not work.
> > > > It therefore makes no sense to attach a transitional device
> > > > to such a backend.
> > > 
> > > My point is that we're losing legacy features with that approach, and
> > > it would not be possible to offer them to legacy guests with newer
> > > qemus (at least with ccw).
> > 
> > What's wrong with adding a disable-modern flag, like pci has?
> > User can set that to get a legacy device.
> 
> The whole idea behind the revision-stuff was that we don't need
> something like disable-modern. If the device is able to figure out on
> its own if it is to act as a modern or a legacy device, why require
> user intervention?

It's about compatibility, e.g. being able to test legacy mode
in transitional drivers in guests.
Consider also bugs, e.g. the fact that linux guests lack WCE
in modern mode ATM.

> > 
> > > What about the other way around (i.e. scsi is configured, therefore the
> > > device is legacy-only)? We'd only retain the scsi bit if it is actually
> > > wanted by the user's configuration. I would need to enforce a max
> > > revision of 0 for such a device in ccw, and pci could disable modern
> > > for it.
> > 
> > Will have to think about it.
> > But I think a flag to disable/enable modern is useful in any case,
> > and it seems sufficient.
> 
> I don't like the idea of disabling modern or legacy for ccw, where the
> differences between both are very minor.
> 
> I also don't think requiring the user to specify a new flag on upgrade
> just to present the same features as before is a good idea: it is
> something that is easily missed and may lead to much headscratching.

And doing this on a driver upgrade won't?  As I said, if you believe
this feature has value, argue that we shouldn't drop scsi off in virtio
1.0 then.

-- 
MST