New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbd: import-diff should discard any zeroed extents #14445

Merged
merged 1 commit into from Apr 13, 2017

Conversation

Projects
None yet
2 participants
@dillaman
Contributor

dillaman commented Apr 11, 2017

Sparse (zeroed) extents cannot be safely skipped. Instead, the
zeroed extent should be discarded from the image to ensure
the import remains consistent with the export.

Signed-off-by: Jason Dillaman dillaman@redhat.com

@trociny trociny self-assigned this Apr 11, 2017

@trociny

This comment has been minimized.

Contributor

trociny commented Apr 11, 2017

@ceph-jenkins retest this please

@dillaman

This comment has been minimized.

Contributor

dillaman commented Apr 11, 2017

retest this please

@trociny

This comment has been minimized.

Contributor

trociny commented Apr 12, 2017

@dillaman In the initial PR the sparse support has been also added to copy. Shouldn't it also use discard?

@dillaman

This comment has been minimized.

Contributor

dillaman commented Apr 12, 2017

@trociny copy and import are safe to skip sparse extents since for new images you can safely assume the non-written values will be zero. For import-diff, you don't have such assurances that the sparse data you skipping is properly zeroed.

@@ -335,7 +337,8 @@ static int read_tag(int fd, __u8 end_tag, int format, __u8 *tag, uint64_t *readl
return 0;
}
int do_import_diff_fd(librbd::Image &image, int fd, bool no_progress, int format, size_t sparse_size)
int do_import_diff_fd(librados::Rados &rados, librbd::Image &image, int fd,

This comment has been minimized.

@trociny

trociny Apr 12, 2017

Contributor

Has rados been added to check for rbd_skip_partial_discard config setting? It still uses global for this.

This comment has been minimized.

@dillaman

dillaman Apr 12, 2017

Contributor

Yeah -- goal was to use Rados::conf_get and then I forgot about it.

This comment has been minimized.

@dillaman

dillaman Apr 12, 2017

Contributor

Update pushed

@trociny

This comment has been minimized.

Contributor

trociny commented Apr 12, 2017

otherwise it looks good to me

rbd: import-diff should discard any zeroed extents
Sparse (zeroed) extents cannot be safely skipped. Instead, the
zeroed extent should be discarded from the image to ensure
the import remains consistent with the export.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
@dillaman

This comment has been minimized.

Contributor

dillaman commented Apr 12, 2017

retest this please

@trociny trociny merged commit b9522ef into ceph:master Apr 13, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment