[libvirt] [PATCH RFC] storage: perform btrfs clone if possible

Chen, Hanxiao chenhanxiao at cn.fujitsu.com
Wed Nov 26 10:07:36 UTC 2014

> -----Original Message-----
> From: Eric Blake [mailto:eblake at redhat.com]
> Sent: Tuesday, November 25, 2014 11:59 PM
> To: Chen, Hanxiao/陈 晗霄; Martin Kletzander
> Cc: libvir-list at redhat.com
> Subject: Re: [libvirt] [PATCH RFC] storage: perform btrfs clone if possible
> On 11/25/2014 03:19 AM, Chen, Hanxiao wrote:
> >> I think it makes sense to expose this functionality; although I suspect
> >> it is better if we do so by having the user pass an explicit new flag
> >> value to existing API instead of doing it automatically.
> >>
> > Thanks for your clarification.
> >
> > But we've already had nocow in virStorageSource and <nocow> tags.
> > So I think if we do not specify <nocow> tags in XML,
> > we should try it according to 'nocow' in codes.
> >
> > Or do we need a new flags such as --reflink
> > for tools like virt-clone?
> It's a design trade-off - do we make the default operation efficient
> when possible (but with possible surprises due to overcommit of
> storage), or safe (but slow)?  Regardless of the default, is it possible
> to explicitly request the opposite action?  To me, it seems backwards
> compatible to make an operation faster by using underlying clone hooks
> as long as the end result is the same, and as long as a user NOT
> specifying a specific use of clone or avoidance of clone does not get an
> error message when clone is attempted but not supported.  So at this
> point, I could go either way for the default as long as I can still have
> full control over the behavior, and we don't break existing users.

FYI, in my limited test,
the suggestion of this patch did not break existing users.
If not supported, it just print some debug logs
and try the original way then:

+    if (!vol->target.nocow) {
+        if (btrfs_clone_file(fd, inputfd) == -1) {
+            if (errno == ENOTSUP)
+                VIR_DEBUG("btrfs clone not supported, try another way.");
+        } else {
+            VIR_DEBUG("btrfs clone findished.");
+            goto cleanup;
+        }
+    }

If I didn't miss something, I think this kind of operation
is acceptable.

- Chen

More information about the libvir-list mailing list