On 3/31/26 14:57, Peter Krempa wrote: > On Thu, Mar 26, 2026 at 12:51:23 +0100, Michal Privoznik via Devel wrote: >> From: Michal Privoznik <[email protected]> >> >> Prior to v7.10.0-rc1~26 seclabels defaulted to > > Given that it's ~5 years since I broke this ... > >> VIR_DOMAIN_SECLABEL_DYNAMIC (type='dynamic'). But after switching >> the parser to virXMLPropEnum() the type is overwritten to >> VIR_DOMAIN_SECLABEL_DEFAULT because the first thing that the >> helper function does is to set variable that holds the result to >> zero. Switch to virXMLPropEnumDefault() to restore the previous >> behavior. > > .... I wonder what the implications actually are (I didn't check). The > commit message explains what happened w.r.t the behaviour of > virXMLPropEnum() but don't detail what the effect of the change was so I > wonder why we didn't see any problem with this or why we now want to > change it back besides "it was like that before". > > Did you happen to figure out what the change actually impacted? >
Yeah, I was thinking about that too. But I think we're on the safe side because of extensive checks the function does. I mean, the moment you want relabel='yes' you also need either type='static' or type='dynamic'. These then get properly formatted into saved domain XML. There's only one corner case (through which I encountered this bug). In the original bug report, Rich specified "<seclabel model="selinux" relabel="no"/>". I did not realize it was disk's seclabel and thought it's the top level domain seclabel. Now, our RNG doesn't consider this valid, nevertheless, ignoring XML validation error, I've realized the seclabel was parsed and stored in virDomainDef, yet not formatted back (thanks to short circuit at the very beginning of virSecurityLabelDefFormat()). Long story short, the only case in which we can have "troubles" is when users do NOT want us to use certain secdriver AND they pass incomplete XML that our RNG rejects. Michal
