[libvirt PATCH] conf: remove duplicated firmware type attribute

[Daniel P. Berrangé]

On Mon, Mar 29, 2021 at 08:20:32PM +0200, Pavel Hrdina wrote:
> On Mon, Mar 29, 2021 at 07:03:52PM +0100, Daniel P. Berrangé wrote:
> > The
> > 
> >   <os firmware='efi'>
> >     <firmware type='efi'>
> >       <feature enabled='no' name='enrolled-keys'/>
> >     </firmware>
> >   </os>
> > 
> > repeats the firmware attribute twice. This has no functional benefit, as
> > evidenced by fact that we use a single struct field to store both
> > attributes, while needlessly introducing an error scenario. The XML can
> > just be simplified to:
> > 
> >   <os firmware='efi'>
> >     <firmware>
> >       <feature enabled='no' name='enrolled-keys'/>
> >     </firmware>
> >   </os>
> > 
> > which also means that we don't need to emit the empty element
> > <firmware type='efi'/> for all existing configs too.
> 
> My original motivation was that if we ever need to introduce another
> attribute to the <firmware> element it would be nicely grouped together.
> But I guess it wound not be a big deal if we would have:
> 
>     <os firmware='efi'>
>       <firmware someAttr='value'>
>         <feature enabled='no' name='enrolled-keys'/>
>       </firmware>
>     </os>
> 
> This would look reasonable as well so

This is pretty much the situation we're already in for a number of existing
elements too. For example <disk> has type=file|block|network on the top
level, but then the variable content is under the child <source> element.

If we didnt already have @firmware on the <os> element it would make sense
to have @type on <firmware>, but given existing schema, avoiding duplicating
information is better.

> Reviewed-by: Pavel Hrdina <phrdina at redhat.com>



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|