Re: [PATCHv3 04/11] Add virtio revision attribute to controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 11 Aug 2016 16:17:10 +0200
Ján Tomko <jtomko@xxxxxxxxxx> wrote:

> On Thu, Aug 11, 2016 at 02:31:55PM +0100, Daniel P. Berrange wrote:
> >On Thu, Aug 11, 2016 at 03:25:53PM +0200, Ján Tomko wrote:
> >> On Thu, Aug 11, 2016 at 01:00:08PM +0100, Daniel P. Berrange wrote:
> >> > On Wed, Aug 10, 2016 at 03:27:15PM +0200, Ján Tomko wrote:
> >> > > <controller type='scsi' index='0' model='virtio-scsi'>
> >> > >     <virtio revision='0.9'/>
> >> >
> >> > I'm wondering about generalizing this. eg what if there are
> >> > other device models where we want the ability to set a
> >> > revision. We don't really want to invent a new sub-elment
> >> > named after each device model
> >>
> >> Not even a new attribute? :)
> >>  <revision virtio='0.9'/>
> >>
> >> How about:
> >>  <revision type='virtio' version='0.9'/>
> >
> >Both of those are quite repetative - we already know its virtio.
> >
> 
> I guess one device having <revision>s of different types is unlikely.
> 
> >Most devices we have alrady include a <driver> or <model> sub-element,
> >so we should really just add a revision= attrbute to those existing
> 
> What I liked about having it as a separate element is that it can be
> repeated, e.g.:
>   <revision type='virtio' version='0.9'/>
>   <revision type='virtio' version='1.0'/>
> 
> for a device with both 0.9 and 1.0.
> 
> I could not come up with a nice way to represent that in a single
> attribute:
> * '0.9+1.0' feels like the two values should rather be separated
>   at the XML level
> * 'all' will not be true if there happens to be another virtio
>   revision in the future

[not a libvirt developer, but let me comment from the qemu virtio
perspective]

I don't think you are expressing the concept of virtio (standard)
revisions (more like releases!) here correctly. Let me elaborate:

- The disable-legacy/disable-modern attributes are virtio-pci only.
Moreover, they don't express 'compliant to virtio-1.0' or so: They do
exactly What It Says On The Tin. A device with both disable attributes
off is in fact virtio-1.0 compliant (transitional devices are
compliant), as is a device with disable-legacy off. But it might also
be virtio-1.1 compliant! (That's the most likely release of the
standard in the near future.)

- virtio-ccw does not have the concept of these disable switches.
Instead, there are virtio-ccw specific 'revisions' which count upwards
and may be limited by the 'max_revision' attribute. However, this is
not an attribute that is supposed to be set by the user, but for
backwards compatibility only. Unlike pci, ccw has nothing to gain by
disabling legacy support.

- We may actually want to add some kind of versioning scheme to virtio
devices in future versions of the standard. But that's just a very
vague idea right now.

Am I right in assuming that you simply want to be able to control
whether your virtio-pci devices are legacy, transitional or modern?
Then I think you'd be best off adding these as virtio-pci attributes
only and leave the concept of a 'virtio revision' for the future when
we might introduce it in the standard.


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]