On Wed, Apr 12, 2023 at 04:52:16AM -0700, Andrea Bolognani wrote:
> On Wed, Apr 12, 2023 at 12:55:51PM +0200, Ján Tomko wrote:
> > On a Tuesday in 2023, Andrea Bolognani wrote:
> > > static int
> > > virDomainDefParseBootFirmwareOptions(virDomainDef *def,
> > > - xmlXPathContextPtr ctxt)
> > > + xmlXPathContextPtr ctxt,
> > > + unsigned int flags)
> > > {
> > > g_autofree char *firmware = virXPathString("string(./os/@firmware)", ctxt);
> > > g_autofree xmlNodePtr *nodes = NULL;
> > > g_autofree int *features = NULL;
> > > + bool abiUpdate = !!(flags & VIR_DOMAIN_DEF_PARSE_ABI_UPDATE);
> >
> > The flag is documented as:
> > /* allow updates in post parse callback that would break ABI otherwise */
> > VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 7,
> >
> > and I also think that this is something that better belongs in
> > post-parse.
>
> Okay, so the idea would be to keep picking up the firmware features
> here and possibly drop them from the DomainDef during the PostParse
> phase? I think that could work too. Let me give it a try.
This doesn't seem to work as smoothly as I hoped it would :(
The problem is that, if the kludge is performed in PostParse, the
call tree ends up looking like
virDomainDefPostParse()
qemuDomainDefPostParse()
qemuFirmwareFillDomain() <- firmware selection
virDomainDefOSValidate()
virDomainDefPostParseCommon()
virDomainDefPostParseOs() <- migration kludge
and firmware selection fails, because the configuration it sees is
the original one, which doesn't pass validation.
I could take virDomainDefPostParseOs() out of
virDomainDefPostParseCommon() and ensure it gets called before the
driver-specific PostParse callback, but to be honest I'm not
convinced making the kludge even more invasive would represent an
improvement.
Other ideas?
--
Andrea Bolognani / Red Hat / Virtualization
