Re: [libvirt] [PATCH 2/2] storage: try to perform btrfs COW copy if possible
-----Original Message-----
From: Jรกn Tomko [mailto:jtomko@redhat.com]
Sent: Friday, January 16, 2015 10:22 PM
To: Chen, Hanxiao/้ ๆ้; libvir-list(a)redhat.com
Subject: Re: [libvirt] [PATCH 2/2] storage: try to perform btrfs COW copy if possible
On 01/13/2015 09:18 AM, Chen Hanxiao wrote:
> We don't take advantage of clone of btrfs.
>
> So
> a)try to do a btrfs lightweight copy
>
> b)fall back to a standard copy
> if COW copy not supported.
>
> Signed-off-by: Chen Hanxiao <chenhanxiao(a)cn.fujitsu.com>
> ---
> include/libvirt/libvirt-storage.h | 1 +
> src/storage/storage_backend.c | 39
++++++++++++++++++++++++++++++++-------
> src/storage/storage_backend_fs.c | 8 ++++++--
> src/storage/storage_driver.c | 4 +++-
> 4 files changed, 42 insertions(+), 10 deletions(-)
>
> diff --git a/include/libvirt/libvirt-storage.h
b/include/libvirt/libvirt-storage.h
> index 1f3087b..4ae42e1 100644
> --- a/include/libvirt/libvirt-storage.h
> +++ b/include/libvirt/libvirt-storage.h
> @@ -306,6 +306,7 @@ const char* virStorageVolGetKey
(virStorageVolPtr vol);
>
> typedef enum {
> VIR_STORAGE_VOL_CREATE_PREALLOC_METADATA = 1 << 0,
> + VIR_STORAGE_VOL_CREATE_REFLINK = 1 << 1,
There should be some human-readable commentary after the flag, that will be
shown on our API reference page:
http://libvirt.org/html/libvirt-libvirt-storage.html#virStorageVolCreateF...
Will do.
> } virStorageVolCreateFlags;
>
> virStorageVolPtr virStorageVolCreateXML (virStoragePoolPtr
pool,
> diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
> index 5ce3566..838398e 100644
> --- a/src/storage/storage_backend.c
> +++ b/src/storage/storage_backend.c
> @@ -187,7 +187,8 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> virStorageVolDefPtr inputvol,
> int fd,
> unsigned long long *total,
> - bool want_sparse)
> + bool want_sparse,
> + bool reflink_copy)
> {
> int inputfd = -1;
> int amtread = -1;
> @@ -226,6 +227,15 @@ virStorageBackendCopyToFD(virStorageVolDefPtr vol,
> goto cleanup;
> }
>
> + if (reflink_copy) {
> + if (btrfsCloneFile(fd, inputfd) == -1) {
> + VIR_DEBUG("btrfs clone not supported, try another way.");
If reflink copy is unsupported, I think we should just error out, as a full
copy won't be as fast / occupy as little space.
Also, this would fall back even on other errors.
That's a better solution, will be changed in v2.
Thanks,
- Chen
The rest looks good to me.
Jan