From e72b3f0bbee40017fa010e5c2db4f56d5b5ce6c2 Mon Sep 17 00:00:00 2001 From: Peter Krempa Date: Fri, 11 May 2018 15:50:57 +0200 Subject: [PATCH] util: storage: Drop pointless 'enabled' form PR definition Everything can be disabled by not using the parent element. There's no need to store this explicitly. Additionally it does not add any value since any configuration is dropped if enabled='no' is configured. Drop the attribute and adjust the code accordingly. Signed-off-by: Peter Krempa --- docs/formatdomain.html.in | 24 ++-- docs/schemas/storagecommon.rng | 3 - src/util/virstoragefile.c | 117 +++++++----------- src/util/virstoragefile.h | 1 - .../disk-virtio-scsi-reservations.xml | 4 +- 5 files changed, 61 insertions(+), 88 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index fdff0aac97..0d0fd3b9f3 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2583,7 +2583,7 @@ <disk type='block' device='lun'> <driver name='qemu' type='raw'/> <source dev='/dev/sda'> - <reservations enabled='yes' managed='no'> + <reservations managed='no'> <source type='unix' path='/path/to/qemu-pr-helper' mode='client'/> </reservations> <target dev='sda' bus='scsi'/> @@ -2952,19 +2952,19 @@
Since libvirt 4.4.0, the reservations can be a sub-element of the source element for storage sources (QEMU driver only). - If present (and enabled) it enables persistent reservations for SCSI + If present it enables persistent reservations for SCSI based disks. The element has one mandatory attribute - enabled with accepted values yes and - no. If the feature is enabled, then there's another - mandatory attribute managed (accepted values are the - same as for enabled) that enables or disables libvirt - spawning a helper process. When the PR is unmanaged, then hypervisor - acts as a client and path to server socket must be provided in child - element source, which currently accepts only the - following attributes: type with one value - unix, path with path the socket, and + managed with accepted values yes and + no. If managed is enabled libvirt prepares + and manages any resources needed. When the persistent reservations + are unmanaged, then the hypervisor acts as a client and the path to + the server socket must be provided in the child element + source, which currently accepts only the following + attributes: + type with one value unix, + path path to the socket, and finally mode which accepts one value - client and specifies the role of hypervisor. + client specifying the role of hypervisor. It's recommended to allow libvirt manage the persistent reservations.
diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng index eed0b33347..cb4f14f52f 100644 --- a/docs/schemas/storagecommon.rng +++ b/docs/schemas/storagecommon.rng @@ -75,9 +75,6 @@ - - - diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 87c3499561..d6907e47bb 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -1906,8 +1906,8 @@ virStoragePRDefFree(virStoragePRDefPtr prd) virStoragePRDefPtr virStoragePRDefParseXML(xmlXPathContextPtr ctxt) { - virStoragePRDefPtr prd, ret = NULL; - char *enabled = NULL; + virStoragePRDefPtr prd; + virStoragePRDefPtr ret = NULL; char *managed = NULL; char *type = NULL; char *path = NULL; @@ -1916,81 +1916,65 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt) if (VIR_ALLOC(prd) < 0) return NULL; - if (!(enabled = virXPathString("string(./@enabled)", ctxt))) { + if (!(managed = virXPathString("string(./@managed)", ctxt))) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing @enabled attribute for ")); + _("missing @managed attribute for ")); goto cleanup; } - if ((prd->enabled = virTristateBoolTypeFromString(enabled)) <= 0) { + if ((prd->managed = virTristateBoolTypeFromString(managed)) <= 0) { virReportError(VIR_ERR_XML_ERROR, - _("invalid value for 'enabled': %s"), enabled); + _("invalid value for 'managed': %s"), managed); goto cleanup; } - if (prd->enabled == VIR_TRISTATE_BOOL_YES) { - if (!(managed = virXPathString("string(./@managed)", ctxt))) { + if (prd->managed == VIR_TRISTATE_BOOL_NO) { + type = virXPathString("string(./source[1]/@type)", ctxt); + path = virXPathString("string(./source[1]/@path)", ctxt); + mode = virXPathString("string(./source[1]/@mode)", ctxt); + + if (!type) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing @managed attribute for ")); + _("missing connection type for ")); goto cleanup; } - if ((prd->managed = virTristateBoolTypeFromString(managed)) <= 0) { - virReportError(VIR_ERR_XML_ERROR, - _("invalid value for 'managed': %s"), managed); + if (!path) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing path for ")); goto cleanup; } - if (prd->managed == VIR_TRISTATE_BOOL_NO) { - type = virXPathString("string(./source[1]/@type)", ctxt); - path = virXPathString("string(./source[1]/@path)", ctxt); - mode = virXPathString("string(./source[1]/@mode)", ctxt); - - if (!type) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing connection type for ")); - goto cleanup; - } - - if (!path) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing path for ")); - goto cleanup; - } - - if (!mode) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing connection mode for ")); - goto cleanup; - } - - if (STRNEQ(type, "unix")) { - virReportError(VIR_ERR_XML_ERROR, - _("unsupported connection type for : %s"), - type); - goto cleanup; - } + if (!mode) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing connection mode for ")); + goto cleanup; + } - if (STRNEQ(mode, "client")) { - virReportError(VIR_ERR_XML_ERROR, - _("unsupported connection mode for : %s"), - mode); - goto cleanup; - } + if (STRNEQ(type, "unix")) { + virReportError(VIR_ERR_XML_ERROR, + _("unsupported connection type for : %s"), + type); + goto cleanup; + } - VIR_STEAL_PTR(prd->path, path); + if (STRNEQ(mode, "client")) { + virReportError(VIR_ERR_XML_ERROR, + _("unsupported connection mode for : %s"), + mode); + goto cleanup; } + + VIR_STEAL_PTR(prd->path, path); } - ret = prd; - prd = NULL; + VIR_STEAL_PTR(ret, prd); cleanup: VIR_FREE(mode); VIR_FREE(path); VIR_FREE(type); VIR_FREE(managed); - VIR_FREE(enabled); virStoragePRDefFree(prd); return ret; } @@ -2000,22 +1984,16 @@ void virStoragePRDefFormat(virBufferPtr buf, virStoragePRDefPtr prd) { - virBufferAsprintf(buf, "enabled)); - if (prd->enabled == VIR_TRISTATE_BOOL_YES) { - virBufferAsprintf(buf, " managed='%s'", - virTristateBoolTypeToString(prd->managed)); - if (prd->managed == VIR_TRISTATE_BOOL_NO) { - virBufferAddLit(buf, ">\n"); - virBufferAdjustIndent(buf, 2); - virBufferAddLit(buf, "path); - virBufferAddLit(buf, " mode='client'/>\n"); - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "\n"); - } else { - virBufferAddLit(buf, "/>\n"); - } + virBufferAsprintf(buf, "managed)); + if (prd->managed == VIR_TRISTATE_BOOL_NO) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virBufferAddLit(buf, "path); + virBufferAddLit(buf, " mode='client'/>\n"); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "\n"); } else { virBufferAddLit(buf, "/>\n"); } @@ -2032,8 +2010,7 @@ virStoragePRDefIsEqual(virStoragePRDefPtr a, if (!a || !b) return false; - if (a->enabled != b->enabled || - a->managed != b->managed || + if (a->managed != b->managed || STRNEQ_NULLABLE(a->path, b->path)) return false; @@ -2044,7 +2021,7 @@ virStoragePRDefIsEqual(virStoragePRDefPtr a, bool virStoragePRDefIsEnabled(virStoragePRDefPtr prd) { - return prd && prd->enabled == VIR_TRISTATE_BOOL_YES; + return !!prd; } diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 0bba016e4e..ec49152880 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -219,7 +219,6 @@ struct _virStorageAuthDef { typedef struct _virStoragePRDef virStoragePRDef; typedef virStoragePRDef *virStoragePRDefPtr; struct _virStoragePRDef { - int enabled; /* enum virTristateBool */ int managed; /* enum virTristateBool */ char *path; }; diff --git a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml index 036c6e3c25..acad600ef8 100644 --- a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml +++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml @@ -17,7 +17,7 @@ - +
@@ -25,7 +25,7 @@ - + -- GitLab