Comment 15 for bug 1633207

TL;DR:
- a dac sec label is parsed
- it has no label, but due to a bug it searches one
- label can't be found for an inactive domain
- exit with Error
- expected fix is reverting part of dfbc9a83

Debug-Analysis:

Interesting part of the call chain:
get_definition -> virDomainDefParseString -> virDomainDefParse -> virDomainDefParseNode -> virDomainDefParseXML -> virSecurityLabelDefsParseXML -> virSecurityLabelDefParseXML

Compiled -O0 -g to see more to see where it is failing.
The code itself (of that failing function) didn't change since 1.3.1 (Xenial).

gdb ~/libvirt-2.1.0/debian/tmp/usr/lib/libvirt/virt-aa-helper
set env LD_LIBRARY_PATH /home/ubuntu/libvirt-2.1.0/debian/tmp/usr/lib/x86_64-linux-gnu/
set solib-search-path /home/ubuntu/libvirt-2.1.0/debian/tmp/usr/lib/x86_64-linux-gnu/
b virSecurityLabelDefsParseXML
run -d -r -p 0 -u libvirt-6e082f89-902c-413c-9d9e-f609089d3374 < yakkety-sec-dac.xml

virSecurityLabelDefParseXML (ctxt=0x5555557ddaf0, flags=1024) at ../../../src/conf/domain_conf.c:6384
n (number of labels) is 1
single def parse in virSecurityLabelDefParseXML
1. type dynamic = VIR_DOMAIN_SECLABEL_DYNAMIC
2. relabel yes
3-5 useless if/jumps
6. fails at parsing the actual label
   it doesn't find a label, but thinks it needs one
   check:
   6.1 seclabel->type == VIR_DOMAIN_SECLABEL_STATIC => it is not
   6.2 !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && => true
   6.3 seclabel->type != VIR_DOMAIN_SECLABEL_NONE => true
=> There is no label for the currently off machine, so it fails to find one and goes to error path

The function does right, but the flags suggest it would be alive.
Definiton:
    /* Parse only parts of the XML that would be present in an inactive libvirt
     * XML. Note that the flag does not imply that ABI incompatible
     * transformations can be used, since it's used to strip runtime info when
     * restoring save images/migration. */
    VIR_DOMAIN_DEF_PARSE_INACTIVE = 1 << 1,

The flag comes from the first in the call chain "get_definition"
   ctl->def = virDomainDefParseString(xmlStr, ctl->caps, ctl->xmlopt, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);

That exactly is a diff of the Ubuntu versions on that call:
     ctl->def = virDomainDefParseString(xmlStr,
                                        ctl->caps, ctl->xmlopt,
- VIR_DOMAIN_DEF_PARSE_INACTIVE);
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);

Almost all other changes do OR it in:
- int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE;
+ int domainflags = VIR_DOMAIN_DEF_PARSE_INACTIVE |
+ VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE;

Check upstream for the reasons:

commit b394af162a3871575d9f9c28f72331f198aafa25
Author: Peter Krempa <email address hidden>
Date: Thu May 26 15:58:53 2016 +0200
    conf: Add infrastructure for adding configuration validation

On the critical place before there was a 0, so setting fix was like ORing in.
Why was there a 0 and not VIR_DOMAIN_DEF_PARSE_INACTIVE like in the past?

That was the reason there was a 0 before b394af16 came in:
commit dfbc9a8382adc0495bf0e034ae6add92bed4822b
Author: Guido Günther <email address hidden>
Date: Sat Apr 2 12:49:28 2016 +0200
    apparmor: QEMU monitor socket moved

That changed the call from VIR_DOMAIN_DEF_PARSE_INACTIVE to 0 for issues starting with apparmor but provides no further detail.

The patch to fix would be as easy as:
--- libvirt-2.1.0.orig/src/security/virt-aa-helper.c
+++ libvirt-2.1.0/src/security/virt-aa-helper.c
@@ -708,6 +708,7 @@ get_definition(vahControl * ctl, const c

     ctl->def = virDomainDefParseString(xmlStr,
                                        ctl->caps, ctl->xmlopt,
+ VIR_DOMAIN_DEF_PARSE_INACTIVE |
                                        VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE);

     if (ctl->def == NULL) {

I checked some related cases on apparmor instead of dac labels:
- if dumpxml runs on an running instance with apparmor labels it adds the label to the output, so next load works as it can find it
- if a uuid is not yet defined it creates new labels and works
- if a uuid is defined, but no lavel in xml aa-helper fails on apparmor seclabels with the same issue (fixed by the same patch)