On 2011年12月23日 08:47, Eric Blake wrote: ACK.

On 2011年12月23日 08:47, Eric Blake wrote: ACK.

On 2011年12月23日 08:47, Eric Blake wrote: ACK.

Implement the parsing and formatting of the XML addition of the previous commit. The new XML doesn't affect qemu command line, so we can now test round-trip XML->memory->XML handling. I chose to reuse the existing structure, even though per-device override doesn't use all of those fields, rather than create a new structure, in order to reuse more code. * src/conf/domain_conf.h (_virDomainDiskDef): Add seclabel member. * src/conf/domain_conf.c (virDomainDiskDefFree): Free it. (virSecurityLabelDefFree): New function. (virDomainDiskDefFormat): Print it. (virSecurityLabelDefFormat): Reduce output if model not present. (virDomainDiskDefParseXML): Alter signature, and parse seclabel. (virSecurityLabelDefParseXML): Split... (virSecurityLabelDefParseXMLHelper): ...into new helper. (virDomainDeviceDefParse, virDomainDefParseXML): Update callers. * tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args: New file. * tests/qemuxml2xmltest.c (mymain): Enhance test. * tests/qemuxml2argvtest.c (mymain): Likewise. --- src/conf/domain_conf.c | 178 +++++++++++++++----- src/conf/domain_conf.h | 1 + .../qemuxml2argv-seclabel-dynamic-override.args | 5 + tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmltest.c | 1 + 5 files changed, 142 insertions(+), 44 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 41db117..dcead7f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -797,6 +797,15 @@ virSecurityLabelDefClear(virSecurityLabelDefPtr def) VIR_FREE(def->baselabel); } +static void +virSecurityLabelDefFree(virSecurityLabelDefPtr def) +{ + if (!def) + return; + virSecurityLabelDefClear(def); + VIR_FREE(def); +} + void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) { int ii; @@ -866,6 +875,7 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def) VIR_FREE(def->serial); VIR_FREE(def->src); + virSecurityLabelDefFree(def->seclabel); VIR_FREE(def->dst); VIR_FREE(def->driverName); VIR_FREE(def->driverType); @@ -2517,31 +2527,32 @@ virDomainDiskDefAssignAddress(virCapsPtr caps, virDomainDiskDefPtr def) return 0; } +/* Parse the portion of a SecurityLabel that is common to both the + * top-level <seclabel> and to a per-device override. + * default_seclabel is NULL for top-level, or points to the top-level + * when parsing an override. */ static int -virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, - xmlXPathContextPtr ctxt, - unsigned int flags) +virSecurityLabelDefParseXMLHelper(virSecurityLabelDefPtr def, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + virSecurityLabelDefPtr default_seclabel, + unsigned int flags) { char *p; + xmlNodePtr save_ctxt = ctxt->node; + int ret = -1; - if (virXPathNode("./seclabel", ctxt) == NULL) - return 0; + ctxt->node = node; - 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; + /* Can't use overrides if top-level doesn't allow relabeling. */ + if (default_seclabel && default_seclabel->norelabel) { + virDomainReportError(VIR_ERR_XML_ERROR, "%s", + _("label overrides require relabeling to be " + "enabled at the domain level")); + goto cleanup; } - p = virXPathStringLimit("string(./seclabel/@relabel)", + + p = virXPathStringLimit("string(./@relabel)", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p != NULL) { if (STREQ(p, "yes")) { @@ -2552,38 +2563,76 @@ virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, virDomainReportError(VIR_ERR_XML_ERROR, _("invalid security relabel value %s"), p); VIR_FREE(p); - goto error; + goto cleanup; } VIR_FREE(p); - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + if (!default_seclabel && + def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && def->norelabel) { virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("dynamic label type must use resource relabeling")); - goto error; + goto cleanup; } } else { - if (def->type == VIR_DOMAIN_SECLABEL_STATIC) + if (!default_seclabel && def->type == VIR_DOMAIN_SECLABEL_STATIC) def->norelabel = true; else def->norelabel = false; } /* Only parse label, if using static labels, or - * if the 'live' VM XML is requested + * if the 'live' VM XML is requested, or if this is a device override */ if (def->type == VIR_DOMAIN_SECLABEL_STATIC || - !(flags & VIR_DOMAIN_XML_INACTIVE)) { - p = virXPathStringLimit("string(./seclabel/label[1])", + !(flags & VIR_DOMAIN_XML_INACTIVE) || + (default_seclabel && !def->norelabel)) { + p = virXPathStringLimit("string(./label[1])", VIR_SECURITY_LABEL_BUFLEN-1, ctxt); if (p == NULL) { virDomainReportError(VIR_ERR_XML_ERROR, "%s", _("security label is missing")); - goto error; + goto cleanup; } def->label = p; } + ret = 0; +cleanup: + ctxt->node = save_ctxt; + return ret; +} + +/* Parse the top-level <seclabel>, if present. */ +static int +virSecurityLabelDefParseXML(virSecurityLabelDefPtr def, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + char *p; + xmlNodePtr node = virXPathNode("./seclabel", ctxt); + + if (node == NULL) + return 0; + + 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; + } + + if (virSecurityLabelDefParseXMLHelper(def, node, ctxt, NULL, flags) < 0) + goto error; + /* Only parse imagelabel, if requested live XML with relabeling */ if (!def->norelabel && !(flags & VIR_DOMAIN_XML_INACTIVE)) { @@ -2709,6 +2758,7 @@ virDomainDiskDefParseXML(virCapsPtr caps, xmlNodePtr node, xmlXPathContextPtr ctxt, virBitmapPtr bootMap, + virSecurityLabelDefPtr default_seclabel, unsigned int flags) { virDomainDiskDefPtr def; @@ -3016,6 +3066,16 @@ virDomainDiskDefParseXML(virCapsPtr caps, goto error; } + /* If source is present, check for an optional seclabel override. */ + if (source) { + xmlNodePtr seclabel = virXPathNode("./source/seclabel", ctxt); + if (seclabel && + (VIR_ALLOC(def->seclabel) < 0 || + virSecurityLabelDefParseXMLHelper(def->seclabel, seclabel, ctxt, + default_seclabel, flags) < 0)) + goto error; + } + if (target == NULL) { virDomainReportError(VIR_ERR_NO_TARGET, source ? "%s" : NULL, source); @@ -6344,7 +6404,8 @@ virDomainDeviceDefPtr virDomainDeviceDefParse(virCapsPtr caps, if (xmlStrEqual(node->name, BAD_CAST "disk")) { dev->type = VIR_DOMAIN_DEVICE_DISK; if (!(dev->data.disk = virDomainDiskDefParseXML(caps, node, ctxt, - NULL, flags))) + NULL, &def->seclabel, + flags))) goto error; } else if (xmlStrEqual(node->name, BAD_CAST "lease")) { dev->type = VIR_DOMAIN_DEVICE_LEASE; @@ -7446,6 +7507,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps, nodes[i], ctxt, bootMap, + &def->seclabel, flags); if (!disk) goto error; @@ -9749,23 +9811,32 @@ virSecurityLabelDefFormat(virBufferPtr buf, virSecurityLabelDefPtr def, if (!sectype) goto cleanup; - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && + if (def->model && + def->type == VIR_DOMAIN_SECLABEL_DYNAMIC && !def->baselabel && (flags & VIR_DOMAIN_XML_INACTIVE)) { /* This is the default for inactive xml, so nothing to output. */ } else { - virBufferAsprintf(buf, "<seclabel type='%s' model='%s' relabel='%s'>\n", - sectype, def->model, + virBufferAddLit(buf, "<seclabel"); + if (def->model) + virBufferAsprintf(buf, " type='%s' model='%s'", + sectype, def->model); + virBufferAsprintf(buf, " relabel='%s'", def->norelabel ? "no" : "yes"); - virBufferEscapeString(buf, " <label>%s</label>\n", - def->label); - if (!def->norelabel) - virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", - def->imagelabel); - if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) - virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", - def->baselabel); - virBufferAddLit(buf, "</seclabel>\n"); + if (def->label || def->baselabel) { + virBufferAddLit(buf, ">\n"); + virBufferEscapeString(buf, " <label>%s</label>\n", + def->label); + if (!def->norelabel) + virBufferEscapeString(buf, " <imagelabel>%s</imagelabel>\n", + def->imagelabel); + if (def->type == VIR_DOMAIN_SECLABEL_DYNAMIC) + virBufferEscapeString(buf, " <baselabel>%s</baselabel>\n", + def->baselabel); + virBufferAddLit(buf, "</seclabel>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } } ret = 0; cleanup: @@ -9885,17 +9956,36 @@ virDomainDiskDefFormat(virBufferPtr buf, def->startupPolicy) { switch (def->type) { case VIR_DOMAIN_DISK_TYPE_FILE: - virBufferAsprintf(buf," <source"); + virBufferAddLit(buf, " <source"); if (def->src) virBufferEscapeString(buf, " file='%s'", def->src); if (def->startupPolicy) virBufferEscapeString(buf, " startupPolicy='%s'", startupPolicy); - virBufferAsprintf(buf, "/>\n"); + if (def->seclabel) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 8); + if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0) + return -1; + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } break; case VIR_DOMAIN_DISK_TYPE_BLOCK: - virBufferEscapeString(buf, " <source dev='%s'/>\n", - def->src); + if (def->src && def->seclabel) { + virBufferEscapeString(buf, " <source dev='%s'>\n", + def->src); + virBufferAdjustIndent(buf, 8); + if (virSecurityLabelDefFormat(buf, def->seclabel, flags) < 0) + return -1; + virBufferAdjustIndent(buf, -8); + virBufferAddLit(buf, " </source>\n"); + } else { + virBufferEscapeString(buf, " <source dev='%s'/>\n", + def->src); + } break; case VIR_DOMAIN_DISK_TYPE_DIR: virBufferEscapeString(buf, " <source dir='%s'/>\n", diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7c5946f..d894288 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -357,6 +357,7 @@ struct _virDomainDiskDef { int device; int bus; char *src; + virSecurityLabelDefPtr seclabel; char *dst; int protocol; int nhosts; diff --git a/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args new file mode 100644 index 0000000..8f78dcf --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-seclabel-dynamic-override.args @@ -0,0 +1,5 @@ +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 \ +-hdb /dev/HostVG/QEMUGuest2 -net none -serial \ +none -parallel none -usb diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 18e8941..69e2612 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -662,6 +662,7 @@ mymain(void) DO_TEST("seclabel-dynamic", false, QEMU_CAPS_NAME); DO_TEST("seclabel-dynamic-baselabel", false, QEMU_CAPS_NAME); + 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); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e4b99c4..0a8a28e 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -195,6 +195,7 @@ mymain(void) DO_TEST("blkdeviotune"); DO_TEST("seclabel-dynamic-baselabel"); + DO_TEST("seclabel-dynamic-override"); DO_TEST("seclabel-static"); /* These tests generate different XML */ -- 1.7.7.4