[libvirt PATCH 2/2] conf: Fix migration in some firmware autoselection scenarios

[Ján Tomko]

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 at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230412/ec69ea00/attachment.sig>