Re: [libvirt] [PATCH 03/11] qemu: Pass QEMUCaps to virDomainDefPostParse
On Fri, Sep 28, 2018 at 22:02:59 -0400, John Ferlan wrote:
On 9/20/18 1:44 PM, Marc Hartmayer wrote:
> ...although priv->qemuCaps will be NULL in almost every case when the
> post parse callback has failed. That may change in the future.
>
> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
> ---
> src/qemu/qemu_process.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 44c63c42d618..6c5a6472d8cd 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -5282,7 +5282,7 @@ qemuProcessInit(virQEMUDriverPtr driver,
The comment just above here could use a tweak for grammar ;-):
/* in case when the post parse callback failed we need to re-run it
on the
* old config prior we start the VM */
> if (vm->def->postParseFailed) {
> VIR_DEBUG("re-running the post parse callback");
>
> - if (virDomainDefPostParse(vm->def, caps, 0, driver->xmlopt, NULL)
< 0)
> + if (virDomainDefPostParse(vm->def, caps, 0, driver->xmlopt,
priv->qemuCaps) < 0)
Searching through history of this line finds Peter's original commit in
this area - 7726d158, which seems to indicate a very specific reason for
providing a NULL capabilities value here.
I think from this patch on is something Peter has worked on a lot, so I
would prefer to defer to Peter on them since I'm sure he understands all
the various PostParse and Validate special conditions and various flags
usage better than I do.
Thanks for notifying me.
In general, when parsing a "new" domain config as it's the case here,
NULL should be passed in.
The qemu driver registers the 'domainPostParseDataAlloc' callback to
qemuDomainPostParseDataAlloc. This callback is executed after the
'basic' post parse callback which fills in the emulator binary.
Passing an explicit qemuCaps is meant only for special cases e.g.
migration where we have a specific set of capabilities which need to be
used rather than a new one.
This patch does not seem to make sense with the justification provided
here as the post-parse callbacks fill in the proper caps here and this
part clearly uses a new domain configuration, so the default behaviour
should be used.
Changing this would also need that we change the normal parser path to
achieve the same results which is impossible as you don't have access to
priv->qemuCaps prior to creating the virDomainObj object.