Re: [libvirt] [PATCH] seclabel: make code and RNG match

of file while reading data: Input/output error

Hmm, this patch does not seem to fix my issue. See the both xml files in the attachment. On Sat, Feb 4, 2012 at 4:10 PM, Eric Blake <eblake(a)redhat.com&gt; wrote: > Commit b170eb99 introduced a bug: domains that had an explicit > <seclabel type='none'/> when started would not be reparsed if > libvirtd restarted. It turns out that our testsuite was not > exercising this because it never tried anything but inactive > parsing. Additionally, the live XML for such a domain failed > to re-validate. Applying just the tests/ portion of this patch > will expose the bugs that are fixed by the other two files. > > * docs/schemas/domaincommon.rng (seclabel): Allow relabel under > type='none'. > * src/conf/domain_conf.c (virSecurityLabelDefParseXML): Per RNG, > presence of <seclabel> with no type implies dynamic. Don't > require sub-elements for type='none'. > * tests/qemuxml2xmltest.c (mymain): Add test. > * tests/qemuxml2argvtest.c (mymain): Likewise. > * tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml: Add file. > * tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args: Add file. > Reported by Ansis Atteka. > --- > docs/schemas/domaincommon.rng | 6 +++ > src/conf/domain_conf.c | 40 > +++++++++----------- > .../qemuxml2argv-seclabel-none.args | 4 ++ > .../qemuxml2argv-seclabel-none.xml | 26 +++++++++++++ > tests/qemuxml2argvtest.c | 1 + > tests/qemuxml2xmltest.c | 29 +++++++++----- > 6 files changed, 74 insertions(+), 32 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 8111045..724d7d0 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -130,9 +130,15 @@ > </interleave> > </group> > <group> > + <!-- with none, relabel must be no if present --> > <attribute name='type'> > <value>none</value> > </attribute> > + <optional> > + <attribute name='relabel'> > + <value>no</value> > + </attribute> > + </optional> > </group> > </choice> > </element> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c > index aa4b32d..6949ece 100644 > --- a/src/conf/domain_conf.c > +++ b/src/conf/domain_conf.c > @@ -2583,17 +2583,15 @@ > virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, > p = virXPathStringLimit("string(./seclabel/@type)", > VIR_SECURITY_LABEL_BUFLEN-1, ctxt); > if (p == NULL) { > - virDomainReportError(VIR_ERR_XML_ERROR, > - "%s", _("missing security type")); > - goto error; > - } > - > - def->type = virDomainSeclabelTypeFromString(p); > - VIR_FREE(p); > - if (def->type <= 0) { > - virDomainReportError(VIR_ERR_XML_ERROR, > - "%s", _("invalid security type")); > - goto error; > + def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; > + } else { > + def->type = virDomainSeclabelTypeFromString(p); > + VIR_FREE(p); > + if (def->type <= 0) { > + virDomainReportError(VIR_ERR_XML_ERROR, > + "%s", _("invalid security type")); > + goto error; > + } > } > > p = virXPathStringLimit("string(./seclabel/@relabel)", > @@ -2634,7 +2632,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr > def, > * if the 'live' VM XML is requested > */ > if (def->type == VIR_DOMAIN_SECLABEL_STATIC || > - !(flags & VIR_DOMAIN_XML_INACTIVE)) { > + (!(flags & VIR_DOMAIN_XML_INACTIVE) && > + def->type != VIR_DOMAIN_SECLABEL_NONE)) { > p = virXPathStringLimit("string(./seclabel/label[1])", > VIR_SECURITY_LABEL_BUFLEN-1, ctxt); > if (p == NULL) { > @@ -2648,7 +2647,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr > def, > > /* Only parse imagelabel, if requested live XML with relabeling */ > if (!def->norelabel && > - !(flags & VIR_DOMAIN_XML_INACTIVE)) { > + (!(flags & VIR_DOMAIN_XML_INACTIVE) && > + def->type != VIR_DOMAIN_SECLABEL_NONE)) { > p = virXPathStringLimit("string(./seclabel/imagelabel[1])", > VIR_SECURITY_LABEL_BUFLEN-1, ctxt); > if (p == NULL) { > @@ -2659,16 +2659,11 @@ > virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, > def->imagelabel = p; > } > > - /* Only parse baselabel, for dynamic or none label types */ > - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC || > - def->type == VIR_DOMAIN_SECLABEL_NONE) { > + /* Only parse baselabel for dynamic label type */ > + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) { > p = virXPathStringLimit("string(./seclabel/baselabel[1])", > VIR_SECURITY_LABEL_BUFLEN-1, ctxt); > - if (p != NULL) { > - def->baselabel = p; > - /* Forces none type to dynamic for back compat */ > - def->type = VIR_DOMAIN_SECLABEL_DYNAMIC; > - } > + def->baselabel = p; > } > > /* Only parse model, if static labelling, or a base > @@ -2676,7 +2671,8 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr > def, > */ > if (def->type == VIR_DOMAIN_SECLABEL_STATIC || > def->baselabel || > - !(flags & VIR_DOMAIN_XML_INACTIVE)) { > + (!(flags & VIR_DOMAIN_XML_INACTIVE) && > + def->type != VIR_DOMAIN_SECLABEL_NONE)) { > p = virXPathStringLimit("string(./seclabel/@model)", > VIR_SECURITY_MODEL_BUFLEN-1, ctxt); > if (p == NULL) { > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args > b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args > new file mode 100644 > index 0000000..651793d > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.args > @@ -0,0 +1,4 @@ > +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu > -S -M \ > +pc -m 214 -smp 1 -name QEMUGuest1 -nographic -monitor > unix:/tmp/test-monitor,\ > +server,nowait -no-acpi -boot c -hda /dev/HostVG/QEMUGuest1 -net none > -serial \ > +none -parallel none -usb > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml > b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml > new file mode 100644 > index 0000000..1ef97ce > --- /dev/null > +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-none.xml > @@ -0,0 +1,26 @@ > +<domain type='qemu'> > + <name>QEMUGuest1</name> > + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> > + <memory>219100</memory> > + <currentMemory>219100</currentMemory> > + <vcpu cpuset='1-4,8-20,525'>1</vcpu> > + <os> > + <type arch='i686' machine='pc'>hvm</type> > + <boot dev='hd'/> > + </os> > + <clock offset='utc'/> > + <on_poweroff>destroy</on_poweroff> > + <on_reboot>restart</on_reboot> > + <on_crash>destroy</on_crash> > + <devices> > + <emulator>/usr/bin/qemu</emulator> > + <disk type='block' device='disk'> > + <source dev='/dev/HostVG/QEMUGuest1'/> > + <target dev='hda' bus='ide'/> > + <address type='drive' controller='0' bus='0' unit='0'/> > + </disk> > + <controller type='ide' index='0'/> > + <memballoon model='virtio'/> > + </devices> > + <seclabel type='none' relabel='no'/> > +</domain> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index c8ce77f..fcffc27 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -728,6 +728,7 @@ mymain(void) > DO_TEST("seclabel-dynamic-override", false, QEMU_CAPS_NAME); > DO_TEST("seclabel-static", false, QEMU_CAPS_NAME); > DO_TEST("seclabel-static-relabel", false, QEMU_CAPS_NAME); > + DO_TEST("seclabel-none", false, QEMU_CAPS_NAME); > > DO_TEST("pseries-basic", false, > QEMU_CAPS_CHARDEV, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); > diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c > index b0f80d3..ddc7cae 100644 > --- a/tests/qemuxml2xmltest.c > +++ b/tests/qemuxml2xmltest.c > @@ -19,7 +19,7 @@ > static struct qemud_driver driver; > > static int > -testCompareXMLToXMLFiles(const char *inxml, const char *outxml) > +testCompareXMLToXMLFiles(const char *inxml, const char *outxml, bool > live) > { > char *inXmlData = NULL; > char *outXmlData = NULL; > @@ -34,7 +34,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char > *outxml) > > if (!(def = virDomainDefParseString(driver.caps, inXmlData, > QEMU_EXPECTED_VIRT_TYPES, > - VIR_DOMAIN_XML_INACTIVE))) > + live ? 0 : > VIR_DOMAIN_XML_INACTIVE))) > goto fail; > > if (!(actual = virDomainDefFormat(def, VIR_DOMAIN_XML_SECURE))) > @@ -58,6 +58,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char > *outxml) > struct testInfo { > const char *name; > int different; > + bool inactive_only; > }; > > static int > @@ -75,9 +76,16 @@ testCompareXMLToXMLHelper(const void *data) > goto cleanup; > > if (info->different) { > - ret = testCompareXMLToXMLFiles(xml_in, xml_out); > + ret = testCompareXMLToXMLFiles(xml_in, xml_out, false); > } else { > - ret = testCompareXMLToXMLFiles(xml_in, xml_in); > + ret = testCompareXMLToXMLFiles(xml_in, xml_in, false); > + } > + if (!info->inactive_only) { > + if (info->different) { > + ret = testCompareXMLToXMLFiles(xml_in, xml_out, true); > + } else { > + ret = testCompareXMLToXMLFiles(xml_in, xml_in, true); > + } > } > > cleanup: > @@ -95,19 +103,19 @@ mymain(void) > if ((driver.caps = testQemuCapsInit()) == NULL) > return (EXIT_FAILURE); > > -# define DO_TEST_FULL(name, is_different) \ > +# define DO_TEST_FULL(name, is_different, inactive) \ > do { \ > - const struct testInfo info = {name, is_different}; \ > + const struct testInfo info = {name, is_different, inactive}; \ > if (virtTestRun("QEMU XML-2-XML " name, \ > 1, testCompareXMLToXMLHelper, &info) < 0) \ > ret = -1; \ > } while (0) > > # define DO_TEST(name) \ > - DO_TEST_FULL(name, 0) > + DO_TEST_FULL(name, 0, false) > > # define DO_TEST_DIFFERENT(name) \ > - DO_TEST_FULL(name, 1) > + DO_TEST_FULL(name, 1, false) > > /* Unset or set all envvars here that are copied in > qemudBuildCommandLine > * using ADD_ENV_COPY, otherwise these tests may fail due to > unexpected > @@ -200,9 +208,10 @@ mymain(void) > DO_TEST("usb-redir"); > DO_TEST("blkdeviotune"); > > - DO_TEST("seclabel-dynamic-baselabel"); > - DO_TEST("seclabel-dynamic-override"); > + DO_TEST_FULL("seclabel-dynamic-baselabel", false, true); > + DO_TEST_FULL("seclabel-dynamic-override", false, true); > DO_TEST("seclabel-static"); > + DO_TEST("seclabel-none"); > > /* These tests generate different XML */ > DO_TEST_DIFFERENT("balloon-device-auto"); > -- > 1.7.7.6 > >