On Mon, Nov 21, 2016 at 03:40:23PM +0100, Christian Ehrhardt wrote: > When virt-aa-helper parses xml content it can fail on security labels. > > It fails by requiring to parse active domain content on seclabels that > is not yet filled in. > > Testcase with virt-aa-helper on a minimal xml: > $ cat << EOF > /tmp/test.xml > <domain type='kvm'> > <name>test-seclabel</name> > <uuid>12345678-9abc-def1-2345-6789abcdef00</uuid> > <memory unit='KiB'>1</memory> > <os><type arch='x86_64'>hvm</type></os> > <seclabel type='dynamic' model='apparmor' relabel='yes'/> > <seclabel type='dynamic' model='dac' relabel='yes'/> > </domain> > EOF > $ /usr/lib/libvirt/virt-aa-helper -d -r -p 0 \ > -u libvirt-12345678-9abc-def1-2345-6789abcdef00 < /tmp/test.xml > > Current Result: > virt-aa-helper: error: could not parse XML > virt-aa-helper: error: could not get VM definition > Expected Result is a valid apparmor profile > > Updates: > v2 > - simplified and clarified commit message > - make the flag skip all secabel parsing > - shorten the new flag name > > fixes: dfbc9a83 ("apparmor: QEMU monitor socket moved") > > Signed-off-by: Christian Ehrhardt <christian.ehrhardt@xxxxxxxxxxxxx> > --- > src/conf/domain_conf.c | 6 ++++-- > src/conf/domain_conf.h | 2 ++ > src/security/virt-aa-helper.c | 1 + > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index 6e008e2..bbe550b 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -16289,8 +16289,10 @@ virDomainDefParseXML(xmlDocPtr xml, > > /* analysis of security label, done early even though we format it > * late, so devices can refer to this for defaults */ > - if (virSecurityLabelDefsParseXML(def, ctxt, caps, flags) == -1) > - goto error; > + if (!(flags & VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL)) { n> + if (virSecurityLabelDefsParseXML(def, ctxt, caps, flags) == -1) > + goto error; > + } > > /* Extract domain memory */ > if (virDomainParseMemory("./memory[1]", NULL, ctxt, > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h > index 541b600..3a9693c 100644 > --- a/src/conf/domain_conf.h > +++ b/src/conf/domain_conf.h > @@ -2666,6 +2666,8 @@ typedef enum { > VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 9, > /* skip definition validation checks meant to be executed on define time only */ > VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE = 1 << 10, > + /* skip parsing of security labels */ > + VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL = 1 << 11, > } virDomainDefParseFlags; > > typedef enum { > diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c > index 77eeaff..5f5d1cd 100644 > --- a/src/security/virt-aa-helper.c > +++ b/src/security/virt-aa-helper.c > @@ -705,6 +705,7 @@ get_definition(vahControl * ctl, const char *xmlStr) > > ctl->def = virDomainDefParseString(xmlStr, > ctl->caps, ctl->xmlopt, NULL, > + VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL | > VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); > > if (ctl->def == NULL) { > -- > 2.7.4 > This looks good to me (and looks more terse now) . It would be great to have another review though. Cheers, -- Guido -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list