[libvirt] [PATCH] virDomainDefCopy: Skip ostype checks

[Ján Tomko]

On Sat, Jun 02, 2018 at 12:57:39PM +0200, Michal Privoznik wrote:
>When parsing domain XML the virCapsDomainData lookup is performed
>in order to fill in missing def->os.arch and def->os.machine
>strings. Well, when doing copy of already existing virDomainDef
>we don't want any automagic fill in of defaults (and those two
>strings are going to be provided at this point anyway by first
>parse of the domain XML).

The sentence in parentheses documents an inefficiency -
If we have machine and arch, the DomainDataLookup call is pointless,
unless we want to validate it.

>
>What is even worse is that we do not look up capabilities for
>parsed emulator path rather than some generic capabilities for

s/than//

>parsed arch. Therefore, if emulator points to qemu under
>non-default path (say $HOME/qemu-system-arm) but there's no such
>qemu under the default path (say /usr/bin/qemu-system-arm) the
>capabilities lookup fails and creating the copy is denied.
>

This is downright a bug - we should be using the specified emulator
path to fill the machine type even for new domains. Are you able
to define a new domain without having a default emulator for it?

>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
> src/conf/domain_conf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>index 86814d5f64..f36a1bfe79 100644
>--- a/src/conf/domain_conf.c
>+++ b/src/conf/domain_conf.c
>@@ -28397,7 +28397,8 @@ virDomainDefCopy(virDomainDefPtr src,
>     virDomainDefPtr ret;
>     unsigned int format_flags = VIR_DOMAIN_DEF_FORMAT_SECURE;
>     unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
>-                               VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;
>+                               VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE |
>+                               VIR_DOMAIN_DEF_PARSE_SKIP_OSTYPE_CHECKS;

For missing QEMU emulator, we postpone all the stuff depending
on qemuCaps until domain startup:
commit 7726d1581f9e433a106f45ed87ec95ece575f817
    qemu: Implement postParse callback skipping on config reload

Ideally, we'd remove this flag, move the automagic to PostParse
and make it non-fatal in the same way, but I don't know how much would
that break.

Practically, this hunk is an improvement. Hoping you'll address the
newly-defined domain bug as well:
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: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180602/08a4ea7a/attachment-0001.sig>