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

Ansis Atteka aatteka at nicira.com
Sun Feb 5 15:42:24 UTC 2012


My bad, it actually seems that your patch fixed my issue after I manually
removed
the domain XML file that was stuck in /var/run. Nevertheless, it seems that
the
issue when virt-manager exited preamturely could be caused by something
else:

2012-02-05 15:39:28.395+0000: 29540: error : virNetSocketReadWire:999 : End
> of file while reading data: Input/output error




On Sat, Feb 4, 2012 at 10:18 PM, Ansis Atteka <aatteka at nicira.com> wrote:

> 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 at redhat.com> 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
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120205/be04bb4c/attachment-0001.htm>


More information about the libvir-list mailing list