Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Michael S. Tsirkin]

On Mon, Aug 08, 2016 at 06:27:15AM +0000, Zeng, Xin wrote:
> On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:address@hidden
> > Sent: Friday, August 05, 2016 3:56 AM
> > To: Gonglei (Arei)
> > Cc: Zeng, Xin; address@hidden; address@hidden;
> > Huangpeng (Peter); Luonengjun; address@hidden;
> > address@hidden; address@hidden; Jani Kokkonen;
> > address@hidden; address@hidden; Keating, Brian A; Ma,
> > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> > 
> > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > +in future.It's currently defined for the version field:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > >
> > > > Suggest to remove this macro,
> > > > Do you think a version which is composed of major version and minor
> > > > version is better?
> > > >
> > >
> > > I think we should tell the developer how to set the value of version
> > > field, but I have no idea about which value or form is better, so I
> > > used 1 as the first version. What's your opinion?
> > 
> > My opinion is that you should drop this completely. We do feature bits, not
> > version numbers in virtio. We do not want each device doing its own thing 
> > for
> > compatibility.
> > 
> 
> But as I mentioned before,  considering the bug fix case, if each backend 
> device 
> release need a feature bit meaning "some bugs are fixed", are the feature bits
> enough? 

It depends on whether the bug is very entrenched and important. In most
cases, for minor bugs, it's better to just fix the bug in the driver or
the hypervisor and be done with it.  For cases where
that's not feasible because many drivers relied on a specific bug,
and the bug is very important, we can always add more
if we run out of feature bits.

> Physical devices usually have a revision ID to mark its version,

Because compatibility is one way (new devices usually need
new drivers).

> could we have a
> revision Id field for each virtio device to distinguish the the virtio 
> devices which 
> have the same feature sets but have different version? 

ccw has version negotiation. It was only changed once at the 0.9->1.0
transition so far, it's not used for bug fixes.  We could discuss adding
this to pci and mmio as well, but if yes this should be discussed
separately.

So far no argument made here was crypto specific, so
let's not put this in the crypto device.


> > --
> > MST