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>

diff --git a/src/tools/rbd/Utils.cc b/src/tools/rbd/Utils.cc
index 4dd4819c8ef831784c33ef5605d08b205fc5f035..a802a77cb8b6c00161b80adf6fe67552c2f0effb 100644 (file)
--- a/src/tools/rbd/Utils.cc
+++ b/src/tools/rbd/Utils.cc
@@ -949,34 +949,39 @@ int snap_set(librbd::Image &image, const std::string &snap_name) {
   return 0;
 }
 
-bool calc_sparse_extent(const bufferptr &bp,
+void calc_sparse_extent(const bufferptr &bp,
                         size_t sparse_size,
-                        uint64_t length,
-                        size_t *write_offset,
+                       size_t buffer_offset,
+                        uint64_t buffer_length,
                         size_t *write_length,
-                        size_t *offset) {
-  size_t extent_size;
-  if (*offset + sparse_size > length) {
-    extent_size = length - *offset;
-  } else {
-    extent_size = sparse_size;
-  }
-
-  bufferptr extent(bp, *offset, extent_size);
-  *offset += extent_size;
+                        bool *zeroed) {
+  if (sparse_size == 0) {
+    // sparse writes are disabled -- write the full extent
+    assert(buffer_offset == 0);
+    *write_length = buffer_length;
+    *zeroed = false;
+    return;
+  }
+
+  *write_length = 0;
+  size_t original_offset = buffer_offset;
+  while (buffer_offset < buffer_length) {
+    size_t extent_size = std::min<size_t>(
+      sparse_size, buffer_length - buffer_offset);
+
+    bufferptr extent(bp, buffer_offset, extent_size);
+
+    bool extent_is_zero = extent.is_zero();
+    if (original_offset == buffer_offset) {
+      *zeroed = extent_is_zero;
+    } else if (*zeroed != extent_is_zero) {
+      assert(*write_length > 0);
+      return;
+    }
 
-  bool extent_is_zero = extent.is_zero();
-  if (!extent_is_zero) {
+    buffer_offset += extent_size;
     *write_length += extent_size;
   }
-  if (extent_is_zero &&  *write_length == 0) {
-    *write_offset += extent_size;
-  }
-
-  if ((extent_is_zero || *offset == length) && *write_length != 0) {
-    return true;
-  }
-  return false;
 }
 
 std::string image_id(librbd::Image& image) {

diff --git a/src/tools/rbd/Utils.h b/src/tools/rbd/Utils.h
index c2b7f8c54b67a3c8b699a8c3fc1b23262c9a401b..943153e25726625a301719d88b1e0fdb9dfa419c 100644 (file)
--- a/src/tools/rbd/Utils.h
+++ b/src/tools/rbd/Utils.h
@@ -187,12 +187,12 @@ int init_and_open_image(const std::string &pool_name,
 
 int snap_set(librbd::Image &image, const std::string &snap_name);
 
-bool calc_sparse_extent(const bufferptr &bp,
+void calc_sparse_extent(const bufferptr &bp,
                         size_t sparse_size,
+                       size_t buffer_offset,
                         uint64_t length,
-                        size_t *write_offset,
                         size_t *write_length,
-                        size_t *offset);
+                       bool *zeroed);
 
 bool check_if_image_spec_present(const boost::program_options::variables_map &vm,
                                  argument_types::ArgumentModifier mod,

diff --git a/src/tools/rbd/action/Import.cc b/src/tools/rbd/action/Import.cc
index c76ea1328e2ea58d91c8df223661e54f299cd85b..348915dbcced233a78714f67bf4ed88166e73916 100644 (file)
--- a/src/tools/rbd/action/Import.cc
+++ b/src/tools/rbd/action/Import.cc
@@ -6,6 +6,7 @@
 #include "tools/rbd/Utils.h"
 #include "include/Context.h"
 #include "common/blkdev.h"
+#include "common/debug.h"
 #include "common/errno.h"
 #include "common/Throttle.h"
 #include "include/compat.h"
@@ -16,6 +17,10 @@
 #include <iostream>
 #include <boost/program_options.hpp>
 #include <boost/scoped_ptr.hpp>
+#include "include/assert.h"
+
+#define dout_context g_ceph_context
+#define dout_subsys ceph_subsys_rbd
 
 namespace rbd {
 namespace action {
@@ -216,49 +221,46 @@ static int do_image_io(ImportDiffContext *idiffctx, bool discard, size_t sparse_
   bl.append(buf, sizeof(buf));
   bufferlist::iterator p = bl.begin();
 
-  uint64_t off, len;
-  ::decode(off, p);
-  ::decode(len, p);
+  uint64_t image_offset, buffer_length;
+  ::decode(image_offset, p);
+  ::decode(buffer_length, p);
 
   if (!discard) {
-    bufferptr bp = buffer::create(len);
-    r = safe_read_exact(idiffctx->fd, bp.c_str(), len);
+    bufferptr bp = buffer::create(buffer_length);
+    r = safe_read_exact(idiffctx->fd, bp.c_str(), buffer_length);
     if (r < 0) {
       return r;
     }
 
-    // skip writing zeros
-    size_t write_offset = 0;
-    size_t write_length = 0;
-    size_t offset = 0;
-    while (offset < len) {
-      if (utils::calc_sparse_extent(bp,
-                                    sparse_size,
-                                    len,
-                                    &write_offset,
-                                    &write_length,
-                                    &offset)) {
-       bufferptr write_ptr(bp, write_offset, write_length);
-        bufferlist write_bl;
-        write_bl.push_back(write_ptr);
+    size_t buffer_offset = 0;
+    while (buffer_offset < buffer_length) {
+      size_t write_length = 0;
+      bool zeroed = false;
+      utils::calc_sparse_extent(bp, sparse_size, buffer_offset, buffer_length,
+                               &write_length, &zeroed);
+      assert(write_length > 0);
+
+      bufferlist write_bl;
+      if (!zeroed) {
+       bufferptr write_ptr(bp, buffer_offset, write_length);
+       write_bl.push_back(write_ptr);
        assert(write_bl.length() == write_length);
+      }
 
-       C_ImportDiff *ctx = new C_ImportDiff(idiffctx, write_bl,
-                                            off + write_offset, write_length,
-                                            false);
-       r = ctx->send();
-
-       if (r < 0) {
-         return r;
-       }
-        // Reset write offset and write length after current write submitted
-        write_offset = offset;
-        write_length = 0;
+      C_ImportDiff *ctx = new C_ImportDiff(idiffctx, write_bl,
+                                          image_offset + buffer_offset,
+                                          write_length, zeroed);
+      r = ctx->send();
+      if (r < 0) {
+       return r;
       }
+
+      buffer_offset += write_length;
     }
   } else {
     bufferlist data;
-    C_ImportDiff *ctx = new C_ImportDiff(idiffctx, data, off, len, true);
+    C_ImportDiff *ctx = new C_ImportDiff(idiffctx, data, image_offset,
+                                        buffer_length, true);
     return ctx->send();
   }
   return r;
@@ -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,
+                     bool no_progress, int format, size_t sparse_size)
 {
   int r;
 
@@ -356,6 +359,14 @@ int do_import_diff_fd(librbd::Image &image, int fd, bool no_progress, int format
     return r;
   }
 
+  std::string skip_partial_discard;
+  r = rados.conf_get("rbd_skip_partial_discard", skip_partial_discard);
+  if (r < 0 || skip_partial_discard != "false") {
+    dout(1) << "disabling sparse import" << dendl;
+    sparse_size = 0;
+    r = 0;
+  }
+
   // begin image import
   std::string tosnap;
   ImportDiffContext idiffctx(&image, fd, size, no_progress);
@@ -393,8 +404,8 @@ int do_import_diff_fd(librbd::Image &image, int fd, bool no_progress, int format
   return r;
 }
 
-int do_import_diff(librbd::Image &image, const char *path,
-                bool no_progress, size_t sparse_size)
+int do_import_diff(librados::Rados &rados, librbd::Image &image,
+                  const char *path, bool no_progress, size_t sparse_size)
 {
   int r;
   int fd;
@@ -409,7 +420,7 @@ int do_import_diff(librbd::Image &image, const char *path,
       return r;
     }
   }
-  r = do_import_diff_fd(image, fd, no_progress, 1, sparse_size);
+  r = do_import_diff_fd(rados, image, fd, no_progress, 1, sparse_size);
 
   if (fd != 0)
     close(fd);
@@ -460,7 +471,8 @@ int execute_diff(const po::variables_map &vm) {
     return r;
   }
 
-  r = do_import_diff(image, path.c_str(), vm[at::NO_PROGRESS].as<bool>(), sparse_size);
+  r = do_import_diff(rados, image, path.c_str(),
+                    vm[at::NO_PROGRESS].as<bool>(), sparse_size);
   if (r < 0) {
     cerr << "rbd: import-diff failed: " << cpp_strerror(r) << std::endl;
     return r;
@@ -590,9 +602,9 @@ static int do_import_header(int fd, int import_format, uint64_t &size, librbd::I
   return r;
 }
 
-static int do_import_v2(int fd, librbd::Image &image, uint64_t size,
-                        size_t imgblklen, utils::ProgressContext &pc,
-                       size_t sparse_size)
+static int do_import_v2(librados::Rados &rados, int fd, librbd::Image &image,
+                       uint64_t size, size_t imgblklen,
+                       utils::ProgressContext &pc, size_t sparse_size)
 {
   int r = 0;
   r = validate_banner(fd, utils::RBD_IMAGE_DIFFS_BANNER_V2);
@@ -612,7 +624,7 @@ static int do_import_v2(int fd, librbd::Image &image, uint64_t size,
   ::decode(diff_num, p);
 
   for (size_t i = 0; i < diff_num; i++) {
-    r = do_import_diff_fd(image, fd, true, 2, sparse_size);
+    r = do_import_diff_fd(rados, image, fd, true, 2, sparse_size);
     if (r < 0) {
       pc.fail();
       std::cerr << "rbd: import-diff failed: " << cpp_strerror(r) << std::endl;
@@ -673,26 +685,25 @@ static int do_import_v1(int fd, librbd::Image &image, uint64_t size,
 
     // write as much as we got; perhaps less than imgblklen
     // but skip writing zeros to create sparse images
-    size_t write_offset = 0;
-    size_t write_length = 0;
-    size_t offset = 0;
-    while (offset < blklen) {
-      if (utils::calc_sparse_extent(blkptr,
-                                    sparse_size,
-                                    blklen,
-                                    &write_offset,
-                                    &write_length,
-                                    &offset)) {
-        bufferptr write_ptr(blkptr, write_offset, write_length);
-        bufferlist write_bl;
-        write_bl.push_back(write_ptr);
-        C_Import *ctx = new C_Import(*throttle, image, write_bl, image_pos + write_offset);
-        ctx->send();
-
-        // Reset write offset and write length after current write submitted
-        write_offset = offset;
-        write_length = 0;
+    size_t buffer_offset = 0;
+    while (buffer_offset < blklen) {
+      size_t write_length = 0;
+      bool zeroed = false;
+      utils::calc_sparse_extent(blkptr, sparse_size, buffer_offset, blklen,
+                               &write_length, &zeroed);
+
+      if (!zeroed) {
+       bufferlist write_bl;
+       bufferptr write_ptr(blkptr, buffer_offset, write_length);
+       write_bl.push_back(write_ptr);
+       assert(write_bl.length() == write_length);
+
+       C_Import *ctx = new C_Import(*throttle, image, write_bl,
+                                    image_pos + buffer_offset);
+       ctx->send();
       }
+
+      buffer_offset += write_length;
     }
 
     // done with whole block, whether written or not
@@ -722,10 +733,10 @@ out:
   return r;
 }
 
-static int do_import(librbd::RBD &rbd, librados::IoCtx& io_ctx,
-                     const char *imgname, const char *path,
-                    librbd::ImageOptions& opts, bool no_progress,
-                    int import_format, size_t sparse_size)
+static int do_import(librados::Rados &rados, librbd::RBD &rbd,
+                    librados::IoCtx& io_ctx, const char *imgname,
+                    const char *path, librbd::ImageOptions& opts,
+                    bool no_progress, int import_format, size_t sparse_size)
 {
   int fd, r;
   struct stat stat_buf;
@@ -804,7 +815,7 @@ static int do_import(librbd::RBD &rbd, librados::IoCtx& io_ctx,
   if (import_format == 1) {
     r = do_import_v1(fd, image, size, imgblklen, pc, sparse_size);
   } else {
-    r = do_import_v2(fd, image, size, imgblklen, pc, sparse_size);
+    r = do_import_v2(rados, fd, image, size, imgblklen, pc, sparse_size);
   }
   if (r < 0) {
     std::cerr << "rbd: failed to import image" << std::endl;
@@ -908,7 +919,7 @@ int execute(const po::variables_map &vm) {
     format = vm["export-format"].as<uint64_t>();
 
   librbd::RBD rbd;
-  r = do_import(rbd, io_ctx, image_name.c_str(), path.c_str(),
+  r = do_import(rados, rbd, io_ctx, image_name.c_str(), path.c_str(),
                 opts, vm[at::NO_PROGRESS].as<bool>(), format, sparse_size);
   if (r < 0) {
     std::cerr << "rbd: import failed: " << cpp_strerror(r) << std::endl;