[libvirt] [PATCH for 5.3.0] storage_backend_rbd: Don't fail iterating over snapshots
When cloning an RBD volume we try to find a snapshot which is not different to the image we're trying to clone. This boils down to calling rbd_diff_iterate() or rbd_diff_iterate2() on systems with newer ceph. These two are passed a callback - virStorageBackendRBDIterateCb() which mistakenly states that if a negative value is returned from the callback, the iteration is stopped but no error is reported. Well, that is not the case. I've looked into librbd sources and found that a negative value does mean stop of iteration but it also means an error. That's why any attempt to clone an RBD image results in the following error: virsh # vol-clone --pool rbd_image_root coreos_2023 coreos00.disk error: Failed to clone vol from coreos_2023 error: failed to iterate RBD snapshot coreos_2023@base: Operation not permitted Reported on libvirt-users list: https://www.redhat.com/archives/libvirt-users/2019-April/msg00060.html Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_rbd.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index f8c968e682..3fe188551c 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1031,14 +1031,9 @@ virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, { /* * Just set that there is a diff for this snapshot, we do not care where - * - * When it returns a negative number the rbd_diff_iterate() function will stop - * - * That's why we return -1, meaning that there is a difference and we can stop - * searching any further. */ *(int*) arg = 1; - return -1; + return 0; } static int -- 2.21.0
On Mon, Apr 29, 2019 at 8:57 AM Michal Privoznik <mprivozn@redhat.com> wrote:
When cloning an RBD volume we try to find a snapshot which is not different to the image we're trying to clone. This boils down to calling rbd_diff_iterate() or rbd_diff_iterate2() on systems with newer ceph. These two are passed a callback - virStorageBackendRBDIterateCb() which mistakenly states that if a negative value is returned from the callback, the iteration is stopped but no error is reported. Well, that is not the case. I've looked into librbd sources and found that a negative value does mean stop of iteration but it also means an error. That's why any attempt to clone an RBD image results in the following error:
virsh # vol-clone --pool rbd_image_root coreos_2023 coreos00.disk error: Failed to clone vol from coreos_2023 error: failed to iterate RBD snapshot coreos_2023@base: Operation not permitted
Reported on libvirt-users list:
https://www.redhat.com/archives/libvirt-users/2019-April/msg00060.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_rbd.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index f8c968e682..3fe188551c 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1031,14 +1031,9 @@ virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, { /* * Just set that there is a diff for this snapshot, we do not care where - * - * When it returns a negative number the rbd_diff_iterate() function will stop - * - * That's why we return -1, meaning that there is a difference and we can stop - * searching any further. */ *(int*) arg = 1; - return -1; + return 0; }
You could also tweak "virStorageBackendRBDSnapshotFindNoDiff" to ignore the error if "diff > 0". That would still allow it to fail-fast once a delta is found.
static int -- 2.21.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Jason
On 4/29/19 3:13 PM, Jason Dillaman wrote:
On Mon, Apr 29, 2019 at 8:57 AM Michal Privoznik <mprivozn@redhat.com> wrote:
When cloning an RBD volume we try to find a snapshot which is not different to the image we're trying to clone. This boils down to calling rbd_diff_iterate() or rbd_diff_iterate2() on systems with newer ceph. These two are passed a callback - virStorageBackendRBDIterateCb() which mistakenly states that if a negative value is returned from the callback, the iteration is stopped but no error is reported. Well, that is not the case. I've looked into librbd sources and found that a negative value does mean stop of iteration but it also means an error. That's why any attempt to clone an RBD image results in the following error:
virsh # vol-clone --pool rbd_image_root coreos_2023 coreos00.disk error: Failed to clone vol from coreos_2023 error: failed to iterate RBD snapshot coreos_2023@base: Operation not permitted
Reported on libvirt-users list:
https://www.redhat.com/archives/libvirt-users/2019-April/msg00060.html
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/storage/storage_backend_rbd.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c index f8c968e682..3fe188551c 100644 --- a/src/storage/storage_backend_rbd.c +++ b/src/storage/storage_backend_rbd.c @@ -1031,14 +1031,9 @@ virStorageBackendRBDIterateCb(uint64_t offset ATTRIBUTE_UNUSED, { /* * Just set that there is a diff for this snapshot, we do not care where - * - * When it returns a negative number the rbd_diff_iterate() function will stop - * - * That's why we return -1, meaning that there is a difference and we can stop - * searching any further. */ *(int*) arg = 1; - return -1; + return 0; }
You could also tweak "virStorageBackendRBDSnapshotFindNoDiff" to ignore the error if "diff > 0". That would still allow it to fail-fast once a delta is found.
Good catch. Let me post a v2. Michal
participants (2)
-
Jason Dillaman -
Michal Privoznik