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

Ansis Atteka aatteka at nicira.com
Sat Feb 4 20:18:02 UTC 2012


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/20120204/6e69615b/attachment-0001.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Extender2.xml_conf
Type: application/octet-stream
Size: 2280 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120204/6e69615b/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Extender2.xml_run
Type: application/octet-stream
Size: 4985 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120204/6e69615b/attachment-0003.obj>


More information about the libvir-list mailing list