Merge pull request ceph#61567 from idryomov/wip-58185

idryomov · web-flow · commit b5d44d3687e8 · 2025-02-04T10:58:24.000+01:00
librbd: stop filtering async request error codes

Reviewed-by: Ramana Raja &lt;rraja@redhat.com&gt;
diff --git a/qa/suites/rbd/cli/workloads/rbd_cli_migration.yaml b/qa/suites/rbd/cli/workloads/rbd_cli_migration.yaml
@@ -1,3 +1,11 @@
+overrides:
+  install:
+    extra_system_packages:
+      rpm:
+      - qemu-kvm-block-rbd
+      deb:
+      - qemu-block-extra
+      - qemu-utils
 tasks:
 - workunit:
     clients:
diff --git a/qa/workunits/rbd/cli_migration.sh b/qa/workunits/rbd/cli_migration.sh
@@ -596,6 +596,68 @@ EOF
     kill_nbd_server
 }
 
+test_import_nbd_stream_disconnected() {
+    local dest_image=$1
+    local migration_execute_pid
+
+    dd if=/dev/urandom of=${TEMPDIR}/large.raw bs=1M count=20480
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        ${TEMPDIR}/large.raw
+
+    cat > ${TEMPDIR}/spec.json <<EOF
+{
+  "type": "raw",
+  "stream": {
+    "type": "nbd",
+    "uri": "nbd://localhost"
+  }
+}
+EOF
+    cat ${TEMPDIR}/spec.json
+
+    # server disappears while importing - abort
+    rbd migration prepare --import-only \
+        --source-spec-path ${TEMPDIR}/spec.json ${dest_image}
+    rbd status ${dest_image} | grep 'state: prepared'
+    qemu-img compare ${TEMPDIR}/large.raw rbd:rbd/${dest_image}
+    rbd migration execute ${dest_image} &
+    migration_execute_pid=$!
+    sleep $((5 + RANDOM % 35))
+    kill_nbd_server
+    expect_false wait $migration_execute_pid
+    expect_false rbd status ${dest_image}
+    rbd migration abort ${dest_image}
+
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        ${TEMPDIR}/large.raw
+
+    # server disappears while importing - resume
+    rbd migration prepare --import-only \
+        --source-spec-path ${TEMPDIR}/spec.json ${dest_image}
+    rbd status ${dest_image} | grep 'state: prepared'
+    qemu-img compare ${TEMPDIR}/large.raw rbd:rbd/${dest_image}
+    rbd migration execute ${dest_image} &
+    migration_execute_pid=$!
+    sleep $((5 + RANDOM % 35))
+    kill_nbd_server
+    expect_false wait $migration_execute_pid
+    expect_false rbd status ${dest_image}
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        ${TEMPDIR}/large.raw
+    rbd status ${dest_image} | grep 'state: executing'
+    qemu-img compare ${TEMPDIR}/large.raw rbd:rbd/${dest_image}
+    rbd migration execute ${dest_image}
+    rbd status ${dest_image} | grep 'state: executed'
+    qemu-img compare ${TEMPDIR}/large.raw rbd:rbd/${dest_image}
+    rbd migration commit ${dest_image}
+    rbd status ${dest_image} | expect_false grep 'Migration:'
+    qemu-img compare ${TEMPDIR}/large.raw rbd:rbd/${dest_image}
+
+    remove_image "${dest_image}"
+
+    kill_nbd_server
+}
+
 # make sure rbd pool is EMPTY.. this is a test script!!
 rbd ls 2>&1 | wc -l | grep -v '^0$' && echo "nonempty rbd pool, aborting!  run this script on an empty test cluster only." && exit 1
 
@@ -614,6 +676,8 @@ test_import_nbd_stream_qcow2 ${IMAGE2} ${IMAGE3}
 test_import_raw_format ${IMAGE1} ${IMAGE2}
 test_import_nbd_stream_raw ${IMAGE1} ${IMAGE2}
 
+test_import_nbd_stream_disconnected ${IMAGE2}
+
 rbd namespace create rbd/${NAMESPACE1}
 rbd namespace create rbd/${NAMESPACE2}
 create_base_image rbd/${NAMESPACE1}/${IMAGE1}
diff --git a/src/librbd/Operations.cc b/src/librbd/Operations.cc
@@ -185,7 +185,6 @@ struct C_InvokeAsyncRequest : public Context {
   bool permit_snapshot;
   boost::function<void(Context*)> local;
   boost::function<void(Context*)> remote;
-  std::set<int> filter_error_codes;
   Context *on_finish;
   bool request_lock = false;
 
@@ -194,11 +193,10 @@ struct C_InvokeAsyncRequest : public Context {
                        bool permit_snapshot,
                        const boost::function<void(Context*)>& local,
                        const boost::function<void(Context*)>& remote,
-                       const std::set<int> &filter_error_codes,
                        Context *on_finish)
     : image_ctx(image_ctx), operation(operation), request_type(request_type),
       permit_snapshot(permit_snapshot), local(local), remote(remote),
-      filter_error_codes(filter_error_codes), on_finish(on_finish) {
+      on_finish(on_finish) {
   }
 
   void send() {
@@ -382,9 +380,6 @@ struct C_InvokeAsyncRequest : public Context {
   }
 
   void finish(int r) override {
-    if (filter_error_codes.count(r) != 0) {
-      r = 0;
-    }
     on_finish->complete(r);
   }
 };
@@ -503,11 +498,8 @@ int Operations<I>::flatten(ProgressContext &prog_ctx) {
                                        m_image_ctx.image_watcher, request_id,
                                        boost::ref(prog_ctx), _1));
 
-  if (r < 0 && r != -EINVAL) {
-    return r;
-  }
   ldout(cct, 20) << "flatten finished" << dendl;
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -582,10 +574,7 @@ int Operations<I>::rebuild_object_map(ProgressContext &prog_ctx) {
                                        boost::ref(prog_ctx), _1));
 
   ldout(cct, 10) << "rebuild object map finished" << dendl;
-  if (r < 0) {
-    return r;
-  }
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -686,12 +675,9 @@ int Operations<I>::rename(const char *dstname) {
                            boost::bind(&ImageWatcher<I>::notify_rename,
                                        m_image_ctx.image_watcher, request_id,
                                        dstname, _1));
-  if (r < 0 && r != -EEXIST) {
-    return r;
-  }
 
   m_image_ctx.set_image_name(dstname);
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -874,7 +860,7 @@ void Operations<I>::snap_create(const cls::rbd::SnapshotNamespace &snap_namespac
     boost::bind(&ImageWatcher<I>::notify_snap_create, m_image_ctx.image_watcher,
                 request_id, snap_namespace, snap_name, flags,
                 boost::ref(prog_ctx), _1),
-    {-EEXIST}, on_finish);
+    on_finish);
   req->send();
 }
 
@@ -1077,7 +1063,7 @@ void Operations<I>::snap_remove(const cls::rbd::SnapshotNamespace& snap_namespac
       boost::bind(&ImageWatcher<I>::notify_snap_remove,
                   m_image_ctx.image_watcher, request_id, snap_namespace,
                   snap_name, _1),
-      {-ENOENT}, on_finish);
+      on_finish);
     req->send();
   } else {
     std::shared_lock owner_lock{m_image_ctx.owner_lock};
@@ -1173,9 +1159,6 @@ int Operations<I>::snap_rename(const char *srcname, const char *dstname) {
                              boost::bind(&ImageWatcher<I>::notify_snap_rename,
                                          m_image_ctx.image_watcher, request_id,
                                          snap_id, dstname, _1));
-    if (r < 0 && r != -EEXIST) {
-      return r;
-    }
   } else {
     C_SaferCond cond_ctx;
     {
@@ -1184,13 +1167,10 @@ int Operations<I>::snap_rename(const char *srcname, const char *dstname) {
     }
 
     r = cond_ctx.wait();
-    if (r < 0) {
-      return r;
-    }
   }
 
   m_image_ctx.perfcounter->inc(l_librbd_snap_rename);
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -1275,9 +1255,6 @@ int Operations<I>::snap_protect(const cls::rbd::SnapshotNamespace& snap_namespac
                              boost::bind(&ImageWatcher<I>::notify_snap_protect,
                                          m_image_ctx.image_watcher, request_id,
 					 snap_namespace, snap_name, _1));
-    if (r < 0 && r != -EBUSY) {
-      return r;
-    }
   } else {
     C_SaferCond cond_ctx;
     {
@@ -1286,11 +1263,9 @@ int Operations<I>::snap_protect(const cls::rbd::SnapshotNamespace& snap_namespac
     }
 
     r = cond_ctx.wait();
-    if (r < 0) {
-      return r;
-    }
   }
-  return 0;
+
+  return r;
 }
 
 template <typename I>
@@ -1373,9 +1348,6 @@ int Operations<I>::snap_unprotect(const cls::rbd::SnapshotNamespace& snap_namesp
                              boost::bind(&ImageWatcher<I>::notify_snap_unprotect,
                                          m_image_ctx.image_watcher, request_id,
 					 snap_namespace, snap_name, _1));
-    if (r < 0 && r != -EINVAL) {
-      return r;
-    }
   } else {
     C_SaferCond cond_ctx;
     {
@@ -1384,11 +1356,9 @@ int Operations<I>::snap_unprotect(const cls::rbd::SnapshotNamespace& snap_namesp
     }
 
     r = cond_ctx.wait();
-    if (r < 0) {
-      return r;
-    }
   }
-  return 0;
+
+  return r;
 }
 
 template <typename I>
@@ -1709,9 +1679,6 @@ int Operations<I>::metadata_remove(const std::string &key) {
                            boost::bind(&ImageWatcher<I>::notify_metadata_remove,
                                        m_image_ctx.image_watcher, request_id,
                                        key, _1));
-  if (r == -ENOENT) {
-    r = 0;
-  }
 
   std::string config_key;
   if (util::is_metadata_config_override(key, &config_key) && r >= 0) {
@@ -1775,11 +1742,8 @@ int Operations<I>::migrate(ProgressContext &prog_ctx) {
                                        m_image_ctx.image_watcher, request_id,
                                        boost::ref(prog_ctx), _1));
 
-  if (r < 0 && r != -EINVAL) {
-    return r;
-  }
   ldout(cct, 20) << "migrate finished" << dendl;
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -1842,11 +1806,9 @@ int Operations<I>::sparsify(size_t sparse_size, ProgressContext &prog_ctx) {
                                            m_image_ctx.image_watcher,
                                            request_id, sparse_size,
                                            boost::ref(prog_ctx), _1));
-  if (r < 0 && r != -EINVAL) {
-    return r;
-  }
+
   ldout(cct, 20) << "resparsify finished" << dendl;
-  return 0;
+  return r;
 }
 
 template <typename I>
@@ -1934,7 +1896,7 @@ int Operations<I>::invoke_async_request(
                                                              permit_snapshot,
                                                              local_request,
                                                              remote_request,
-                                                             {}, &ctx);
+                                                             &ctx);
   req->send();
   return ctx.wait();
 }
