[PATCH 00/25] conf: domain: Refactor virDomainDiskDefParseXML
Split out all default setting and validation code into appropriate functions and convert the XML parser to contemporary helpers. Peter Krempa (25): virXMLParseHelper: Add root XML node name validation capability util: xml: Introduce virXMLParseStringCtxtRoot conf: domain: Introduce virDomainDiskDefParseSource qemu: driver: Use virDomainDiskDefParseSource in qemuDomainBlockCopy conf: domain: Introduce an internal variant of virDomainDiskDefNew conf: domain: Split out source validation part from virDomainDiskDefParseValidate conf: domain: Split out parsing of source data from <disk> XML parser conf: domain: Remove VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag virDomainDiskDefValidate: Consolidate conditions conf: Move code from virDomainDiskDefParseValidate to virDomainDiskDefValidate conf: Move disk vendor and product pritability check to domain_validate conf: Move checks from virDomainDiskDefPostParse to virDomainDiskDefValidate conf: Move disk target 'ioemu:' stripping to virDomainDiskDefPostParse conf: domain: Introduce VIR_DOMAIN_DISK_BUS_NONE vmx: Mark CDROM disk elements as read-only conf: domain: Move default setting from virDomainDiskDefParseXML to virDomainDiskDefPostParse conf: domain: Move checks from virDomainDiskDefParseXML to virDomainDiskDefValidate conf: domain: Convert virDomainDiskDef's 'removable' to virTristateSwitch conf: domain: Convert virDomainDiskDef's 'rawio' to virTristateBool conf: domain: Convert virDomainDiskDef's 'sgio' to virDomainDeviceSGIO conf: domain: Convert virDomainDiskDef's 'model' to virDomainDiskModel conf: domain: Convert virDomainDiskDef's 'snapshot' to unsigned int conf: domain: Convert virDomainDiskDef's 'bus' to virDomainDiskBus conf: domain: Convert virDomainDiskDef's 'device' to virDomainDiskDevice conf: domain: Refactor virDomainDiskDefParseXML src/bhyve/bhyve_command.c | 12 + src/bhyve/bhyve_domain.c | 12 + src/conf/domain_conf.c | 715 ++++++------------ src/conf/domain_conf.h | 32 +- src/conf/domain_validate.c | 245 +++++- src/conf/domain_validate.h | 2 + src/conf/storage_conf.c | 16 +- src/hyperv/hyperv_driver.c | 18 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 11 + src/qemu/qemu_alias.c | 1 + src/qemu/qemu_command.c | 6 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_driver.c | 9 +- src/qemu/qemu_hotplug.c | 2 + src/qemu/qemu_validate.c | 3 + src/util/virxml.c | 18 +- src/util/virxml.h | 17 +- src/vbox/vbox_common.c | 1 + src/vmx/vmx.c | 4 + src/vz/vz_sdk.c | 22 +- src/vz/vz_utils.c | 8 + .../vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 1 + .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 1 + .../vmx2xml-cdrom-ide-raw-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 1 + .../vmx2xml-cdrom-scsi-passthru.xml | 1 + .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 1 + .../vmx2xml-cdrom-scsi-raw-device.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 3 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml | 1 + .../vmx2xml-fusion-in-the-wild-1.xml | 1 + .../vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 1 + .../vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 1 + 43 files changed, 645 insertions(+), 534 deletions(-) -- 2.30.2
Some callers want to validate the root XML node name. Add the capability to the parser helper to prevent open-coding. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.c | 18 ++++++++++++++++-- src/util/virxml.h | 13 +++++++------ 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 83644af8ce..d0d9494009 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1020,11 +1020,15 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) * @filename: file to be parsed or NULL if string parsing is requested * @xmlStr: XML string to be parsed in case filename is NULL * @url: URL of XML document for string parser + * @rootelement: Optional name of the expected root element * @ctxt: optional pointer to populate with new context pointer * * Parse XML document provided either as a file or a string. The function * guarantees that the XML document contains a root element. * + * If @rootelement is not NULL, the name of the root element of the parsed XML + * is vaidated against + * * Returns parsed XML document. */ xmlDocPtr @@ -1032,11 +1036,13 @@ virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, const char *url, + const char *rootelement, xmlXPathContextPtr *ctxt) { struct virParserData private; g_autoptr(xmlParserCtxt) pctxt = NULL; g_autoptr(xmlDoc) xml = NULL; + xmlNodePtr rootnode; const char *docname; if (filename) @@ -1075,18 +1081,26 @@ virXMLParseHelper(int domcode, return NULL; } - if (xmlDocGetRootElement(xml) == NULL) { + if (!(rootnode = xmlDocGetRootElement(xml))) { virGenericReportError(domcode, VIR_ERR_INTERNAL_ERROR, "%s", _("missing root element")); return NULL; } + if (rootelement && + !virXMLNodeNameEqual(rootnode, rootelement)) { + virReportError(VIR_ERR_XML_ERROR, + _("expecting root element of '%s', not '%s'"), + rootelement, rootnode->name); + return NULL; + } + if (ctxt) { if (!(*ctxt = virXMLXPathContextNew(xml))) return NULL; - (*ctxt)->node = xmlDocGetRootElement(xml); + (*ctxt)->node = rootnode; } return g_steal_pointer(&xml); diff --git a/src/util/virxml.h b/src/util/virxml.h index 022ead58b5..38da2931a4 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -150,6 +150,7 @@ virXMLParseHelper(int domcode, const char *filename, const char *xmlStr, const char *url, + const char *rootelement, xmlXPathContextPtr *ctxt); const char * @@ -166,7 +167,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParse(filename, xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL) + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, NULL) /** * virXMLParseString: @@ -178,7 +179,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseString(xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, NULL) /** * virXMLParseFile: @@ -189,7 +190,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseFile(filename) \ - virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, NULL) /** * virXMLParseCtxt: @@ -204,7 +205,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseCtxt(filename, xmlStr, url, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, pctxt) + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, pctxt) /** * virXMLParseStringCtxt: @@ -218,7 +219,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseStringCtxt(xmlStr, url, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, pctxt) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, pctxt) /** * virXMLParseFileCtxt: @@ -231,7 +232,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseFileCtxt(filename, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, pctxt) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, pctxt) int virXMLSaveFile(const char *path, -- 2.30.2
Use the new macro instead of virXMLParseStringCtxt in places where the root node is being validated. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 9 +-------- src/conf/storage_conf.c | 16 +++++----------- src/util/virxml.h | 4 ++++ 3 files changed, 10 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 17bbeddec6..6795f0b3be 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -16438,16 +16438,9 @@ virDomainDiskDefParse(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(disk_definition)"), &ctxt))) + if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) return NULL; - if (!virXMLNodeNameEqual(ctxt->node, "disk")) { - virReportError(VIR_ERR_XML_ERROR, - _("expecting root element of 'disk', not '%s'"), - ctxt->node->name); - return NULL; - } - return virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, flags); } diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 66419616da..328650bd6a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -691,25 +691,19 @@ virStoragePoolDefParseSourceString(const char *srcSpec, int pool_type) { g_autoptr(xmlDoc) doc = NULL; - xmlNodePtr node = NULL; g_autoptr(xmlXPathContext) xpath_ctxt = NULL; g_autoptr(virStoragePoolSource) def = NULL; - if (!(doc = virXMLParseStringCtxt(srcSpec, - _("(storage_source_specification)"), - &xpath_ctxt))) + if (!(doc = virXMLParseStringCtxtRoot(srcSpec, + _("(storage_source_specification)"), + "source", + &xpath_ctxt))) return NULL; def = g_new0(virStoragePoolSource, 1); - if (!(node = virXPathNode("/source", xpath_ctxt))) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("root element was not source")); - return NULL; - } - if (virStoragePoolDefParseSource(xpath_ctxt, def, pool_type, - node) < 0) + xpath_ctxt->node) < 0) return NULL; return g_steal_pointer(&def); diff --git a/src/util/virxml.h b/src/util/virxml.h index 38da2931a4..2b40398eee 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -221,6 +221,10 @@ virXMLPickShellSafeComment(const char *str1, #define virXMLParseStringCtxt(xmlStr, url, pctxt) \ virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, pctxt) +/* virXMLParseStringCtxtRoot is same as above, except it also validates root node name */ +#define virXMLParseStringCtxtRoot(xmlStr, url, rootnode, pctxt) \ + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, rootnode, pctxt) + /** * virXMLParseFileCtxt: * @filename: file to parse -- 2.30.2
Add a helper function which will parse the source portion of a <disk>. The idea is to replace *virDomainDiskDefParse with VIR_DOMAIN_DEF_PARSE_DISK_SOURCE with the new helper in the future. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 30 ++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 3 +++ src/libvirt_private.syms | 1 + 3 files changed, 34 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6795f0b3be..57cad6ffde 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9704,6 +9704,21 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } +static virStorageSource * +virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + g_autoptr(virDomainDiskDef) diskdef = NULL; + + if (!(diskdef = virDomainDiskDefParseXML(xmlopt, node, ctxt, + flags | VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) + return NULL; + + return g_steal_pointer(&diskdef->src); +} + /** * virDomainParseMemory: @@ -16445,6 +16460,21 @@ virDomainDiskDefParse(const char *xmlStr, } +virStorageSource * +virDomainDiskDefParseSource(const char *xmlStr, + virDomainXMLOption *xmlopt, + unsigned int flags) +{ + g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; + + if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) + return NULL; + + return virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags); +} + + static const char * virDomainChrTargetTypeToString(int deviceType, int targetType) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 99ab2a96d9..83b9288941 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3406,6 +3406,9 @@ virDomainDeviceDef *virDomainDeviceDefParse(const char *xmlStr, virDomainDiskDef *virDomainDiskDefParse(const char *xmlStr, virDomainXMLOption *xmlopt, unsigned int flags); +virStorageSource *virDomainDiskDefParseSource(const char *xmlStr, + virDomainXMLOption *xmlopt, + unsigned int flags); virDomainDef *virDomainDefParseString(const char *xmlStr, virDomainXMLOption *xmlopt, void *parseOpaque, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0fcbf546a6..fe3b443171 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -371,6 +371,7 @@ virDomainDiskDefCheckDuplicateInfo; virDomainDiskDefFree; virDomainDiskDefNew; virDomainDiskDefParse; +virDomainDiskDefParseSource; virDomainDiskDetectZeroesTypeFromString; virDomainDiskDetectZeroesTypeToString; virDomainDiskDeviceTypeToString; -- 2.30.2
qemuDomainBlockCopy needs just the source portion of the disk but uses the disk parser for it. Since we have a specific function now, refactor the code to avoid having to deal with the unused virDomainDiskDef. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 88ee9e5d5e..d908e95ba7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15273,7 +15273,6 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, unsigned long long bandwidth = 0; unsigned int granularity = 0; unsigned long long buf_size = 0; - virDomainDiskDef *diskdef = NULL; virStorageSource *dest = NULL; size_t i; @@ -15326,18 +15325,14 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *disk, const char *destxml, } } - if (!(diskdef = virDomainDiskDefParse(destxml, driver->xmlopt, - VIR_DOMAIN_DEF_PARSE_INACTIVE | - VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) + if (!(dest = virDomainDiskDefParseSource(destxml, driver->xmlopt, + VIR_DOMAIN_DEF_PARSE_INACTIVE))) goto cleanup; - dest = g_steal_pointer(&diskdef->src); - ret = qemuDomainBlockCopyCommon(vm, dom->conn, disk, dest, bandwidth, granularity, buf_size, flags, false); cleanup: - virDomainDiskDefFree(diskdef); virDomainObjEndAPI(&vm); return ret; } -- 2.30.2
The <disk> XML element parser is going to be modified so that the virStorageSource bits are pre-parsed. Add virDomainDiskDefNewSource, which uses an existing 'src' pointer. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 57cad6ffde..f59d17930b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2230,25 +2230,32 @@ virDomainDefGetVcpusTopology(const virDomainDef *def, } -virDomainDiskDef * -virDomainDiskDefNew(virDomainXMLOption *xmlopt) +static virDomainDiskDef * +virDomainDiskDefNewSource(virDomainXMLOption *xmlopt, + virStorageSource **src) { + void *privateData = NULL; virDomainDiskDef *ret; - ret = g_new0(virDomainDiskDef, 1); - - ret->src = virStorageSourceNew(); - if (xmlopt && xmlopt->privateData.diskNew && - !(ret->privateData = xmlopt->privateData.diskNew())) - goto error; + !(privateData = xmlopt->privateData.diskNew())) + return NULL; + + ret = g_new0(virDomainDiskDef, 1); + ret->src = g_steal_pointer(src); + ret->privateData = privateData; return ret; +} - error: - virDomainDiskDefFree(ret); - return NULL; + +virDomainDiskDef * +virDomainDiskDefNew(virDomainXMLOption *xmlopt) +{ + g_autoptr(virStorageSource) src = virStorageSourceNew(); + + return virDomainDiskDefNewSource(xmlopt, &src); } -- 2.30.2
Separate the validation of the source so that it can be reused once we split up the XML parser too. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 82 +++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 36 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f59d17930b..0eec4c9356 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9045,29 +9045,55 @@ virDomainDiskDefGeometryParse(virDomainDiskDef *def, static int -virDomainDiskSourceDefParseAuthValidate(const virStorageSource *src) +virDomainDiskDefParseValidateSourceChainOne(const virStorageSource *src) { - virStorageAuthDef *authdef = src->auth; - int actUsage; + if (src->type == VIR_STORAGE_TYPE_NETWORK && src->auth) { + virStorageAuthDef *authdef = src->auth; + int actUsage; - if (src->type != VIR_STORAGE_TYPE_NETWORK || !authdef) - return 0; + if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown secret type '%s'"), + NULLSTR(authdef->secrettype)); + return -1; + } - if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown secret type '%s'"), - NULLSTR(authdef->secrettype)); - return -1; + if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && + actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) || + (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && + actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid secret type '%s'"), + virSecretUsageTypeToString(actUsage)); + return -1; + } } - if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && - actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) || - (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && - actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid secret type '%s'"), - virSecretUsageTypeToString(actUsage)); - return -1; + if (src->encryption) { + virStorageEncryption *encryption = src->encryption; + + if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + encryption->encinfo.cipher_name) { + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("supplying <cipher> for domain disk definition " + "is unnecessary")); + return -1; + } + } + + return 0; +} + + +static int +virDomainDiskDefParseValidateSource(const virStorageSource *src) +{ + const virStorageSource *next; + + for (next = src; next; next = next->backingStore) { + if (virDomainDiskDefParseValidateSourceChainOne(next) < 0) + return -1; } return 0; @@ -9078,7 +9104,8 @@ static int virDomainDiskDefParseValidate(const virDomainDiskDef *def) { - virStorageSource *next; + if (virDomainDiskDefParseValidateSource(def->src) < 0) + return -1; if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { @@ -9150,23 +9177,6 @@ virDomainDiskDefParseValidate(const virDomainDiskDef *def) } } - for (next = def->src; next; next = next->backingStore) { - if (virDomainDiskSourceDefParseAuthValidate(next) < 0) - return -1; - - if (next->encryption) { - virStorageEncryption *encryption = next->encryption; - - if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && - encryption->encinfo.cipher_name) { - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("supplying <cipher> for domain disk definition " - "is unnecessary")); - return -1; - } - } - } return 0; } -- 2.30.2
Extract all code related to parsing data which ends up in the 'src' member of a virDomainDiskDef. This allows to use the new function directly in virDomainDiskDefParseSource and removes the use of the VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 185 ++++++++++++++++++++++------------------- 1 file changed, 100 insertions(+), 85 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0eec4c9356..1793032301 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9341,6 +9341,86 @@ virDomainDiskDefParsePrivateData(xmlXPathContextPtr ctxt, } +static virStorageSource * +virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, + xmlNodePtr node, + xmlXPathContextPtr ctxt, + unsigned int flags) +{ + g_autoptr(virStorageSource) src = virStorageSourceNew(); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + g_autofree char *type = NULL; + xmlNodePtr tmp; + + ctxt->node = node; + + src->type = VIR_STORAGE_TYPE_FILE; + + if ((type = virXMLPropString(node, "type")) && + (src->type = virStorageTypeFromString(type)) <= 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown disk type '%s'"), type); + return NULL; + } + + if ((tmp = virXPathNode("./source[1]", ctxt))) { + if (virDomainStorageSourceParse(tmp, ctxt, src, flags, xmlopt) < 0) + return NULL; + + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + g_autofree char *sourceindex = NULL; + + if ((sourceindex = virXMLPropString(tmp, "index")) && + virStrToLong_uip(sourceindex, NULL, 10, &src->id) < 0) { + virReportError(VIR_ERR_XML_ERROR, + _("invalid disk index '%s'"), sourceindex); + return NULL; + } + } + } else { + /* Reset src->type in case when 'source' was not present */ + src->type = VIR_STORAGE_TYPE_FILE; + } + + if (virXPathNode("./readonly[1]", ctxt)) + src->readonly = true; + + if (virXPathNode("./shareable[1]", ctxt)) + src->shared = true; + + if ((tmp = virXPathNode("./auth", ctxt))) { + /* If we've already parsed <source> and found an <auth> child, + * then generate an error to avoid ambiguity */ + if (src->auth) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <auth> definition already found for disk source")); + return NULL; + } + + if (!(src->auth = virStorageAuthDefParse(tmp, ctxt))) + return NULL; + } + + if ((tmp = virXPathNode("./encryption", ctxt))) { + /* If we've already parsed <source> and found an <encryption> child, + * then generate an error to avoid ambiguity */ + if (src->encryption) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("an <encryption> definition already found for disk source")); + return NULL; + } + + if (!(src->encryption = virStorageEncryptionParseNode(tmp, ctxt))) + return NULL; + } + + if (virDomainDiskBackingStoreParse(ctxt, src, flags, xmlopt) < 0) + return NULL; + + return g_steal_pointer(&src); +} + + static virDomainDiskDef * virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr node, @@ -9351,8 +9431,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt) bool source = false; - g_autoptr(virStorageEncryption) encryption = NULL; - g_autoptr(virStorageAuthDef) authdef = NULL; g_autofree char *tmp = NULL; g_autofree char *snapshot = NULL; g_autofree char *rawio = NULL; @@ -9368,24 +9446,19 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *product = NULL; g_autofree char *domain_name = NULL; g_autofree char *rotation_rate = NULL; + g_autoptr(virStorageSource) src = NULL; + + if (!(src = virDomainDiskDefParseSourceXML(xmlopt, node, ctxt, flags))) + return NULL; - if (!(def = virDomainDiskDefNew(xmlopt))) + if (!(def = virDomainDiskDefNewSource(xmlopt, &src))) return NULL; ctxt->node = node; /* defaults */ - def->src->type = VIR_STORAGE_TYPE_FILE; def->device = VIR_DOMAIN_DISK_DEVICE_DISK; - if ((tmp = virXMLPropString(node, "type")) && - (def->src->type = virStorageTypeFromString(tmp)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk type '%s'"), tmp); - return NULL; - } - VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "device")) && (def->device = virDomainDiskDeviceTypeFromString(tmp)) < 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -9412,9 +9485,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, continue; if (!source && virXMLNodeNameEqual(cur, "source")) { - if (virDomainStorageSourceParse(cur, ctxt, def->src, flags, xmlopt) < 0) - return NULL; - source = true; if (virXMLPropEnum(cur, "startupPolicy", @@ -9423,13 +9493,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, &def->startupPolicy) < 0) return NULL; - if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) && - (tmp = virXMLPropString(cur, "index")) && - virStrToLong_uip(tmp, NULL, 10, &def->src->id) < 0) { - virReportError(VIR_ERR_XML_ERROR, _("invalid disk index '%s'"), tmp); - return NULL; - } - VIR_FREE(tmp); } else if (!target && virXMLNodeNameEqual(cur, "target")) { target = virXMLPropString(cur, "dev"); @@ -9484,27 +9547,15 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0) return NULL; - } else if (!authdef && - virXMLNodeNameEqual(cur, "auth")) { - if (!(authdef = virStorageAuthDefParse(cur, ctxt))) - return NULL; + } else if (virXMLNodeNameEqual(cur, "auth")) { def->diskElementAuth = true; } else if (virXMLNodeNameEqual(cur, "iotune")) { if (virDomainDiskDefIotuneParse(def, ctxt) < 0) return NULL; - } else if (virXMLNodeNameEqual(cur, "readonly")) { - def->src->readonly = true; - } else if (virXMLNodeNameEqual(cur, "shareable")) { - def->src->shared = true; } else if (virXMLNodeNameEqual(cur, "transient")) { def->transient = true; - } else if (!encryption && - virXMLNodeNameEqual(cur, "encryption")) { - if (!(encryption = virStorageEncryptionParseNode(cur, ctxt))) - return NULL; - + } else if (virXMLNodeNameEqual(cur, "encryption")) { def->diskElementEnc = true; - } else if (!serial && virXMLNodeNameEqual(cur, "serial")) { if (!(serial = virXMLNodeContentString(cur))) @@ -9548,10 +9599,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - /* Reset def->src->type in case when 'source' was not present */ - if (!source) - def->src->type = VIR_STORAGE_TYPE_FILE; - /* Only CDROM and Floppy devices are allowed missing source path * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ @@ -9676,40 +9723,12 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } def->dst = g_steal_pointer(&target); - if (authdef) { - /* If we've already parsed <source> and found an <auth> child, - * then generate an error to avoid ambiguity */ - if (def->src->auth) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("an <auth> definition already found for " - "disk source")); - return NULL; - } - - def->src->auth = g_steal_pointer(&authdef); - } - - if (encryption) { - /* If we've already parsed <source> and found an <encryption> child, - * then generate an error to avoid ambiguity */ - if (def->src->encryption) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("an <encryption> definition already found for " - "disk source")); - return NULL; - } - - def->src->encryption = g_steal_pointer(&encryption); - } def->domain_name = g_steal_pointer(&domain_name); def->serial = g_steal_pointer(&serial); def->wwn = g_steal_pointer(&wwn); def->vendor = g_steal_pointer(&vendor); def->product = g_steal_pointer(&product); - if (virDomainDiskBackingStoreParse(ctxt, def->src, flags, xmlopt) < 0) - return NULL; - if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) return NULL; @@ -9721,22 +9740,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } -static virStorageSource * -virDomainDiskDefParseSourceXML(virDomainXMLOption *xmlopt, - xmlNodePtr node, - xmlXPathContextPtr ctxt, - unsigned int flags) -{ - g_autoptr(virDomainDiskDef) diskdef = NULL; - - if (!(diskdef = virDomainDiskDefParseXML(xmlopt, node, ctxt, - flags | VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) - return NULL; - - return g_steal_pointer(&diskdef->src); -} - - /** * virDomainParseMemory: * @xpath: XPath to memory amount @@ -16484,11 +16487,23 @@ virDomainDiskDefParseSource(const char *xmlStr, { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; + g_autoptr(virStorageSource) src = NULL; if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) return NULL; - return virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags); + if (!(src = virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags))) + return NULL; + + if (virStorageSourceIsEmpty(src)) { + virReportError(VIR_ERR_NO_SOURCE, NULL); + return NULL; + } + + if (virDomainDiskDefParseValidateSource(src) < 0) + return NULL; + + return g_steal_pointer(&src); } -- 2.30.2
There's no code which would assert it at this point. Remove the flag. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 47 ++++++++++++++++++++---------------------- src/conf/domain_conf.h | 14 ++++++------- 2 files changed, 28 insertions(+), 33 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1793032301..3f32e046f3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9603,14 +9603,13 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, * to indicate no media present. LUN is for raw access CD-ROMs * that are not attached to a physical device presently */ if (virStorageSourceIsEmpty(def->src) && - (def->device == VIR_DOMAIN_DISK_DEVICE_DISK || - (flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE))) { + def->device == VIR_DOMAIN_DISK_DEVICE_DISK) { virReportError(VIR_ERR_NO_SOURCE, target ? "%s" : NULL, target); return NULL; } - if (!target && !(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { + if (!target) { if (def->src->srcpool) { tmp = g_strdup_printf("pool = '%s', volume = '%s'", def->src->srcpool->pool, def->src->srcpool->volume); @@ -9623,29 +9622,27 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { - if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && - !STRPREFIX(target, "fd")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid floppy device name: %s"), target); - return NULL; - } + if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && + !STRPREFIX(target, "fd")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid floppy device name: %s"), target); + return NULL; + } - /* Force CDROM to be listed as read only */ - if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - def->src->readonly = true; + /* Force CDROM to be listed as read only */ + if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + def->src->readonly = true; - if ((def->device == VIR_DOMAIN_DISK_DEVICE_DISK || - def->device == VIR_DOMAIN_DISK_DEVICE_LUN) && - !STRPREFIX((const char *)target, "hd") && - !STRPREFIX((const char *)target, "sd") && - !STRPREFIX((const char *)target, "vd") && - !STRPREFIX((const char *)target, "xvd") && - !STRPREFIX((const char *)target, "ubd")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid harddisk device name: %s"), target); - return NULL; - } + if ((def->device == VIR_DOMAIN_DISK_DEVICE_DISK || + def->device == VIR_DOMAIN_DISK_DEVICE_LUN) && + !STRPREFIX((const char *)target, "hd") && + !STRPREFIX((const char *)target, "sd") && + !STRPREFIX((const char *)target, "vd") && + !STRPREFIX((const char *)target, "xvd") && + !STRPREFIX((const char *)target, "ubd")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid harddisk device name: %s"), target); + return NULL; } if (snapshot) { @@ -9686,7 +9683,7 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } else { if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { def->bus = VIR_DOMAIN_DISK_BUS_FDC; - } else if (!(flags & VIR_DOMAIN_DEF_PARSE_DISK_SOURCE)) { + } else { if (STRPREFIX(target, "hd")) def->bus = VIR_DOMAIN_DISK_BUS_IDE; else if (STRPREFIX(target, "sd")) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 83b9288941..1a2e0fc872 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3353,25 +3353,23 @@ typedef enum { VIR_DOMAIN_DEF_PARSE_ALLOW_ROM = 1 << 4, /* internal flag passed to device info sub-parser to allow specifying boot order */ VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT = 1 << 5, - /* parse only source half of <disk> */ - VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 6, /* perform RNG schema validation on the passed XML document */ - VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA = 1 << 7, + VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA = 1 << 6, /* allow updates in post parse callback that would break ABI otherwise */ - VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 8, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE = 1 << 7, /* skip definition validation checks meant to be executed on define time only */ - VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE = 1 << 9, + VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE = 1 << 8, /* skip parsing of security labels */ - VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL = 1 << 10, + VIR_DOMAIN_DEF_PARSE_SKIP_SECLABEL = 1 << 9, /* Allows updates in post parse callback for incoming persistent migration * that would break ABI otherwise. This should be used only if it's safe * to do such change. */ - VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION = 1 << 11, + VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION = 1 << 10, /* Allows to ignore certain failures in the post parse callbacks, which * may happen due to missing packages and can be fixed by re-running the * post parse callbacks before starting. Failure of the post parse callback * is recorded as def->postParseFail */ - VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL = 1 << 12, + VIR_DOMAIN_DEF_PARSE_ALLOW_POST_PARSE_FAIL = 1 << 11, } virDomainDefParseFlags; typedef enum { -- 2.30.2
Consolidate the checks for '<reservations/>' and viritio queues under already existing blocks which have the same condition. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_validate.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 570279cd92..4d253599af 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -451,13 +451,12 @@ virDomainDiskDefValidate(const virDomainDef *def, "device='lun'"), disk->dst); return -1; } - } - - if (disk->src->pr && - disk->device != VIR_DOMAIN_DISK_DEVICE_LUN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("<reservations/> allowed only for lun devices")); - return -1; + } else { + if (disk->src->pr) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("<reservations/> allowed only for lun devices")); + return -1; + } } /* Reject disks with a bus type that is not compatible with the @@ -474,13 +473,6 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } - if (disk->queues && disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("queues attribute in disk driver element is only " - "supported by virtio-blk")); - return -1; - } - if (disk->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { if (disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO || disk->model == VIR_DOMAIN_DISK_MODEL_VIRTIO_TRANSITIONAL || @@ -492,6 +484,12 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->queues) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("queues attribute in disk driver element is only supported by virtio-blk")); + return -1; + } + if (virDomainCheckVirtioOptionsAreAbsent(disk->virtio) < 0) return -1; } -- 2.30.2
Unify the two distinct disk definition validators. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 143 +------------------------------------ src/conf/domain_validate.c | 125 ++++++++++++++++++++++++++++++++ src/conf/domain_validate.h | 2 + 3 files changed, 128 insertions(+), 142 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3f32e046f3..c8c0476a42 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9044,144 +9044,6 @@ virDomainDiskDefGeometryParse(virDomainDiskDef *def, } -static int -virDomainDiskDefParseValidateSourceChainOne(const virStorageSource *src) -{ - if (src->type == VIR_STORAGE_TYPE_NETWORK && src->auth) { - virStorageAuthDef *authdef = src->auth; - int actUsage; - - if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown secret type '%s'"), - NULLSTR(authdef->secrettype)); - return -1; - } - - if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && - actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) || - (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && - actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("invalid secret type '%s'"), - virSecretUsageTypeToString(actUsage)); - return -1; - } - } - - if (src->encryption) { - virStorageEncryption *encryption = src->encryption; - - if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && - encryption->encinfo.cipher_name) { - - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("supplying <cipher> for domain disk definition " - "is unnecessary")); - return -1; - } - } - - return 0; -} - - -static int -virDomainDiskDefParseValidateSource(const virStorageSource *src) -{ - const virStorageSource *next; - - for (next = src; next; next = next->backingStore) { - if (virDomainDiskDefParseValidateSourceChainOne(next) < 0) - return -1; - } - - return 0; -} - - -static int -virDomainDiskDefParseValidate(const virDomainDiskDef *def) - -{ - if (virDomainDiskDefParseValidateSource(def->src) < 0) - return -1; - - if (def->bus != VIR_DOMAIN_DISK_BUS_VIRTIO) { - if (def->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk event_idx mode supported only for virtio bus")); - return -1; - } - - if (def->ioeventfd != VIR_TRISTATE_SWITCH_ABSENT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("disk ioeventfd mode supported only for virtio bus")); - return -1; - } - } - - if (def->device != VIR_DOMAIN_DISK_DEVICE_LUN) { - if (def->rawio != VIR_TRISTATE_BOOL_ABSENT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("rawio can be used only with device='lun'")); - return -1; - } - - if (def->sgio != VIR_DOMAIN_DEVICE_SGIO_DEFAULT) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("sgio can be used only with device='lun'")); - return -1; - } - } - - if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && - def->bus != VIR_DOMAIN_DISK_BUS_FDC) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid bus type '%s' for floppy disk"), - virDomainDiskBusTypeToString(def->bus)); - return -1; - } - - if (def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && - def->bus == VIR_DOMAIN_DISK_BUS_FDC) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid bus type '%s' for disk"), - virDomainDiskBusTypeToString(def->bus)); - return -1; - } - - if (def->removable != VIR_TRISTATE_SWITCH_ABSENT && - def->bus != VIR_DOMAIN_DISK_BUS_USB) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("removable is only valid for usb disks")); - return -1; - } - - if (def->startupPolicy != VIR_DOMAIN_STARTUP_POLICY_DEFAULT) { - if (def->src->type == VIR_STORAGE_TYPE_NETWORK) { - virReportError(VIR_ERR_XML_ERROR, - _("Setting disk %s is not allowed for " - "disk of network type"), - virDomainStartupPolicyTypeToString(def->startupPolicy)); - return -1; - } - - if (def->device != VIR_DOMAIN_DISK_DEVICE_CDROM && - def->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && - def->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Setting disk 'requisite' is allowed only for " - "cdrom or floppy")); - return -1; - } - } - - - return 0; -} - - static int virDomainDiskDefDriverParseXML(virDomainDiskDef *def, xmlNodePtr cur, @@ -9730,9 +9592,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) return NULL; - if (virDomainDiskDefParseValidate(def) < 0) - return NULL; - return g_steal_pointer(&def); } @@ -16497,7 +16356,7 @@ virDomainDiskDefParseSource(const char *xmlStr, return NULL; } - if (virDomainDiskDefParseValidateSource(src) < 0) + if (virDomainDiskDefValidateSource(src) < 0) return NULL; return g_steal_pointer(&src); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 4d253599af..97fa218252 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -430,6 +430,62 @@ virDomainDiskVhostUserValidate(const virDomainDiskDef *disk) } +static int +virDomainDiskDefValidateSourceChainOne(const virStorageSource *src) +{ + if (src->type == VIR_STORAGE_TYPE_NETWORK && src->auth) { + virStorageAuthDef *authdef = src->auth; + int actUsage; + + if ((actUsage = virSecretUsageTypeFromString(authdef->secrettype)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown secret type '%s'"), + NULLSTR(authdef->secrettype)); + return -1; + } + + if ((src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI && + actUsage != VIR_SECRET_USAGE_TYPE_ISCSI) || + (src->protocol == VIR_STORAGE_NET_PROTOCOL_RBD && + actUsage != VIR_SECRET_USAGE_TYPE_CEPH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("invalid secret type '%s'"), + virSecretUsageTypeToString(actUsage)); + return -1; + } + } + + if (src->encryption) { + virStorageEncryption *encryption = src->encryption; + + if (encryption->format == VIR_STORAGE_ENCRYPTION_FORMAT_LUKS && + encryption->encinfo.cipher_name) { + + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("supplying <cipher> for domain disk definition " + "is unnecessary")); + return -1; + } + } + + return 0; +} + + +int +virDomainDiskDefValidateSource(const virStorageSource *src) +{ + const virStorageSource *next; + + for (next = src; next; next = next->backingStore) { + if (virDomainDiskDefValidateSourceChainOne(next) < 0) + return -1; + } + + return 0; +} + + #define VENDOR_LEN 8 #define PRODUCT_LEN 16 @@ -439,6 +495,9 @@ virDomainDiskDefValidate(const virDomainDef *def, { virStorageSource *next; + if (virDomainDiskDefValidateSource(disk->src) < 0) + return -1; + /* Validate LUN configuration */ if (disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) { /* volumes haven't been translated at this point, so accept them */ @@ -457,6 +516,18 @@ virDomainDiskDefValidate(const virDomainDef *def, _("<reservations/> allowed only for lun devices")); return -1; } + + if (disk->rawio != VIR_TRISTATE_BOOL_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("rawio can be used only with device='lun'")); + return -1; + } + + if (disk->sgio != VIR_DOMAIN_DEVICE_SGIO_DEFAULT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("sgio can be used only with device='lun'")); + return -1; + } } /* Reject disks with a bus type that is not compatible with the @@ -490,6 +561,18 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->event_idx != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk event_idx mode supported only for virtio bus")); + return -1; + } + + if (disk->ioeventfd != VIR_TRISTATE_SWITCH_ABSENT) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("disk ioeventfd mode supported only for virtio bus")); + return -1; + } + if (virDomainCheckVirtioOptionsAreAbsent(disk->virtio) < 0) return -1; } @@ -538,6 +621,48 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && + disk->bus != VIR_DOMAIN_DISK_BUS_FDC) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid bus type '%s' for floppy disk"), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && + disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid bus type '%s' for disk"), + virDomainDiskBusTypeToString(disk->bus)); + return -1; + } + + if (disk->removable != VIR_TRISTATE_SWITCH_ABSENT && + disk->bus != VIR_DOMAIN_DISK_BUS_USB) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("removable is only valid for usb disks")); + return -1; + } + + if (disk->startupPolicy != VIR_DOMAIN_STARTUP_POLICY_DEFAULT) { + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK) { + virReportError(VIR_ERR_XML_ERROR, + _("Setting disk %s is not allowed for " + "disk of network type"), + virDomainStartupPolicyTypeToString(disk->startupPolicy)); + return -1; + } + + if (disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && + disk->device != VIR_DOMAIN_DISK_DEVICE_FLOPPY && + disk->startupPolicy == VIR_DOMAIN_STARTUP_POLICY_REQUISITE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Setting disk 'requisite' is allowed only for " + "cdrom or floppy")); + return -1; + } + } + return 0; } diff --git a/src/conf/domain_validate.h b/src/conf/domain_validate.h index 5f31616c85..38a1abcc8f 100644 --- a/src/conf/domain_validate.h +++ b/src/conf/domain_validate.h @@ -38,3 +38,5 @@ int virDomainDeviceDefValidate(const virDomainDeviceDef *dev, unsigned int parseFlags, virDomainXMLOption *xmlopt, void *parseOpaque); + +int virDomainDiskDefValidateSource(const virStorageSource *src); -- 2.30.2
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 12 ------------ src/conf/domain_validate.c | 37 +++++++++++++++++++++++++++---------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c8c0476a42..0738bf7f1f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9433,22 +9433,10 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, virXMLNodeNameEqual(cur, "vendor")) { if (!(vendor = virXMLNodeContentString(cur))) return NULL; - - if (!virStringIsPrintable(vendor)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk vendor is not printable string")); - return NULL; - } } else if (!product && virXMLNodeNameEqual(cur, "product")) { if (!(product = virXMLNodeContentString(cur))) return NULL; - - if (!virStringIsPrintable(product)) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk product is not printable string")); - return NULL; - } } else if (virXMLNodeNameEqual(cur, "boot")) { /* boot is parsed as part of virDomainDeviceInfoParseXML */ } else if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) && diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 97fa218252..b2780b3562 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -27,6 +27,7 @@ #include "virconftypes.h" #include "virlog.h" #include "virutil.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN @@ -607,18 +608,34 @@ virDomainDiskDefValidate(const virDomainDef *def, return -1; } - if (disk->vendor && strlen(disk->vendor) > VENDOR_LEN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk vendor is more than %d characters"), - VENDOR_LEN); - return -1; + if (disk->vendor) { + if (!virStringIsPrintable(disk->vendor)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk vendor is not printable string")); + return -1; + } + + if (strlen(disk->vendor) > VENDOR_LEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk vendor is more than %d characters"), + VENDOR_LEN); + return -1; + } } - if (disk->product && strlen(disk->product) > PRODUCT_LEN) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("disk product is more than %d characters"), - PRODUCT_LEN); - return -1; + if (disk->product) { + if (!virStringIsPrintable(disk->product)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk product is not printable string")); + return -1; + } + + if (strlen(disk->product) > PRODUCT_LEN) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("disk product is more than %d characters"), + PRODUCT_LEN); + return -1; + } } if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && -- 2.30.2
The moved code contains only checks and does not modify the parsed document so it doesn't belong into the PostParse code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 19 ------------------- src/conf/domain_validate.c | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0738bf7f1f..ce0931c160 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5367,25 +5367,6 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk, const virDomainDef *def, virDomainXMLOption *xmlopt) { - /* internal snapshots and config files are currently supported - * only with rbd: */ - if (virStorageSourceGetActualType(disk->src) != VIR_STORAGE_TYPE_NETWORK && - disk->src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { - if (disk->src->snapshot) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("<snapshot> element is currently supported " - "only with 'rbd' disks")); - return -1; - } - - if (disk->src->configFile) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("<config> element is currently supported " - "only with 'rbd' disks")); - return -1; - } - } - if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) { virDomainPostParseCheckISCSIPath(&disk->src->path); diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index b2780b3562..5118d6a25e 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -469,6 +469,24 @@ virDomainDiskDefValidateSourceChainOne(const virStorageSource *src) } } + /* internal snapshots and config files are currently supported only with rbd: */ + if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK && + src->protocol != VIR_STORAGE_NET_PROTOCOL_RBD) { + if (src->snapshot) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("<snapshot> element is currently supported " + "only with 'rbd' disks")); + return -1; + } + + if (src->configFile) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("<config> element is currently supported " + "only with 'rbd' disks")); + return -1; + } + } + return 0; } -- 2.30.2
Modifications of the data such as this one don't belong into the parser. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ce0931c160..2803055204 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5367,6 +5367,16 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk, const virDomainDef *def, virDomainXMLOption *xmlopt) { + if (disk->dst) { + char *newdst; + + /* Work around for compat with Xen driver in previous libvirt releases */ + if ((newdst = g_strdup(STRSKIP(disk->dst, "ioemu:")))) { + g_free(disk->dst); + disk->dst = newdst; + } + } + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) { virDomainPostParseCheckISCSIPath(&disk->src->path); @@ -9345,12 +9355,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, return NULL; removable = virXMLPropString(cur, "removable"); rotation_rate = virXMLPropString(cur, "rotation_rate"); - - /* HACK: Work around for compat with Xen - * driver in previous libvirt releases */ - if (target && - STRPREFIX(target, "ioemu:")) - memmove(target, target+6, strlen(target)-5); } else if (!domain_name && virXMLNodeNameEqual(cur, "backenddomain")) { domain_name = virXMLPropString(cur, "name"); -- 2.30.2
Add a disk bus value represending no selected bus. This will help split up the XML parser. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 3 ++- src/conf/domain_conf.h | 1 + src/conf/domain_validate.c | 1 + src/qemu/qemu_alias.c | 1 + src/qemu/qemu_command.c | 2 ++ src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_hotplug.c | 2 ++ src/vbox/vbox_common.c | 1 + 8 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2803055204..c600c1e6b1 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -340,6 +340,7 @@ VIR_ENUM_IMPL(virDomainDiskGeometryTrans, VIR_ENUM_IMPL(virDomainDiskBus, VIR_DOMAIN_DISK_BUS_LAST, + "none", "ide", "fdc", "scsi", @@ -9510,7 +9511,7 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } if (bus) { - if ((def->bus = virDomainDiskBusTypeFromString(bus)) < 0) { + if ((def->bus = virDomainDiskBusTypeFromString(bus)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown disk bus type '%s'"), bus); return NULL; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1a2e0fc872..cb5ce68fdb 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -371,6 +371,7 @@ typedef enum { } virDomainDiskDevice; typedef enum { + VIR_DOMAIN_DISK_BUS_NONE, VIR_DOMAIN_DISK_BUS_IDE, VIR_DOMAIN_DISK_BUS_FDC, VIR_DOMAIN_DISK_BUS_SCSI, diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 5118d6a25e..1073da3bfa 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -227,6 +227,7 @@ virDomainDiskAddressDiskBusCompatibility(virDomainDiskBus bus, case VIR_DOMAIN_DISK_BUS_USB: case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: return true; } diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 7c86a6eeaa..63638b1402 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -255,6 +255,7 @@ qemuAssignDeviceDiskAlias(virDomainDef *def, case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: break; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 77d8e3f38c..6ac36da1bb 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1607,6 +1607,7 @@ qemuCheckIOThreads(const virDomainDef *def, case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SATA: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("IOThreads not available for bus %s target %s"), @@ -1849,6 +1850,7 @@ qemuBuildDiskDeviceStr(const virDomainDef *def, case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 8adef60675..1ee75b8f2e 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -800,6 +800,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SATA: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: return 0; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 20c34ef104..4344edc75b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1092,6 +1092,7 @@ qemuDomainAttachDeviceDiskLiveInternal(virQEMUDriver *driver, /* Note that SD card hotplug support should be added only once * they support '-device' (don't require -drive only). * See also: qemuDiskBusIsSD */ + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: virReportError(VIR_ERR_OPERATION_UNSUPPORTED, _("disk bus '%s' cannot be hotplugged."), @@ -5315,6 +5316,7 @@ qemuDomainDetachPrepDisk(virDomainObj *vm, _("This type of disk cannot be hot unplugged")); return -1; + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: default: virReportEnumRangeError(virDomainDiskBus, disk->bus); diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1ea1e4e537..1ca521321c 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1154,6 +1154,7 @@ vboxAttachDrives(virDomainDef *def, struct _vboxDriver *data, IMachine *machine) case VIR_DOMAIN_DISK_BUS_USB: case VIR_DOMAIN_DISK_BUS_UML: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: case VIR_DOMAIN_DISK_BUS_LAST: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("The vbox driver does not support %s bus type"), -- 2.30.2
Mark it explicitly as read only in accordance with the comment outlining configuration. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/vmx/vmx.c | 3 +++ tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml | 1 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 3 +++ tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 1 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 1 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 1 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml | 1 + tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml | 1 + tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml | 1 + tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 1 + tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 1 + 22 files changed, 26 insertions(+) diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 7aa76c0055..65d2850f2c 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -2411,6 +2411,9 @@ virVMXParseDisk(virVMXContext *ctx, virDomainXMLOption *xmlopt, virConf *conf, goto cleanup; } } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) { + /* set cdrom to read-only */ + (*def)->src->readonly = true; + if (fileName && virStringHasCaseSuffix(fileName, ".vmdk")) { /* * This function was called in order to parse a CDROM device, but diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml index e5a242d1f4..8e67347e7d 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-device.xml @@ -15,6 +15,7 @@ <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml index 6af0ee0704..406c3e9cc0 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml @@ -14,6 +14,7 @@ <devices> <disk type='block' device='cdrom'> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml index 93670e7eb4..0c37b64dbb 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml @@ -14,6 +14,7 @@ <devices> <disk type='file' device='cdrom'> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml index 98548e9da8..03d8eaec8e 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml @@ -15,6 +15,7 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/cdrom.iso'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml index 392d580ffa..87bf2ee847 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-auto-detect.xml @@ -15,6 +15,7 @@ <disk type='block' device='lun'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml index c7cba73aa8..764443f4fe 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-ide-raw-device.xml @@ -15,6 +15,7 @@ <disk type='block' device='lun'> <source dev='/dev/scd0'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='ide' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml index 9e5a985788..48453d4a3c 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-device.xml @@ -15,6 +15,7 @@ <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml index 3678ef38b0..a972a4f538 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml @@ -14,6 +14,7 @@ <devices> <disk type='file' device='cdrom'> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml index e79927ada8..977655cb53 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml @@ -15,6 +15,7 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/cdrom.iso'/> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml index b851e73b3b..c9445a1754 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-passthru.xml @@ -15,6 +15,7 @@ <disk type='block' device='lun'> <source dev='/vmfs/devices/cdrom/mpx.vmhba32:C0:T0:L0'/> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml index 497066972d..41f33885a6 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-auto-detect.xml @@ -15,6 +15,7 @@ <disk type='block' device='lun'> <source startupPolicy='optional'/> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml index d7d881e4a6..3d541cb14b 100644 --- a/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml +++ b/tests/vmx2xmldata/vmx2xml-cdrom-scsi-raw-device.xml @@ -15,6 +15,7 @@ <disk type='block' device='lun'> <source dev='/dev/scd0'/> <target dev='sda' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml index b079808363..59071b5d3a 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml @@ -21,16 +21,19 @@ <disk type='file' device='cdrom'> <source file='[datastore] directory/Debian1-cdrom.iso'/> <target dev='sdp' bus='scsi'/> + <readonly/> <address type='drive' controller='1' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='cdrom'> <source file='/vmimages/tools-isoimages/linux.iso'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='block' device='cdrom'> <source dev='/dev/scd0'/> <target dev='hdb' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <disk type='file' device='disk'> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml index d05318c7d8..cbe8eceb37 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml @@ -20,6 +20,7 @@ <disk type='file' device='cdrom'> <source file='[498076b2-02796c1a-ef5b-000ae484a6a3] Isos/debian-testing-amd64-netinst.iso'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <disk type='file' device='floppy'> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml index 82643e9ffe..9eb975afe9 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml @@ -27,6 +27,7 @@ <disk type='file' device='cdrom'> <source file='[4af0231d-1eff559a-6369-0024e84773b6] isos/CentOS-5.5-x86_64-bin-DVD-1of2.iso'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='lsilogic'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml index 913bfedf30..51c74dd8a1 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml @@ -20,6 +20,7 @@ <disk type='file' device='cdrom'> <source file='/usr/lib/vmware/isoimages/linux.iso'/> <target dev='hdc' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml index 91913a2918..c117bd62e5 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml @@ -20,6 +20,7 @@ <disk type='block' device='lun'> <source dev='/vmfs/devices/cdrom/mpx.vmhba32:C0:T0:L0'/> <target dev='sdb' bus='scsi'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='1'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml index 32affb5935..0eea610709 100644 --- a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml @@ -37,6 +37,7 @@ <disk type='file' device='cdrom'> <source file='[692eb778-2d4937fe] CentOS-4.7.ServerCD-x86_64.iso'/> <target dev='sda' bus='sata'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='vmpvscsi'/> diff --git a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml index f6e9f4acdf..a47fab5cd5 100644 --- a/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml +++ b/tests/vmx2xmldata/vmx2xml-fusion-in-the-wild-1.xml @@ -20,6 +20,7 @@ <disk type='block' device='cdrom'> <source startupPolicy='optional'/> <target dev='hda' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='0' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='buslogic'/> diff --git a/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml b/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml index 9901033bb9..5dcc6eb48e 100644 --- a/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml +++ b/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml @@ -20,6 +20,7 @@ <disk type='file' device='cdrom'> <source file='/usr/lib/vmware/isoimages/linux.iso'/> <target dev='hdc' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='lsilogic'/> diff --git a/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml b/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml index 6f8f30393c..dfabb16e58 100644 --- a/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml +++ b/tests/vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml @@ -20,6 +20,7 @@ <disk type='file' device='cdrom'> <source file='/usr/lib/vmware/isoimages/linux.iso'/> <target dev='hdc' bus='ide'/> + <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> <controller type='scsi' index='0' model='lsilogic'/> -- 2.30.2
Move the setting of read-only state, the default disk bus and setting of 'snapshot' state for read-only disks to the post parse callback to clean up the disk parser. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 50 +++++++++++++++++++++++------------------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c600c1e6b1..867d74f31f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5378,6 +5378,33 @@ virDomainDiskDefPostParse(virDomainDiskDef *disk, } } + /* Force CDROM to be listed as read only */ + if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM) + disk->src->readonly = true; + + if (disk->bus == VIR_DOMAIN_DISK_BUS_NONE) { + disk->bus = VIR_DOMAIN_DISK_BUS_IDE; + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { + disk->bus = VIR_DOMAIN_DISK_BUS_FDC; + } else if (disk->dst) { + if (STRPREFIX(disk->dst, "hd")) + disk->bus = VIR_DOMAIN_DISK_BUS_IDE; + else if (STRPREFIX(disk->dst, "sd")) + disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; + else if (STRPREFIX(disk->dst, "vd")) + disk->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; + else if (STRPREFIX(disk->dst, "xvd")) + disk->bus = VIR_DOMAIN_DISK_BUS_XEN; + else if (STRPREFIX(disk->dst, "ubd")) + disk->bus = VIR_DOMAIN_DISK_BUS_UML; + } + } + + if (disk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_DEFAULT && + disk->src->readonly) + disk->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; + if (disk->src->type == VIR_STORAGE_TYPE_NETWORK && disk->src->protocol == VIR_STORAGE_NET_PROTOCOL_ISCSI) { virDomainPostParseCheckISCSIPath(&disk->src->path); @@ -9465,10 +9492,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, return NULL; } - /* Force CDROM to be listed as read only */ - if (def->device == VIR_DOMAIN_DISK_DEVICE_CDROM) - def->src->readonly = true; - if ((def->device == VIR_DOMAIN_DISK_DEVICE_DISK || def->device == VIR_DOMAIN_DISK_DEVICE_LUN) && !STRPREFIX((const char *)target, "hd") && @@ -9489,8 +9512,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, snapshot); return NULL; } - } else if (def->src->readonly) { - def->snapshot = VIR_DOMAIN_SNAPSHOT_LOCATION_NONE; } if (rawio) { @@ -9516,23 +9537,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, _("unknown disk bus type '%s'"), bus); return NULL; } - } else { - if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY) { - def->bus = VIR_DOMAIN_DISK_BUS_FDC; - } else { - if (STRPREFIX(target, "hd")) - def->bus = VIR_DOMAIN_DISK_BUS_IDE; - else if (STRPREFIX(target, "sd")) - def->bus = VIR_DOMAIN_DISK_BUS_SCSI; - else if (STRPREFIX(target, "vd")) - def->bus = VIR_DOMAIN_DISK_BUS_VIRTIO; - else if (STRPREFIX(target, "xvd")) - def->bus = VIR_DOMAIN_DISK_BUS_XEN; - else if (STRPREFIX(target, "ubd")) - def->bus = VIR_DOMAIN_DISK_BUS_UML; - else - def->bus = VIR_DOMAIN_DISK_BUS_IDE; - } } if (removable) { -- 2.30.2
Move the rest of the validations to the vaidation code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 44 -------------------------------------- src/conf/domain_validate.c | 44 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 867d74f31f..ff408188d5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9440,8 +9440,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (!(wwn = virXMLNodeContentString(cur))) return NULL; - if (!virValidateWWN(wwn)) - return NULL; } else if (!vendor && virXMLNodeNameEqual(cur, "vendor")) { if (!(vendor = virXMLNodeContentString(cur))) @@ -9462,48 +9460,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - /* Only CDROM and Floppy devices are allowed missing source path - * to indicate no media present. LUN is for raw access CD-ROMs - * that are not attached to a physical device presently */ - if (virStorageSourceIsEmpty(def->src) && - def->device == VIR_DOMAIN_DISK_DEVICE_DISK) { - virReportError(VIR_ERR_NO_SOURCE, - target ? "%s" : NULL, target); - return NULL; - } - - if (!target) { - if (def->src->srcpool) { - tmp = g_strdup_printf("pool = '%s', volume = '%s'", - def->src->srcpool->pool, def->src->srcpool->volume); - - virReportError(VIR_ERR_NO_TARGET, "%s", tmp); - VIR_FREE(tmp); - } else { - virReportError(VIR_ERR_NO_TARGET, def->src->path ? "%s" : NULL, def->src->path); - } - return NULL; - } - - if (def->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && - !STRPREFIX(target, "fd")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid floppy device name: %s"), target); - return NULL; - } - - if ((def->device == VIR_DOMAIN_DISK_DEVICE_DISK || - def->device == VIR_DOMAIN_DISK_DEVICE_LUN) && - !STRPREFIX((const char *)target, "hd") && - !STRPREFIX((const char *)target, "sd") && - !STRPREFIX((const char *)target, "vd") && - !STRPREFIX((const char *)target, "xvd") && - !STRPREFIX((const char *)target, "ubd")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Invalid harddisk device name: %s"), target); - return NULL; - } - if (snapshot) { def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot); if (def->snapshot <= 0) { diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 1073da3bfa..686b9e8d16 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -699,6 +699,50 @@ virDomainDiskDefValidate(const virDomainDef *def, } } + if (disk->wwn && !virValidateWWN(disk->wwn)) + return -1; + + if (!disk->dst) { + if (disk->src->srcpool) { + virReportError(VIR_ERR_NO_TARGET, _("pool = '%s', volume = '%s'"), + disk->src->srcpool->pool, + disk->src->srcpool->volume); + } else { + virReportError(VIR_ERR_NO_TARGET, + disk->src->path ? "%s" : NULL, disk->src->path); + } + + return -1; + } + + if ((disk->device == VIR_DOMAIN_DISK_DEVICE_DISK || + disk->device == VIR_DOMAIN_DISK_DEVICE_LUN) && + !STRPREFIX(disk->dst, "hd") && + !STRPREFIX(disk->dst, "sd") && + !STRPREFIX(disk->dst, "vd") && + !STRPREFIX(disk->dst, "xvd") && + !STRPREFIX(disk->dst, "ubd")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid harddisk device name: %s"), disk->dst); + return -1; + } + + if (disk->device == VIR_DOMAIN_DISK_DEVICE_FLOPPY && + !STRPREFIX(disk->dst, "fd")) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid floppy device name: %s"), disk->dst); + return -1; + } + + /* Only CDROM and Floppy devices are allowed missing source path to + * indicate no media present. LUN is for raw access CD-ROMs that are not + * attached to a physical device presently */ + if (virStorageSourceIsEmpty(disk->src) && + disk->device == VIR_DOMAIN_DISK_DEVICE_DISK) { + virReportError(VIR_ERR_NO_SOURCE, "%s", disk->dst); + return -1; + } + return 0; } -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 15 +++++---------- src/conf/domain_conf.h | 2 +- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index ff408188d5..5e27ca6265 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9319,7 +9319,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, g_autofree char *target = NULL; g_autofree char *bus = NULL; g_autofree char *serial = NULL; - g_autofree char *removable = NULL; g_autofree char *logical_block_size = NULL; g_autofree char *physical_block_size = NULL; g_autofree char *wwn = NULL; @@ -9381,7 +9380,11 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropEnum(cur, "tray", virDomainDiskTrayTypeFromString, VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0) return NULL; - removable = virXMLPropString(cur, "removable"); + + if (virXMLPropTristateSwitch(cur, "removable", VIR_XML_PROP_OPTIONAL, + &def->removable) < 0) + return NULL; + rotation_rate = virXMLPropString(cur, "rotation_rate"); } else if (!domain_name && virXMLNodeNameEqual(cur, "backenddomain")) { @@ -9495,14 +9498,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (removable) { - if ((def->removable = virTristateSwitchTypeFromString(removable)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk removable status '%s'"), removable); - return NULL; - } - } - if (rotation_rate && virStrToLong_ui(rotation_rate, NULL, 10, &def->rotation_rate) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index cb5ce68fdb..878ba4c961 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -539,7 +539,7 @@ struct _virDomainDiskDef { int bus; /* enum virDomainDiskBus */ char *dst; virDomainDiskTray tray_status; - int removable; /* enum virTristateSwitch */ + virTristateSwitch removable; unsigned int rotation_rate; virStorageSource *mirror; -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 14 +++----------- src/conf/domain_conf.h | 2 +- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5e27ca6265..6641a7a78b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9314,7 +9314,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, bool source = false; g_autofree char *tmp = NULL; g_autofree char *snapshot = NULL; - g_autofree char *rawio = NULL; g_autofree char *sgio = NULL; g_autofree char *target = NULL; g_autofree char *bus = NULL; @@ -9357,7 +9356,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, snapshot = virXMLPropString(node, "snapshot"); - rawio = virXMLPropString(node, "rawio"); + if (virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_OPTIONAL, &def->rawio) < 0) + return NULL; + sgio = virXMLPropString(node, "sgio"); for (cur = node->children; cur != NULL; cur = cur->next) { @@ -9473,15 +9474,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (rawio) { - if ((def->rawio = virTristateBoolTypeFromString(rawio)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown disk rawio setting '%s'"), - rawio); - return NULL; - } - } - if (sgio) { if ((def->sgio = virDomainDeviceSGIOTypeFromString(sgio)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 878ba4c961..b3a91c0a4c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -577,7 +577,7 @@ struct _virDomainDiskDef { virDomainStartupPolicy startupPolicy; bool transient; virDomainDeviceInfo info; - int rawio; /* enum virTristateBool */ + virTristateBool rawio; int sgio; /* enum virDomainDeviceSGIO */ int discard; /* enum virDomainDiskDiscard */ unsigned int iothread; /* unused = 0, > 0 specific thread # */ -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 13 +++---------- src/conf/domain_conf.h | 2 +- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6641a7a78b..fd933fefc8 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9314,7 +9314,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, bool source = false; g_autofree char *tmp = NULL; g_autofree char *snapshot = NULL; - g_autofree char *sgio = NULL; g_autofree char *target = NULL; g_autofree char *bus = NULL; g_autofree char *serial = NULL; @@ -9359,7 +9358,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_OPTIONAL, &def->rawio) < 0) return NULL; - sgio = virXMLPropString(node, "sgio"); + if (virXMLPropEnum(node, "sgio", virDomainDeviceSGIOTypeFromString, + VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, &def->sgio) < 0) + return NULL; for (cur = node->children; cur != NULL; cur = cur->next) { if (cur->type != XML_ELEMENT_NODE) @@ -9474,14 +9475,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (sgio) { - if ((def->sgio = virDomainDeviceSGIOTypeFromString(sgio)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk sgio mode '%s'"), sgio); - return NULL; - } - } - if (bus) { if ((def->bus = virDomainDiskBusTypeFromString(bus)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b3a91c0a4c..1b62af6d63 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -578,7 +578,7 @@ struct _virDomainDiskDef { bool transient; virDomainDeviceInfo info; virTristateBool rawio; - int sgio; /* enum virDomainDeviceSGIO */ + virDomainDeviceSGIO sgio; int discard; /* enum virDomainDiskDiscard */ unsigned int iothread; /* unused = 0, > 0 specific thread # */ int detect_zeroes; /* enum virDomainDiskDetectZeroes */ -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 8 ++------ src/conf/domain_conf.h | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index fd933fefc8..da0e7700ff 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9345,13 +9345,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } VIR_FREE(tmp); - if ((tmp = virXMLPropString(node, "model")) && - (def->model = virDomainDiskModelTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk model '%s'"), tmp); + if (virXMLPropEnum(node, "model", virDomainDiskModelTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->model) < 0) return NULL; - } - VIR_FREE(tmp); snapshot = virXMLPropString(node, "snapshot"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1b62af6d63..a83d5b337f 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -584,7 +584,7 @@ struct _virDomainDiskDef { int detect_zeroes; /* enum virDomainDiskDetectZeroes */ char *domain_name; /* backend domain name */ unsigned int queues; - int model; /* enum virDomainDiskModel */ + virDomainDiskModel model; virDomainVirtioOptions *virtio; bool diskElementAuth; -- 2.30.2
Unfortunately virDomainSnapshotLocation is declared in snapshot_conf.h which includes domain_conf.h. To avoid a circular dependency use 'unsigned int' for now. Use XML parser can use virXMLPropEnum. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 15 +++------------ src/conf/domain_conf.h | 2 +- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index da0e7700ff..6f7948da0a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9313,7 +9313,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, VIR_XPATH_NODE_AUTORESTORE(ctxt) bool source = false; g_autofree char *tmp = NULL; - g_autofree char *snapshot = NULL; g_autofree char *target = NULL; g_autofree char *bus = NULL; g_autofree char *serial = NULL; @@ -9349,7 +9348,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, VIR_XML_PROP_OPTIONAL, &def->model) < 0) return NULL; - snapshot = virXMLPropString(node, "snapshot"); + if (virXMLPropEnum(node, "snapshot", virDomainSnapshotLocationTypeFromString, + VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, &def->snapshot) < 0) + return NULL; if (virXMLPropTristateBool(node, "rawio", VIR_XML_PROP_OPTIONAL, &def->rawio) < 0) return NULL; @@ -9461,16 +9462,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (snapshot) { - def->snapshot = virDomainSnapshotLocationTypeFromString(snapshot); - if (def->snapshot <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk snapshot setting '%s'"), - snapshot); - return NULL; - } - } - if (bus) { if ((def->bus = virDomainDiskBusTypeFromString(bus)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a83d5b337f..17d830a822 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -573,7 +573,7 @@ struct _virDomainDiskDef { virTristateSwitch ioeventfd; virTristateSwitch event_idx; virTristateSwitch copy_on_read; - int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */ + unsigned int snapshot; /* virDomainSnapshotLocation, snapshot_conf.h */ virDomainStartupPolicy startupPolicy; bool transient; virDomainDeviceInfo info; -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Changes to other places using switch statements were required. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_command.c | 9 +++++++++ src/bhyve/bhyve_domain.c | 12 ++++++++++++ src/conf/domain_conf.c | 26 ++++++++++++++++---------- src/conf/domain_conf.h | 2 +- src/hyperv/hyperv_driver.c | 16 ++++++++++++++++ src/qemu/qemu_validate.c | 3 +++ src/vz/vz_sdk.c | 22 +++++++++++++++++++--- src/vz/vz_utils.c | 8 ++++++++ 8 files changed, 84 insertions(+), 14 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index b1558132e1..9731fee22f 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -309,6 +309,15 @@ bhyveBuildDiskArgStr(const virDomainDef *def, if (bhyveBuildVirtIODiskArgStr(def, disk, cmd) < 0) return -1; break; + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk device")); diff --git a/src/bhyve/bhyve_domain.c b/src/bhyve/bhyve_domain.c index 0213878f26..33e74e2e25 100644 --- a/src/bhyve/bhyve_domain.c +++ b/src/bhyve/bhyve_domain.c @@ -133,6 +133,18 @@ bhyveDomainDiskDefAssignAddress(struct _bhyveConn *driver, def->info.addr.drive.bus = 0; break; + case VIR_DOMAIN_DISK_BUS_SCSI: + case VIR_DOMAIN_DISK_BUS_IDE: + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: + default: + break; } return 0; } diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6f7948da0a..9afb548ea7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7874,6 +7874,13 @@ virDomainDiskDefAssignAddress(virDomainXMLOption *xmlopt, def->info.addr.drive.unit = idx % 2; break; + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: /* Other disk bus's aren't controller based */ break; @@ -9314,7 +9321,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, bool source = false; g_autofree char *tmp = NULL; g_autofree char *target = NULL; - g_autofree char *bus = NULL; g_autofree char *serial = NULL; g_autofree char *logical_block_size = NULL; g_autofree char *physical_block_size = NULL; @@ -9375,7 +9381,11 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } else if (!target && virXMLNodeNameEqual(cur, "target")) { target = virXMLPropString(cur, "dev"); - bus = virXMLPropString(cur, "bus"); + if (virXMLPropEnum(cur, "bus", + virDomainDiskBusTypeFromString, + VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, + &def->bus) < 0) + return NULL; if (virXMLPropEnum(cur, "tray", virDomainDiskTrayTypeFromString, VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0) return NULL; @@ -9462,14 +9472,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, } } - if (bus) { - if ((def->bus = virDomainDiskBusTypeFromString(bus)) <= 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk bus type '%s'"), bus); - return NULL; - } - } - if (rotation_rate && virStrToLong_ui(rotation_rate, NULL, 10, &def->rotation_rate) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -30042,6 +30044,10 @@ virDiskNameToBusDeviceIndex(virDomainDiskDef *disk, case VIR_DOMAIN_DISK_BUS_VIRTIO: case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_LAST: default: *busIdx = 0; *devIdx = idx; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 17d830a822..29866927c7 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -536,7 +536,7 @@ struct _virDomainDiskDef { virObject *privateData; int device; /* enum virDomainDiskDevice */ - int bus; /* enum virDomainDiskBus */ + virDomainDiskBus bus; char *dst; virDomainDiskTray tray_status; virTristateSwitch removable; diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 0ac4f93d13..3a0eeb5178 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -961,6 +961,14 @@ hypervDomainAttachStorage(virDomainPtr domain, virDomainDef *def, const char *ho if (hypervDomainAttachFloppy(domain, def->disks[i], floppySettings, hostname) < 0) return -1; break; + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unsupported controller type")); return -1; @@ -3108,6 +3116,14 @@ hypervDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int if (!entry) return -1; break; + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SATA: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Invalid disk bus in definition")); return -1; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 18c851d693..44d20088bf 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -2755,6 +2755,9 @@ qemuValidateDomainDeviceDefDiskFrontend(const virDomainDiskDef *disk, case VIR_DOMAIN_DISK_BUS_XEN: case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_LAST: break; } diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 048d4adacd..e09950812d 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -595,7 +595,7 @@ prlsdkAddDomainVideoInfoVm(PRL_HANDLE sdkdom, virDomainDef *def) } static int -prlsdkGetDiskId(PRL_HANDLE disk, int *bus, char **dst) +prlsdkGetDiskId(PRL_HANDLE disk, virDomainDiskBus *bus, char **dst) { PRL_RESULT pret; PRL_UINT32 pos, ifType; @@ -1624,7 +1624,7 @@ prlsdkBootOrderCheck(PRL_HANDLE sdkdom, PRL_DEVICE_TYPE sdkType, int sdkIndex, PRL_HANDLE dev = PRL_INVALID_HANDLE; virDomainDiskDef *disk; virDomainDiskDevice device; - int bus; + virDomainDiskBus bus; char *dst = NULL; int ret = -1; @@ -3462,6 +3462,14 @@ static int prlsdkConfigureDisk(struct _vzDriver *driver, sdkbus = PMS_SATA_DEVICE; idx = drive->unit; break; + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified disk bus is not " @@ -3500,7 +3508,7 @@ prlsdkGetDisk(PRL_HANDLE sdkdom, virDomainDiskDef *disk) PRL_UINT32 num; size_t i; PRL_HANDLE sdkdisk = PRL_INVALID_HANDLE; - int bus; + virDomainDiskBus bus; char *dst = NULL; PRL_DEVICE_TYPE devType; @@ -4339,6 +4347,14 @@ prlsdkGetBlockStats(PRL_HANDLE sdkstats, prefix = "scsi"; idx = address->unit; break; + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown disk bus: %X"), disk->bus); diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index a07754d5ec..8fed875281 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -237,6 +237,14 @@ vzCheckDiskAddressDriveUnsupportedParams(virDomainDiskDef *disk) return -1; } break; + case VIR_DOMAIN_DISK_BUS_FDC: + case VIR_DOMAIN_DISK_BUS_NONE: + case VIR_DOMAIN_DISK_BUS_VIRTIO: + case VIR_DOMAIN_DISK_BUS_XEN: + case VIR_DOMAIN_DISK_BUS_USB: + case VIR_DOMAIN_DISK_BUS_UML: + case VIR_DOMAIN_DISK_BUS_SD: + case VIR_DOMAIN_DISK_BUS_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Specified disk bus is not supported by vz driver.")); -- 2.30.2
Use the appropriate type for the variable and refactor the XML parser to parse it correctly using virXMLPropEnum. Changes to other places using switch statements were required. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/bhyve/bhyve_command.c | 3 +++ src/conf/domain_conf.c | 9 ++------- src/conf/domain_conf.h | 2 +- src/hyperv/hyperv_driver.c | 2 ++ src/libxl/libxl_driver.c | 11 +++++++++++ src/qemu/qemu_command.c | 4 ++++ src/vmx/vmx.c | 1 + 7 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 9731fee22f..f8e0ce5123 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -208,6 +208,9 @@ bhyveBuildAHCIControllerArgStr(const virDomainDef *def, else virBufferAsprintf(&device, "-cd,%s", disk_source); break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("unsupported disk device")); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 9afb548ea7..242839d60f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9319,7 +9319,6 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt) bool source = false; - g_autofree char *tmp = NULL; g_autofree char *target = NULL; g_autofree char *serial = NULL; g_autofree char *logical_block_size = NULL; @@ -9342,13 +9341,9 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, /* defaults */ def->device = VIR_DOMAIN_DISK_DEVICE_DISK; - if ((tmp = virXMLPropString(node, "device")) && - (def->device = virDomainDiskDeviceTypeFromString(tmp)) < 0) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("unknown disk device '%s'"), tmp); + if (virXMLPropEnum(node, "device", virDomainDiskDeviceTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->device) < 0) return NULL; - } - VIR_FREE(tmp); if (virXMLPropEnum(node, "model", virDomainDiskModelTypeFromString, VIR_XML_PROP_OPTIONAL, &def->model) < 0) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 29866927c7..a17f241c53 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -535,7 +535,7 @@ struct _virDomainDiskDef { virObject *privateData; - int device; /* enum virDomainDiskDevice */ + virDomainDiskDevice device; virDomainDiskBus bus; char *dst; virDomainDiskTray tray_status; diff --git a/src/hyperv/hyperv_driver.c b/src/hyperv/hyperv_driver.c index 3a0eeb5178..ff20d5548b 100644 --- a/src/hyperv/hyperv_driver.c +++ b/src/hyperv/hyperv_driver.c @@ -886,6 +886,8 @@ hypervDomainAttachStorageVolume(virDomainPtr domain, return hypervDomainAttachCDROM(domain, disk, controller, hostname); case VIR_DOMAIN_DISK_DEVICE_FLOPPY: return hypervDomainAttachFloppy(domain, disk, controller, hostname); + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unsupported disk bus")); break; diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index d924e033d9..cf3ee4db3d 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -3084,6 +3084,9 @@ libxlDomainAttachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) virDomainDiskBusTypeToString(l_disk->bus)); } break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk device type '%s' cannot be hotplugged"), @@ -3369,6 +3372,10 @@ libxlDomainDetachDeviceDiskLive(virDomainObj *vm, virDomainDeviceDef *dev) virDomainDiskBusTypeToString(dev->data.disk->bus)); } break; + case VIR_DOMAIN_DISK_DEVICE_CDROM: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("device type '%s' cannot hot unplugged"), @@ -4031,6 +4038,10 @@ libxlDomainUpdateDeviceLive(virDomainObj *vm, virDomainDeviceDef *dev) if (ret == 0) dev->data.disk = NULL; break; + case VIR_DOMAIN_DISK_DEVICE_DISK: + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LUN: + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("disk bus '%s' cannot be updated."), diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 6ac36da1bb..e55858bb79 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2252,6 +2252,10 @@ qemuBuildDisksCommandLine(virCommand *cmd, bootindex = bootDisk; bootDisk = 0; break; + case VIR_DOMAIN_DISK_DEVICE_FLOPPY: + case VIR_DOMAIN_DISK_DEVICE_LAST: + default: + break; } } diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 65d2850f2c..1cd5a82227 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -3474,6 +3474,7 @@ virVMXFormatConfig(virVMXContext *ctx, virDomainXMLOption *xmlopt, virDomainDef break; + case VIR_DOMAIN_DISK_DEVICE_LAST: default: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported disk device type '%s'"), -- 2.30.2
Use the new virXMLProp helpers and XPath queries to get rid of the old style of iteration through element children. Note that in case of def->blockio.logical_block_size, def->blockio.physical_block_size and def->rotation_rate the wraparound behaviour of 'virStrToLong_ui' was _not_ forward ported to the new code as it makes no sense with the attributes. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 196 +++++++++++++++++------------------------ 1 file changed, 80 insertions(+), 116 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 242839d60f..a36d0a2713 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9316,18 +9316,13 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, unsigned int flags) { g_autoptr(virDomainDiskDef) def = NULL; - xmlNodePtr cur; VIR_XPATH_NODE_AUTORESTORE(ctxt) - bool source = false; - g_autofree char *target = NULL; - g_autofree char *serial = NULL; - g_autofree char *logical_block_size = NULL; - g_autofree char *physical_block_size = NULL; - g_autofree char *wwn = NULL; - g_autofree char *vendor = NULL; - g_autofree char *product = NULL; - g_autofree char *domain_name = NULL; - g_autofree char *rotation_rate = NULL; + xmlNodePtr sourceNode; + xmlNodePtr targetNode; + xmlNodePtr geometryNode; + xmlNodePtr blockioNode; + xmlNodePtr driverNode; + xmlNodePtr mirrorNode; g_autoptr(virStorageSource) src = NULL; if (!(src = virDomainDiskDefParseSourceXML(xmlopt, node, ctxt, flags))) @@ -9360,132 +9355,101 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt, VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, &def->sgio) < 0) return NULL; - for (cur = node->children; cur != NULL; cur = cur->next) { - if (cur->type != XML_ELEMENT_NODE) - continue; + if ((sourceNode = virXPathNode("./source", ctxt))) { + if (virXMLPropEnum(sourceNode, "startupPolicy", + virDomainStartupPolicyTypeFromString, + VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, + &def->startupPolicy) < 0) + return NULL; + } - if (!source && virXMLNodeNameEqual(cur, "source")) { - source = true; + if ((targetNode = virXPathNode("./target", ctxt))) { + def->dst = virXMLPropString(targetNode, "dev"); - if (virXMLPropEnum(cur, "startupPolicy", - virDomainStartupPolicyTypeFromString, - VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, - &def->startupPolicy) < 0) - return NULL; + if (virXMLPropEnum(targetNode, "bus", + virDomainDiskBusTypeFromString, + VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, + &def->bus) < 0) + return NULL; - } else if (!target && - virXMLNodeNameEqual(cur, "target")) { - target = virXMLPropString(cur, "dev"); - if (virXMLPropEnum(cur, "bus", - virDomainDiskBusTypeFromString, - VIR_XML_PROP_OPTIONAL | VIR_XML_PROP_NONZERO, - &def->bus) < 0) - return NULL; - if (virXMLPropEnum(cur, "tray", virDomainDiskTrayTypeFromString, - VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0) - return NULL; + if (virXMLPropEnum(targetNode, "tray", virDomainDiskTrayTypeFromString, + VIR_XML_PROP_OPTIONAL, &def->tray_status) < 0) + return NULL; - if (virXMLPropTristateSwitch(cur, "removable", VIR_XML_PROP_OPTIONAL, - &def->removable) < 0) - return NULL; + if (virXMLPropTristateSwitch(targetNode, "removable", VIR_XML_PROP_OPTIONAL, + &def->removable) < 0) + return NULL; - rotation_rate = virXMLPropString(cur, "rotation_rate"); - } else if (!domain_name && - virXMLNodeNameEqual(cur, "backenddomain")) { - domain_name = virXMLPropString(cur, "name"); - } else if (virXMLNodeNameEqual(cur, "geometry")) { - if (virDomainDiskDefGeometryParse(def, cur) < 0) - return NULL; - } else if (virXMLNodeNameEqual(cur, "blockio")) { - logical_block_size = - virXMLPropString(cur, "logical_block_size"); - if (logical_block_size && - virStrToLong_ui(logical_block_size, NULL, 0, - &def->blockio.logical_block_size) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid logical block size '%s'"), - logical_block_size); - return NULL; - } - physical_block_size = - virXMLPropString(cur, "physical_block_size"); - if (physical_block_size && - virStrToLong_ui(physical_block_size, NULL, 0, - &def->blockio.physical_block_size) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("invalid physical block size '%s'"), - physical_block_size); - return NULL; - } - } else if (!virDomainDiskGetDriver(def) && - virXMLNodeNameEqual(cur, "driver")) { - if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0) - return NULL; + if (virXMLPropUInt(targetNode, "rotation_rate", 10, VIR_XML_PROP_OPTIONAL, + &def->rotation_rate) < 0) + return NULL; + } - if (virDomainDiskDefDriverParseXML(def, cur, ctxt) < 0) - return NULL; - } else if (!def->mirror && - virXMLNodeNameEqual(cur, "mirror") && - !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { - if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0) - return NULL; - } else if (virXMLNodeNameEqual(cur, "auth")) { - def->diskElementAuth = true; - } else if (virXMLNodeNameEqual(cur, "iotune")) { - if (virDomainDiskDefIotuneParse(def, ctxt) < 0) - return NULL; - } else if (virXMLNodeNameEqual(cur, "transient")) { - def->transient = true; - } else if (virXMLNodeNameEqual(cur, "encryption")) { - def->diskElementEnc = true; - } else if (!serial && - virXMLNodeNameEqual(cur, "serial")) { - if (!(serial = virXMLNodeContentString(cur))) - return NULL; - } else if (!wwn && - virXMLNodeNameEqual(cur, "wwn")) { - if (!(wwn = virXMLNodeContentString(cur))) - return NULL; + if ((geometryNode = virXPathNode("./geometry", ctxt))) { + if (virDomainDiskDefGeometryParse(def, geometryNode) < 0) + return NULL; + } - } else if (!vendor && - virXMLNodeNameEqual(cur, "vendor")) { - if (!(vendor = virXMLNodeContentString(cur))) - return NULL; - } else if (!product && - virXMLNodeNameEqual(cur, "product")) { - if (!(product = virXMLNodeContentString(cur))) + if ((blockioNode = virXPathNode("./blockio", ctxt))) { + if (virXMLPropUInt(blockioNode, "logical_block_size", 10, VIR_XML_PROP_OPTIONAL, + &def->blockio.logical_block_size) < 0) + return NULL; + + if (virXMLPropUInt(blockioNode, "physical_block_size", 10, VIR_XML_PROP_OPTIONAL, + &def->blockio.physical_block_size) < 0) + return NULL; + } + + if ((driverNode = virXPathNode("./driver", ctxt))) { + if (virDomainVirtioOptionsParseXML(driverNode, &def->virtio) < 0) + return NULL; + + if (virDomainDiskDefDriverParseXML(def, driverNode, ctxt) < 0) + return NULL; + } + + if ((mirrorNode = virXPathNode("./mirror", ctxt))) { + if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) { + if (virDomainDiskDefMirrorParse(def, mirrorNode, ctxt, flags, xmlopt) < 0) return NULL; - } else if (virXMLNodeNameEqual(cur, "boot")) { - /* boot is parsed as part of virDomainDeviceInfoParseXML */ - } else if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) && - virXMLNodeNameEqual(cur, "diskSecretsPlacement")) { - g_autofree char *secretAuth = virXMLPropString(cur, "auth"); - g_autofree char *secretEnc = virXMLPropString(cur, "enc"); + } + } + + if (virXPathNode("./auth", ctxt)) + def->diskElementAuth = true; + + if (virXPathNode("./encryption", ctxt)) + def->diskElementEnc = true; + + if (flags & VIR_DOMAIN_DEF_PARSE_STATUS) { + xmlNodePtr diskSecretsPlacementNode; + + if ((diskSecretsPlacementNode = virXPathNode("./diskSecretsPlacement", ctxt))) { + g_autofree char *secretAuth = virXMLPropString(diskSecretsPlacementNode, "auth"); + g_autofree char *secretEnc = virXMLPropString(diskSecretsPlacementNode, "enc"); def->diskElementAuth = !!secretAuth; def->diskElementEnc = !!secretEnc; } } - if (rotation_rate && - virStrToLong_ui(rotation_rate, NULL, 10, &def->rotation_rate) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Cannot parse rotation rate '%s'"), rotation_rate); + if (virXPathNode("./transient", ctxt)) + def->transient = true; + + if (virDomainDiskDefIotuneParse(def, ctxt) < 0) return NULL; - } + + def->domain_name = virXPathString("string(./backenddomain/@name)", ctxt); + def->serial = virXPathString("string(./serial)", ctxt); + def->wwn = virXPathString("string(./wwn)", ctxt); + def->vendor = virXPathString("string(./vendor)", ctxt); + def->product = virXPathString("string(./product)", ctxt); if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags | VIR_DOMAIN_DEF_PARSE_ALLOW_BOOT) < 0) { return NULL; } - def->dst = g_steal_pointer(&target); - def->domain_name = g_steal_pointer(&domain_name); - def->serial = g_steal_pointer(&serial); - def->wwn = g_steal_pointer(&wwn); - def->vendor = g_steal_pointer(&vendor); - def->product = g_steal_pointer(&product); - if (flags & VIR_DOMAIN_DEF_PARSE_STATUS && virDomainDiskDefParsePrivateData(ctxt, def, xmlopt) < 0) return NULL; -- 2.30.2
On 4/16/21 5:34 PM, Peter Krempa wrote:
Split out all default setting and validation code into appropriate functions and convert the XML parser to contemporary helpers.
Peter Krempa (25): virXMLParseHelper: Add root XML node name validation capability util: xml: Introduce virXMLParseStringCtxtRoot conf: domain: Introduce virDomainDiskDefParseSource qemu: driver: Use virDomainDiskDefParseSource in qemuDomainBlockCopy conf: domain: Introduce an internal variant of virDomainDiskDefNew conf: domain: Split out source validation part from virDomainDiskDefParseValidate conf: domain: Split out parsing of source data from <disk> XML parser conf: domain: Remove VIR_DOMAIN_DEF_PARSE_DISK_SOURCE parser flag virDomainDiskDefValidate: Consolidate conditions conf: Move code from virDomainDiskDefParseValidate to virDomainDiskDefValidate conf: Move disk vendor and product pritability check to domain_validate conf: Move checks from virDomainDiskDefPostParse to virDomainDiskDefValidate conf: Move disk target 'ioemu:' stripping to virDomainDiskDefPostParse conf: domain: Introduce VIR_DOMAIN_DISK_BUS_NONE vmx: Mark CDROM disk elements as read-only conf: domain: Move default setting from virDomainDiskDefParseXML to virDomainDiskDefPostParse conf: domain: Move checks from virDomainDiskDefParseXML to virDomainDiskDefValidate conf: domain: Convert virDomainDiskDef's 'removable' to virTristateSwitch conf: domain: Convert virDomainDiskDef's 'rawio' to virTristateBool conf: domain: Convert virDomainDiskDef's 'sgio' to virDomainDeviceSGIO conf: domain: Convert virDomainDiskDef's 'model' to virDomainDiskModel conf: domain: Convert virDomainDiskDef's 'snapshot' to unsigned int conf: domain: Convert virDomainDiskDef's 'bus' to virDomainDiskBus conf: domain: Convert virDomainDiskDef's 'device' to virDomainDiskDevice conf: domain: Refactor virDomainDiskDefParseXML
src/bhyve/bhyve_command.c | 12 + src/bhyve/bhyve_domain.c | 12 + src/conf/domain_conf.c | 715 ++++++------------ src/conf/domain_conf.h | 32 +- src/conf/domain_validate.c | 245 +++++- src/conf/domain_validate.h | 2 + src/conf/storage_conf.c | 16 +- src/hyperv/hyperv_driver.c | 18 + src/libvirt_private.syms | 1 + src/libxl/libxl_driver.c | 11 + src/qemu/qemu_alias.c | 1 + src/qemu/qemu_command.c | 6 + src/qemu/qemu_domain_address.c | 1 + src/qemu/qemu_driver.c | 9 +- src/qemu/qemu_hotplug.c | 2 + src/qemu/qemu_validate.c | 3 + src/util/virxml.c | 18 +- src/util/virxml.h | 17 +- src/vbox/vbox_common.c | 1 + src/vmx/vmx.c | 4 + src/vz/vz_sdk.c | 22 +- src/vz/vz_utils.c | 8 + .../vmx2xmldata/vmx2xml-cdrom-ide-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-ide-empty-2.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-ide-file.xml | 1 + .../vmx2xml-cdrom-ide-raw-auto-detect.xml | 1 + .../vmx2xml-cdrom-ide-raw-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-scsi-device.xml | 1 + .../vmx2xmldata/vmx2xml-cdrom-scsi-empty.xml | 1 + tests/vmx2xmldata/vmx2xml-cdrom-scsi-file.xml | 1 + .../vmx2xml-cdrom-scsi-passthru.xml | 1 + .../vmx2xml-cdrom-scsi-raw-auto-detect.xml | 1 + .../vmx2xml-cdrom-scsi-raw-device.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-2.xml | 3 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-3.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-5.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-7.xml | 1 + .../vmx2xmldata/vmx2xml-esx-in-the-wild-8.xml | 1 + .../vmx2xml-fusion-in-the-wild-1.xml | 1 + .../vmx2xmldata/vmx2xml-ws-in-the-wild-1.xml | 1 + .../vmx2xmldata/vmx2xml-ws-in-the-wild-2.xml | 1 + 43 files changed, 645 insertions(+), 534 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa