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 <pkrempa@redhat.com>

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fdff0aac978b902bd132a744c3559fe8f7a6cd55..0d0fd3b9f3eae39a0ad230247f8a5e79789401aa 100644 (file)
--- 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 @@
           <dd><span class="since">Since libvirt 4.4.0</span>, the
             <code>reservations</code> can be a sub-element of the
             <code>source</code> 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
-            <code>enabled</code> with accepted values <code>yes</code> and
-            <code>no</code>. If the feature is enabled, then there's another
-            mandatory attribute <code>managed</code> (accepted values are the
-            same as for <code>enabled</code>) 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 <code>source</code>, which currently accepts only the
-            following attributes: <code>type</code> with one value
-            <code>unix</code>, <code>path</code> with path the socket, and
+            <code>managed</code> with accepted values <code>yes</code> and
+            <code>no</code>. If <code>managed</code> 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
+            <code>source</code>, which currently accepts only the following
+            attributes:
+            <code>type</code> with one value <code>unix</code>,
+            <code>path</code> path to the socket, and
             finally <code>mode</code> which accepts one value
-            <code>client</code> and specifies the role of hypervisor.
+            <code>client</code> specifying the role of hypervisor.
             It's recommended to allow libvirt manage the persistent
             reservations.
           </dd>

diff --git a/docs/schemas/storagecommon.rng b/docs/schemas/storagecommon.rng
index eed0b3334742b70d7439d7151e80189ab5e10c0a..cb4f14f52fd89abf7782804470dc52664fac8fe0 100644 (file)
--- a/docs/schemas/storagecommon.rng
+++ b/docs/schemas/storagecommon.rng
@@ -75,9 +75,6 @@
 
   <define name='reservations'>
     <element name='reservations'>
-      <attribute name='enabled'>
-        <ref name='virYesNo'/>
-      </attribute>
       <optional>
         <attribute name='managed'>
           <ref name='virYesNo'/>

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 87c34995614d825888f9e83c8f0d121c91bf0d53..d6907e47bbb8f7d3e153696fc6475e1ba2982fba 100644 (file)
--- 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 <reservations/>"));
+                       _("missing @managed attribute for <reservations/>"));
         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 <reservations/>"));
+                           _("missing connection type for <reservations/>"));
             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 <reservations/>"));
             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 <reservations/>"));
-                goto cleanup;
-            }
-
-            if (!path) {
-                virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("missing path for <reservations/>"));
-                goto cleanup;
-            }
-
-            if (!mode) {
-                virReportError(VIR_ERR_XML_ERROR, "%s",
-                               _("missing connection mode for <reservations/>"));
-                goto cleanup;
-            }
-
-            if (STRNEQ(type, "unix")) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("unsupported connection type for <reservations/>: %s"),
-                               type);
-                goto cleanup;
-            }
+        if (!mode) {
+            virReportError(VIR_ERR_XML_ERROR, "%s",
+                           _("missing connection mode for <reservations/>"));
+            goto cleanup;
+        }
 
-            if (STRNEQ(mode, "client")) {
-                virReportError(VIR_ERR_XML_ERROR,
-                               _("unsupported connection mode for <reservations/>: %s"),
-                               mode);
-                goto cleanup;
-            }
+        if (STRNEQ(type, "unix")) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unsupported connection type for <reservations/>: %s"),
+                           type);
+            goto cleanup;
+        }
 
-            VIR_STEAL_PTR(prd->path, path);
+        if (STRNEQ(mode, "client")) {
+            virReportError(VIR_ERR_XML_ERROR,
+                           _("unsupported connection mode for <reservations/>: %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, "<reservations enabled='%s'",
-                      virTristateBoolTypeToString(prd->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, "<source type='unix'");
-            virBufferEscapeString(buf, " path='%s'", prd->path);
-            virBufferAddLit(buf, " mode='client'/>\n");
-            virBufferAdjustIndent(buf, -2);
-            virBufferAddLit(buf, "</reservations>\n");
-        } else {
-            virBufferAddLit(buf, "/>\n");
-        }
+    virBufferAsprintf(buf, "<reservations managed='%s'",
+                      virTristateBoolTypeToString(prd->managed));
+    if (prd->managed == VIR_TRISTATE_BOOL_NO) {
+        virBufferAddLit(buf, ">\n");
+        virBufferAdjustIndent(buf, 2);
+        virBufferAddLit(buf, "<source type='unix'");
+        virBufferEscapeString(buf, " path='%s'", prd->path);
+        virBufferAddLit(buf, " mode='client'/>\n");
+        virBufferAdjustIndent(buf, -2);
+        virBufferAddLit(buf, "</reservations>\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 0bba016e4ebb7af3fecf27f55fe8fb85d26fe375..ec4915288073030857f8bf783caf07de922ac0f6 100644 (file)
--- 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 036c6e3c25024d6f0b8bd6336d51d9ed05b02038..acad600ef8acc493c35c88d410613ae1578ebae2 100644 (file)
--- a/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml
+++ b/tests/qemuxml2argvdata/disk-virtio-scsi-reservations.xml
@@ -17,7 +17,7 @@
     <disk type='block' device='lun'>
       <driver name='qemu' type='raw'/>
       <source dev='/dev/HostVG/QEMUGuest1'>
-        <reservations enabled='yes' managed='yes'/>
+        <reservations managed='yes'/>
       </source>
       <target dev='sda' bus='scsi'/>
       <address type='drive' controller='0' bus='0' target='0' unit='0'/>
@@ -25,7 +25,7 @@
     <disk type='block' device='lun'>
       <driver name='qemu' type='raw'/>
       <source dev='/dev/HostVG/QEMUGuest2'>
-        <reservations enabled='yes' managed='no'>
+        <reservations managed='no'>
           <source type='unix' path='/path/to/qemu-pr-helper.sock' mode='client'/>
         </reservations>
       </source>