On a Wednesday in 2023, Andrea Bolognani wrote:
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.
You could call it virDomainDefPrePostParse.
Other ideas?
Make the complete separation of XML parsing and post-parsing a problem of future ourselves. Reviewed-by: Ján Tomko <jtomko@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: PGP signature
