On Thu, Jul 23, 2020 at 14:57:38 +0200, Peter Krempa wrote:
On Mon, Jul 20, 2020 at 18:33:19 +0100, Daniel Berrange wrote:
> btrfs defaults to performing copy-on-write for files. This is often
> undesirable for VM images, so we need to be able to control whether this
> behaviour is used.
>
> The virFileSetCOW() will allow for this. We use a tristate, since out of
> the box, we want the default behaviour attempt to disable cow, but only
> on btrfs, silently do nothing on non-btrfs. If someone explicitly asks
> to disable/enable cow, then we want to raise a hard error on non-btrfs.
>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/libvirt_private.syms | 1 +
> src/util/virfile.c | 76 ++++++++++++++++++++++++++++++++++++++++
> src/util/virfile.h | 3 ++
> 3 files changed, 80 insertions(+)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 73b72c9e10..eea31a736d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2125,6 +2125,7 @@ virFileRewrite;
> virFileRewriteStr;
> virFileSanitizePath;
> virFileSetACLs;
> +virFileSetCOW;
> virFileSetupDev;
> virFileSetXAttr;
> virFileTouch;
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 213acdbcaa..5b169b3d11 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -71,6 +71,7 @@
> # endif
> # include <sys/ioctl.h>
> # include <linux/cdrom.h>
> +# include <linux/fs.h>
> #endif
>
> #if HAVE_LIBATTR
> @@ -4504,3 +4505,78 @@ virFileDataSync(int fd)
> return fdatasync(fd);
> #endif
> }
> +
> +
> +int
> +virFileSetCOW(const char *path,
> + virTristateBool state)
> +{
> +#if __linux__
> + int val = 0;
> + struct statfs buf;
> + VIR_AUTOCLOSE fd = -1;
> +
> + VIR_DEBUG("Setting COW flag on '%s' to '%s'",
> + path, virTristateBoolTypeToString(state));
> +
> + fd = open(path, O_RDONLY|O_NONBLOCK|O_LARGEFILE);
> + if (fd < 0) {
> + virReportSystemError(errno, _("unable to open '%s'"),
> + path);
> + return -1;
> + }
> +
> + if (fstatfs(fd, &buf) < 0) {
> + virReportSystemError(errno, _("unable query filesystem type on
'%s'"),
> + path);
> + return -1;
> + }
> +
> + if (buf.f_type != BTRFS_SUPER_MAGIC) {
> + if (state == VIR_TRISTATE_BOOL_ABSENT) {
Can't we handle the _ABSENT case before even attempting to open the
file?
I see now. This function actually attempts to disable CoW also when
VIR_TRISTATE_BOOL_ABSENT is passed.
That definitely is not obvious from the function name. Please add a
comment explaing what it actually does.