From: Josh Durgin Date: Sun, 31 Mar 2013 00:25:18 +0000 (-0700) Subject: librbd: change diff_iterate interface to be more C-friendly X-Git-Tag: v0.62~118^2~9 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c680531e070aef46bde50a8185d3a48ff9f2d9df;p=ceph.git librbd: change diff_iterate interface to be more C-friendly Use int instead of bool for the callback, and make it represent whether the data exists, rather than the opposite, since callers are likely to test for whether it's data instead of whether its zeroes. Change the return value to 0, since an int64_t will wrap around for large reads, and there's no value in reporting the length read when it will always be the length requested clipped to the size of the image. Signed-off-by: Josh Durgin --- diff --git a/src/include/rbd/librbd.hpp b/src/include/rbd/librbd.hpp index d061bdbe4eb8..600a3ccbcda0 100644 --- a/src/include/rbd/librbd.hpp +++ b/src/include/rbd/librbd.hpp @@ -161,22 +161,23 @@ public: * * This will return the differences between two versions of an image * via a callback, which gets the offset and length and a flag - * indicating whether the extent is known/defined to be zeros (a - * hole). If the source snapshot name is NULL, we interpret that as - * the beginning of time and return all allocated regions of the - * image. The end version is whatever is currently selected for the - * image handle (either a snapshot or the writeable head). + * indicating whether the extent exists (1), or is known/defined to + * be zeros (a hole, 0). If the source snapshot name is NULL, we + * interpret that as the beginning of time and return all allocated + * regions of the image. The end version is whatever is currently + * selected for the image handle (either a snapshot or the writeable + * head). * * @param fromsnapname start snapshot name, or NULL * @param ofs start offset * @param len len in bytes of region to report on * @param cb callback to call for each allocated region * @param arg argument to pass to the callback - * @returns len on success, or negative error code on error + * @returns 0 on success, or negative error code on error */ - int64_t diff_iterate(const char *fromsnapname, - uint64_t ofs, size_t len, - int (*cb)(uint64_t, size_t, bool, void *), void *arg); + int diff_iterate(const char *fromsnapname, + uint64_t ofs, size_t len, + int (*cb)(uint64_t, size_t, int, void *), void *arg); ssize_t write(uint64_t ofs, size_t len, ceph::bufferlist& bl); int discard(uint64_t ofs, uint64_t len); diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index 4daa5b801db3..49573966f8ba 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -2269,18 +2269,22 @@ reprotect_and_return_err: return total_read; } - int simple_diff_cb(uint64_t off, size_t len, bool exists, void *arg) + int simple_diff_cb(uint64_t off, size_t len, int exists, void *arg) { + // This reads the existing extents in a parent from the beginning + // of time. Since images are thin-provisioned, the extents will + // always represent data, not holes. + assert(exists); interval_set *diff = static_cast *>(arg); diff->insert(off, len); return 0; } - int64_t diff_iterate(ImageCtx *ictx, const char *fromsnapname, - uint64_t off, uint64_t len, - int (*cb)(uint64_t, size_t, bool, void *), - void *arg) + int diff_iterate(ImageCtx *ictx, const char *fromsnapname, + uint64_t off, uint64_t len, + int (*cb)(uint64_t, size_t, int, void *), + void *arg) { utime_t start_time, elapsed; @@ -2291,8 +2295,7 @@ reprotect_and_return_err: if (r < 0) return r; - uint64_t mylen = len; - r = clip_io(ictx, off, &mylen); + r = clip_io(ictx, off, &len); if (r < 0) return r; @@ -2347,9 +2350,8 @@ reprotect_and_return_err: return r; } - int64_t total_read = 0; uint64_t period = ictx->get_stripe_period(); - uint64_t left = mylen; + uint64_t left = len; while (left > 0) { uint64_t period_off = off - (off % period); @@ -2385,7 +2387,7 @@ reprotect_and_return_err: o.intersection_of(parent_diff); ldout(ictx->cct, 20) << " reporting parent overlap " << o << dendl; for (interval_set::iterator s = o.begin(); s != o.end(); ++s) { - cb(s.get_start(), s.get_len(), false, arg); + cb(s.get_start(), s.get_len(), true, arg); } } } @@ -2431,7 +2433,7 @@ reprotect_and_return_err: << " logical " << logical_off << "~" << s.get_len() << dendl; - cb(logical_off, s.get_len(), !end_exists, arg); + cb(logical_off, s.get_len(), end_exists, arg); } opos += r->second; } @@ -2439,12 +2441,11 @@ reprotect_and_return_err: } } - total_read += read_len; left -= read_len; off += read_len; } - return total_read; + return 0; } int simple_read_cb(uint64_t ofs, size_t len, const char *buf, void *arg) diff --git a/src/librbd/internal.h b/src/librbd/internal.h index 7f66006e12c4..e407d44af9e0 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -168,10 +168,10 @@ namespace librbd { int64_t read_iterate(ImageCtx *ictx, uint64_t off, size_t len, int (*cb)(uint64_t, size_t, const char *, void *), void *arg); - int64_t diff_iterate(ImageCtx *ictx, const char *fromsnapname, - uint64_t off, uint64_t len, - int (*cb)(uint64_t, size_t, bool, void *), - void *arg); + int diff_iterate(ImageCtx *ictx, const char *fromsnapname, + uint64_t off, uint64_t len, + int (*cb)(uint64_t, size_t, int, void *), + void *arg); ssize_t read(ImageCtx *ictx, uint64_t off, size_t len, char *buf); ssize_t read(ImageCtx *ictx, const vector >& image_extents, char *buf, bufferlist *pbl); diff --git a/src/librbd/librbd.cc b/src/librbd/librbd.cc index b522288b6a7d..20dbf62393b4 100644 --- a/src/librbd/librbd.cc +++ b/src/librbd/librbd.cc @@ -442,10 +442,10 @@ namespace librbd { return librbd::read_iterate(ictx, ofs, len, cb, arg); } - int64_t Image::diff_iterate(const char *fromsnapname, - uint64_t ofs, uint64_t len, - int (*cb)(uint64_t, size_t, bool, void *), - void *arg) + int Image::diff_iterate(const char *fromsnapname, + uint64_t ofs, uint64_t len, + int (*cb)(uint64_t, size_t, int, void *), + void *arg) { ImageCtx *ictx = (ImageCtx *)ctx; return librbd::diff_iterate(ictx, fromsnapname, ofs, len, cb, arg); @@ -1032,6 +1032,16 @@ extern "C" int64_t rbd_read_iterate(rbd_image_t image, uint64_t ofs, size_t len, return librbd::read_iterate(ictx, ofs, len, cb, arg); } +extern "C" int diff_iterate(rbd_image_t image, + const char *fromsnapname, + uint64_t ofs, uint64_t len, + int (*cb)(uint64_t, size_t, int, void *), + void *arg) +{ + librbd::ImageCtx *ictx = (librbd::ImageCtx *)image; + return librbd::diff_iterate(ictx, fromsnapname, ofs, len, cb, arg); +} + extern "C" ssize_t rbd_write(rbd_image_t image, uint64_t ofs, size_t len, const char *buf) { diff --git a/src/rbd.cc b/src/rbd.cc index f6cdb5839051..d776ef018cdf 100644 --- a/src/rbd.cc +++ b/src/rbd.cc @@ -1035,14 +1035,14 @@ static int do_export(librbd::Image& image, const char *path) return r; } -static int export_diff_cb(uint64_t ofs, size_t _len, bool zero, void *arg) +static int export_diff_cb(uint64_t ofs, size_t _len, int exists, void *arg) { ExportContext *ec = static_cast(arg); int r; // extent bufferlist bl; - __u8 tag = zero ? 'z' : 'w'; + __u8 tag = exists ? 'w' : 'z'; ::encode(tag, bl); ::encode(ofs, bl); uint64_t len = _len; @@ -1051,7 +1051,7 @@ static int export_diff_cb(uint64_t ofs, size_t _len, bool zero, void *arg) if (r < 0) return r; - if (!zero) { + if (exists) { // read block bl.clear(); r = ec->image->read(ofs, len, bl); @@ -1071,7 +1071,7 @@ static int do_export_diff(librbd::Image& image, const char *fromsnapname, const char *endsnapname, const char *path) { - int64_t r; + int r; librbd::image_info_t info; int fd; @@ -1139,7 +1139,7 @@ static int do_export_diff(librbd::Image& image, const char *fromsnapname, return r; } -static int diff_cb(uint64_t ofs, size_t len, bool zero, void *arg) +static int diff_cb(uint64_t ofs, size_t len, int exists, void *arg) { cout << ofs << "\t" << len << "\t" << (zero ? "zero" : "data") << "\n"; @@ -1148,7 +1148,7 @@ static int diff_cb(uint64_t ofs, size_t len, bool zero, void *arg) static int do_diff(librbd::Image& image, const char *fromsnapname) { - int64_t r; + int r; librbd::image_info_t info; r = image.stat(info, sizeof(info)); diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc index ff083ae817f4..c24f41ec9534 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -1503,7 +1503,7 @@ TEST(LibRBD, FlushAioPP) } -int iterate_cb(uint64_t off, size_t len, bool zero, void *arg) +int iterate_cb(uint64_t off, size_t len, int exists, void *arg) { cout << "iterate_cb " << off << "~" << len << std::endl; interval_set *diff = static_cast *>(arg); @@ -1564,7 +1564,7 @@ TEST(LibRBD, DiffIterate) cout << " wrote " << two << std::endl; interval_set diff; - ASSERT_EQ((int)size, image.diff_iterate("one", 0, size, iterate_cb, (void *)&diff)); + ASSERT_EQ(0, image.diff_iterate("one", 0, size, iterate_cb, (void *)&diff)); cout << " diff was " << diff << std::endl; if (!two.subset_of(diff)) { interval_set i; @@ -1620,7 +1620,7 @@ TEST(LibRBD, DiffIterateStress) cout << "from " << i << " to " << j << " diff " << diff << std::endl; image.snap_set(snap[j].c_str()); - ASSERT_EQ((int)size, image.diff_iterate(snap[i].c_str(), 0, size, iterate_cb, (void *)&actual)); + ASSERT_EQ(0, image.diff_iterate(snap[i].c_str(), 0, size, iterate_cb, (void *)&actual)); cout << " actual was " << actual << std::endl; if (!diff.subset_of(actual)) { interval_set i;