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@xxxxxxxx>
> ---
> 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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list