[libvirt] [PATCH V3 1/2] storagevol: add nocow to vol xml

[John Ferlan]

Coverity flagged this commit....  see below...

On 07/15/2014 04:49 AM, Chunyan Liu wrote:
> Add 'nocow' to storage volume xml so that user can have an option
> to set NOCOW flag to the newly created volume. It's useful on btrfs
> file system to enhance performance.
> 
> Btrfs has low performance when hosting VM images, even more when the guest
> in those VM are also using btrfs as file system. One way to mitigate this
> bad performance is to turn off COW attributes on VM files. Generally, there
> are two ways to turn off COW on btrfs: a) by mounting fs with nodatacow,
> then all newly created files will be NOCOW. b) per file. Add the NOCOW file
> attribute. It could only be done to empty or new files.
> 
> This patch tries the second way, according to 'nocow' option, it could set
> NOCOW flag per file:
> for raw file images, handle 'nocow' in libvirt code; for non-raw file images,
> pass 'nocow=on' option to qemu-img, and let qemu-img to handle that (requires
> qemu-img version >= 2.1).
> 
> Signed-off-by: Chunyan Liu <cyliu at suse.com>
> ---
> Changes:
>   * fix typo
> 
>  docs/formatstorage.html.in    |  7 +++++++
>  docs/schemas/storagevol.rng   |  5 +++++
>  src/conf/storage_conf.c       |  3 +++
>  src/storage/storage_backend.c | 22 ++++++++++++++++++++++
>  src/util/virstoragefile.h     |  1 +
>  5 files changed, 38 insertions(+)
> 
> diff --git a/docs/formatstorage.html.in b/docs/formatstorage.html.in
> index 1cd82b4..8d51402 100644
> --- a/docs/formatstorage.html.in
> +++ b/docs/formatstorage.html.in
> @@ -385,6 +385,7 @@
>              <label>virt_image_t</label>
>            </permissions>
>            <compat>1.1</compat>
> +          <nocow/>
>            <features>
>              <lazy_refcounts/>
>            </features>
> @@ -424,6 +425,12 @@
>          1.1 is used. If omitted, qemu-img default is used.
>          <span class="since">Since 1.1.0</span>
>        </dd>
> +      <dt><code>nocow</code></dt>
> +      <dd>Turn off COW of the newly created volume. So far, this is only valid
> +        for a file image in btrfs file system. It will improve performance when
> +        the file image is used in VM. To create non-raw file images, it
> +        requires QEMU version since 2.1. <span class="since">Since 1.2.6</span>
> +      </dd>
>        <dt><code>features</code></dt>
>        <dd>Format-specific features. Only used for <code>qcow2</code> now.
>          Valid sub-elements are:
> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng
> index 3798476..1b2d4cc 100644
> --- a/docs/schemas/storagevol.rng
> +++ b/docs/schemas/storagevol.rng
> @@ -138,6 +138,11 @@
>            <ref name='compat'/>
>          </optional>
>          <optional>
> +          <element name='nocow'>
> +            <empty/>
> +          </element>
> +        </optional>
> +        <optional>
>            <ref name='fileFormatFeatures'/>
>          </optional>
>        </interleave>
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index aa29658..f28a314 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -1285,6 +1285,9 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool,
>          virStringFreeList(version);
>      }
>  
> +    if (virXPathNode("./target/nocow", ctxt))
> +        ret->target.nocow = true;
> +
>      if (options->featureFromString && virXPathNode("./target/features", ctxt)) {
>          if ((n = virXPathNodeSet("./target/features/*", ctxt, &nodes)) < 0)
>              goto error;
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 7b17ca4..5f2dc5d 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -37,6 +37,9 @@
>  #ifdef __linux__
>  # include <sys/ioctl.h>
>  # include <linux/fs.h>
> +# ifndef FS_NOCOW_FL
> +#  define FS_NOCOW_FL                     0x00800000 /* Do not cow file */
> +# endif
>  #endif
>  
>  #if WITH_SELINUX
> @@ -453,6 +456,21 @@ virStorageBackendCreateRaw(virConnectPtr conn ATTRIBUTE_UNUSED,
>          goto cleanup;
>      }
>  
> +    if (vol->target.nocow) {
> +#ifdef __linux__
> +        int attr;
> +
> +        /* Set NOCOW flag. This is an optimisation for btrfs.
> +         * The FS_IOC_SETFLAGS ioctl return value will be ignored since any
> +         * failure of this operation should not block the left work.
> +         */
> +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) == 0) {
> +            attr |= FS_NOCOW_FL;
> +            ioctl(fd, FS_IOC_SETFLAGS, &attr);

              ^^^

Status is not checked here causing complaint about CHECKED_RETURN

John
> +        }
> +#endif
> +    }
> +
>      if ((ret = createRawFile(fd, vol, inputvol)) < 0)
>          /* createRawFile already reported the exact error. */
>          ret = -1;
> @@ -718,6 +736,7 @@ virStorageBackendCreateQemuImgOpts(char **opts,
>                                     bool preallocate,
>                                     int format,
>                                     const char *compat,
> +                                   bool nocow,
>                                     virBitmapPtr features)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -730,6 +749,8 @@ virStorageBackendCreateQemuImgOpts(char **opts,
>          virBufferAddLit(&buf, "encryption=on,");
>      if (preallocate)
>          virBufferAddLit(&buf, "preallocation=metadata,");
> +    if (nocow)
> +        virBufferAddLit(&buf, "nocow=on,");
>  
>      if (compat)
>          virBufferAsprintf(&buf, "compat=%s,", compat);
> @@ -949,6 +970,7 @@ virStorageBackendCreateQemuImgCmd(virConnectPtr conn,
>                                                 do_encryption, preallocate,
>                                                 vol->target.format,
>                                                 compat,
> +                                               vol->target.nocow,
>                                                 vol->target.features) < 0) {
>              virCommandFree(cmd);
>              return NULL;
> diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
> index 0ba746a..c840aa0 100644
> --- a/src/util/virstoragefile.h
> +++ b/src/util/virstoragefile.h
> @@ -247,6 +247,7 @@ struct _virStorageSource {
>                   * pool-specific enum for storage volumes */
>      virBitmapPtr features;
>      char *compat;
> +    bool nocow;
>  
>      virStoragePermsPtr perms;
>      virStorageTimestampsPtr timestamps;
>