[libvirt PATCH 1/4] util: add a helper method for controlling the COW flag on btrfs

[Daniel P. Berrangé]

On Thu, Jul 23, 2020 at 03:08:41PM +0200, Peter Krempa wrote:
> On Thu, Jul 23, 2020 at 14:00:40 +0100, Daniel Berrange wrote:
> > On Thu, Jul 23, 2020 at 02:57:32PM +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 at redhat.com>
> > > > ---
> > > >  src/libvirt_private.syms |  1 +
> > > >  src/util/virfile.c       | 76 ++++++++++++++++++++++++++++++++++++++++
> > > >  src/util/virfile.h       |  3 ++
> > > >  3 files changed, 80 insertions(+)
> 
> [...]
> 
> > > > +    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?
> > 
> > This would require us to use statfs() instad of fstatfs() in order to
> > check the super magic. I'm not seeing that improves things.
> 
> I definitely agree. But adding a function which does non-obvious things
> without any comment pointing to the non-obvious behaviour isn't good
> practice either.

I'll add this

/**
 * virFileSetCow:
 * @path: file or directory to control the COW flag on
 * @state: the desired state of the COW flag
 *
 * When @state is VIR_TRISTATE_BOOL_ABSENT, some helpful
 * default logic will be used. Specifically if the filesystem
 * containing @path is 'btrfs', then it will attempt to
 * disable the COW flag, but errors will be ignored. For
 * any other filesystem no change will be made.
 *
 * When @state is VIR_TRISTATE_BOOL_YES or VIR_TRISTATE_BOOL_NO,
 * it will attempt to set the COW flag state to that explicit
 * value, and always return an error if it fails. Note this
 * means it will always return error if the filesystem is not
 * 'btrfs'.
 */

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|