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@xxxxxxxxxx>
> > ---
> > 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.
