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

On Fri, Aug 12, 2016 at 10:47:13AM +0200, Cornelia Huck wrote: > On Thu, 11 Aug 2016 16:17:10 +0200 > Ján Tomko <jtomko(a)redhat.com&gt; 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? Yes. > 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. > So XML like this: <model legacy='on/off' modern='on/off'/> or <model compatibility='legacy/transitional/modern'/> (which could possibly be reused for other hypervisors with a similar concept, not just QEMU and virtio)

was what we needed all along, but I misunderstood their purpose. That's good to know :) Jan -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list