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

On Thu, Aug 11, 2016 at 10:17:34AM -0400, Laine Stump wrote: > On 08/11/2016 09:31 AM, 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. >> >> Most devices we have alrady include a <driver> or <model> sub-element, >> so we should really just add a revision= attrbute to those existing >> elements. For places which don't already have this we can add a new >> <driver> element > > Yeah, I was going to suggest <driver> as well. > > The only thing missing would be the ability to specify multiple > versions. > I think that's something we want to allow - didn't you mention somewhere that 'disable-legacy=off,disable-modern=off' gets different results than omitting it completely?

Other than awkward enum values, it could be represented as a (rather verbose) subelement: <driver> <revision>0.9</revision> <revision>1.0</revision>

</driver> > Also, it does make the parsing and formatting a bit awkward, since > <driver> has different parse/format functions for each device that uses > it (necessary due to the difference in possible attributes). That means more work for me, but should not be a factor when deciding on better-looking XML.

> > In spite of that, it would definitely look better from the outside. > Personally I liked the consistence of having the same element under each device more (and the ability to have more of them), but putting it under <driver>/<model> also has its beauty, if we can figure out how to express multiple versions.