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

Jim Fehlig jfehlig at suse.com
Thu Jul 23 20:22:44 UTC 2020


On 7/23/20 8:59 AM, Daniel P. Berrangé wrote:
> 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 was reviewing this series Monday before getting distracted with other things 
and had to read this function a few times to get my head around it.

> 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'.
>   */

But that definitely helps! Thanks for adding it.

Regards,
Jim





More information about the libvir-list mailing list