Re: [libvirt] [PATCH RFC] storage: perform btrfs clone if possible
-----Original Message-----
From: Eric Blake [mailto:eblake@redhat.com]
Sent: Tuesday, November 25, 2014 11:59 PM
To: Chen, Hanxiao/陈 晗霄; Martin Kletzander
Cc: libvir-list(a)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.
Thanks,
- Chen