Re: [libvirt PATCH 4/4] conf: add control over COW for storage pool directories

2020-07-23 Thread Peter Krempa
On Mon, Jul 20, 2020 at 18:33:22 +0100, Daniel Berrange wrote:
> The storage pool code now attempts to disable COW by default on btrfs,
> but management applications may wish to override this behaviour. Thus we
> introduce a concept of storage pool features:
> 
>   
> 
>   
> 
> If the  feature policy is set, it will be enforced. It will always
> return an hard error if COW cannot be explicitly set or unset.
> 
> Signed-off-by: Daniel P. Berrangé 
> ---
>  docs/formatstorage.html.in   | 25 ++
>  docs/schemas/storagepool.rng | 30 
>  src/conf/storage_conf.c  | 49 
>  src/conf/storage_conf.h  |  8 
>  src/storage/storage_util.c   |  2 +-
>  tests/storagepoolxml2xmlin/pool-dir-cow.xml  | 10 
>  tests/storagepoolxml2xmlout/pool-dir-cow.xml | 15 ++
>  tests/storagepoolxml2xmltest.c   |  1 +
>  8 files changed, 139 insertions(+), 1 deletion(-)
>  create mode 100644 tests/storagepoolxml2xmlin/pool-dir-cow.xml
>  create mode 100644 tests/storagepoolxml2xmlout/pool-dir-cow.xml


> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 65d9b33049..4e63865b39 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -839,6 +839,33 @@ virStoragePoolDefRefreshFormat(virBufferPtr buf,
>  }
>  
>  
> +static int
> +virStoragePoolDefParseFeatures(virStoragePoolDefPtr def,
> +   xmlXPathContextPtr ctxt)
> +{
> +g_autofree char *cow = virXPathString("string(./features/cow/@state)", 
> ctxt);
> +
> +if (cow) {
> +int val;
> +if (def->type != VIR_STORAGE_POOL_FS &&
> +def->type != VIR_STORAGE_POOL_DIR) {
> +virReportError(VIR_ERR_NO_SUPPORT, "%s",
> +   _("cow feature may only be used for 'fs' and 
> 'dir' pools"));
> +return -1;
> +}
> +if ((val = virTristateBoolTypeFromString(cow)) < 0) {

<= 0

> +virReportError(VIR_ERR_XML_ERROR,
> +   _("invalid storage pool cow feature state '%s'"),
> +   cow);
> +return -1;
> +}
> +def->features.cow = val;
> +}
> +
> +return 0;
> +}
> +
> +
>  virStoragePoolDefPtr
>  virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>  {
> @@ -910,6 +937,9 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>  }
>  }
>  
> +if (virStoragePoolDefParseFeatures(def, ctxt) < 0)
> +return NULL;
> +
>  if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) {
>  if (!def->source.nhost) {
>  virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -1131,6 +1161,23 @@ virStoragePoolSourceFormat(virBufferPtr buf,
>  }
>  
>  
> +static void
> +virStoragePoolDefFormatFeatures(virBufferPtr buf,
> +virStoragePoolDefPtr def)
> +{
> +if (!def->features.cow)
> +return;
> +
> +virBufferAddLit(buf, "\n");
> +virBufferAdjustIndent(buf, 2);
> +if (def->features.cow)

def->features.cow != VIR_TRISTATE_BOOL_ABSENT

> +virBufferAsprintf(buf, "\n",
> +  virTristateBoolTypeToString(def->features.cow));
> +virBufferAdjustIndent(buf, -2);
> +virBufferAddLit(buf, "\n");
> +}



[libvirt PATCH 4/4] conf: add control over COW for storage pool directories

2020-07-20 Thread Daniel P . Berrangé
The storage pool code now attempts to disable COW by default on btrfs,
but management applications may wish to override this behaviour. Thus we
introduce a concept of storage pool features:

  

  

If the  feature policy is set, it will be enforced. It will always
return an hard error if COW cannot be explicitly set or unset.

Signed-off-by: Daniel P. Berrangé 
---
 docs/formatstorage.html.in   | 25 ++
 docs/schemas/storagepool.rng | 30 
 src/conf/storage_conf.c  | 49 
 src/conf/storage_conf.h  |  8 
 src/storage/storage_util.c   |  2 +-
 tests/storagepoolxml2xmlin/pool-dir-cow.xml  | 10 
 tests/storagepoolxml2xmlout/pool-dir-cow.xml | 15 ++
 tests/storagepoolxml2xmltest.c   |  1 +
 8 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 tests/storagepoolxml2xmlin/pool-dir-cow.xml
 create mode 100644 tests/storagepoolxml2xmlout/pool-dir-cow.xml

diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
index 2a7604d136..7493714c5c 100644
--- a/docs/formatstorage.html.in
+++ b/docs/formatstorage.html.in
@@ -67,6 +67,31 @@
 pool. Since 0.4.1
 
 
+Features
+
+
+  Some pools support optional features:
+
+
+
+...
+features
+  cow state='no'
+/features
+...
+
+
+  Valid features are:
+
+
+  cow
+  Controls whether the filesystem performs copy-on-write (COW) for
+images in the pool. This may only be set for directory / filesystem
+pools on the btrfs filesystem. If not set then libvirt
+will attempt to disable COW on any btrfs filesystems.
+Since 6.6.0.
+
+
 Source elements
 
 
diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
index ff0d3c836c..f5cf6769c8 100644
--- a/docs/schemas/storagepool.rng
+++ b/docs/schemas/storagepool.rng
@@ -37,6 +37,7 @@
 
   
   
+  
   
   
 
@@ -49,6 +50,7 @@
 
   
   
+  
   
   
 
@@ -64,6 +66,7 @@
 
   
   
+  
   
   
 
@@ -79,6 +82,7 @@
 
   
   
+  
   
   
 
@@ -91,6 +95,7 @@
 
   
   
+  
   
   
 
@@ -103,6 +108,7 @@
 
   
   
+  
   
   
 
@@ -117,6 +123,7 @@
   
 
   
+  
   
 
   
@@ -128,6 +135,7 @@
 
   
   
+  
   
   
 
@@ -140,6 +148,7 @@
 
   
   
+  
   
 
   
@@ -154,6 +163,7 @@
 
   
   
+  
   
   
 
@@ -169,6 +179,7 @@
 
   
   
+  
   
 
   
@@ -180,6 +191,7 @@
 
   
   
+  
   
 
   
@@ -191,6 +203,7 @@
 
   
   
+  
   
   
 
@@ -205,6 +218,7 @@
 
   
   
+  
   
   
 
@@ -277,6 +291,22 @@
 
   
 
+  
+
+  
+
+  
+
+  
+
+
+
+  
+
+  
+
+  
+
   
 
   
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 65d9b33049..4e63865b39 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -839,6 +839,33 @@ virStoragePoolDefRefreshFormat(virBufferPtr buf,
 }
 
 
+static int
+virStoragePoolDefParseFeatures(virStoragePoolDefPtr def,
+   xmlXPathContextPtr ctxt)
+{
+g_autofree char *cow = virXPathString("string(./features/cow/@state)", 
ctxt);
+
+if (cow) {
+int val;
+if (def->type != VIR_STORAGE_POOL_FS &&
+def->type != VIR_STORAGE_POOL_DIR) {
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
+   _("cow feature may only be used for 'fs' and 'dir' 
pools"));
+return -1;
+}
+if ((val = virTristateBoolTypeFromString(cow)) < 0) {
+virReportError(VIR_ERR_XML_ERROR,
+   _("invalid storage pool cow feature state '%s'"),
+   cow);
+return -1;
+}
+def->features.cow = val;
+}
+
+return 0;
+}
+
+
 virStoragePoolDefPtr
 virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
 {
@@ -910,6 +937,9 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
 }
 }
 
+if (virStoragePoolDefParseFeatures(def, ctxt) < 0)
+return NULL;
+
 if (options->flags & VIR_STORAGE_POOL_SOURCE_HOST) {
 if (!def->source.nhost) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -1131,6 +1161,23 @@ virStoragePoolSourceFormat(virBufferPtr buf,
 }
 
 
+static void
+virStoragePoolDefFormatFeatures(virBufferPtr buf,
+virStoragePoolDefPtr def)
+{
+if (!def->features.cow)
+return;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+if (def->features.cow)
+