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

[Michael S. Tsirkin]

On Thu, Jul 16, 2015 at 02:37:12PM +0200, Cornelia Huck wrote:
> On Wed, 15 Jul 2015 21:51:32 +0300
> "Michael S. Tsirkin" <address@hidden> wrote:
> 
> > 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" <address@hidden> 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" <address@hidden> 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.
> 
> Let's step back a bit. Why do we need legacy? To support old hosts and
> old guests. So what we need is a test matrix combining old/modern hosts
> with old/modern guests. I don't think forcing to an old version is
> something we should spend time on except during development.
> 
> > Consider also bugs, e.g. the fact that linux guests lack WCE
> > in modern mode ATM.
> 
> If we consider this a bug, shouldn't VERSION_1 be forced off
> unconditionally until specs and codebase are fixed?

I don't see why.  We'll never fix all bugs.  It causes a performance
regression but otherwise things work correctly.
modern is off by default for now, this seems sufficient, if we
never even ship it as an option we'll never find all bugs.

> > 
> > > > 
> > > > > 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.
> 
> I seem to have problems explaining myself :(
> 
> I don't care about the feature, except for supporting old guests that
> should continue working as before even if the host updated. And without
> special configuration on the host, as this is too easily forgotten.
> What else is legacy good for, other than providing backwards
> compatibility?

Maybe it's me having trouble explaining.

There are two kind of blk devices:

- those that allow scsi passthrough
        Such devices can't work properly through the modern interface.
        We could add a modern interface but device can not operate through it.

- those that don't allow scsi passthrough
        Such devices can't work properly through the modern interface.
        We could add a modern interface and we'll get a transitional
        device.


I think for 2.4 it's a good idea to avoid enabling modern interface
by default. Therefore, for 2.4 we can keep scsi enabled unless modern
is requested by user. I am also fine with just doing

        if (modern && scsi)
                exit;


-- 
MST