<div>My bad, it actually seems that your patch fixed my issue after I manually removed</div><div>the domain XML file that was stuck in /var/run. Nevertheless, it seems that the</div><div>issue when virt-manager exited preamturely could be caused by something else:</div> <div> </div><div><blockquote class="gmail_quote" style="margin-top:0px;margin-right:0px;margin-bottom:0px;margin-left:0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> 2012-02-05 15:39:28.395+0000: 29540: error : virNetSocketReadWire:999 : End of file while reading data: Input/output error</blockquote></div><div> </div><div> </div><div> <div class="gmail_quote">On Sat, Feb 4, 2012 at 10:18 PM, Ansis Atteka <<a href="mailto:aatteka@nicira.com">aatteka@nicira.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hmm, this patch does not seem to fix my issue. See the both xml files in the attachment.<div class="HOEnZb"><div class="h5"> <div> </div><div> <div class="gmail_quote">On Sat, Feb 4, 2012 at 4:10 PM, Eric Blake <<a href="mailto:eblake@redhat.com" target="_blank">eblake@redhat.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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> +  <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 </blockquote></div> </div> </div></div></blockquote></div> </div>