librbd/migration/NBDStream: switch to NBD URIs

This removes the constraint on the transport being TCP, allowing to
use a Unix domain socket or other options.  It also allows specifying
export names which a) are needed in case of serving different content
on different exports and b) some servers may require regardless.

Additionally, NBD URIs are future proof as all that NBDStream needs to
do is forward the string to libnbd.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
(cherry picked from commit 90b55cfdc25e9ad5247f076ff48a80e91d907adb)

diff --git a/doc/rbd/rbd-live-migration.rst b/doc/rbd/rbd-live-migration.rst
index 7584338ab35d4ba6b2b12059e106176b006d0b7f..a117bed362314b0422116584b7eb0d72c0487248 100644 (file)
--- a/doc/rbd/rbd-live-migration.rst
+++ b/doc/rbd/rbd-live-migration.rst
@@ -313,11 +313,25 @@ The ``nbd`` stream can be used to import from a remote NBD export. Its
             <format unique parameters>
             "stream": {
                 "type": "nbd",
-                "server": "<server>",
-                "port": "<port>"
+                "uri": "<nbd-uri>",
             }
         }
 
+For example, to import a raw-format image from an NBD export located at
+``nbd://nbd.ceph.com`` with export name ``image.raw``, its ``source-spec``
+JSON is encoded as follows::
+
+        {
+            "type": "raw",
+            "stream": {
+                "type": "nbd",
+                "uri": "nbd://nbd.ceph.com/image.raw",
+            }
+        }
+
+``nbd-uri`` parameter should follow the `NBD URI specification`_. The
+default NBD port is ``10809``.
+
 
 Execute Migration
 =================
@@ -383,3 +397,4 @@ to the original source image being restored::
 
 
 .. _layered images: ../rbd-snapshot/#layering
+.. _NBD URI specification: https://github.com/NetworkBlockDevice/nbd/blob/master/doc/uri.md

diff --git a/qa/workunits/rbd/cli_migration.sh b/qa/workunits/rbd/cli_migration.sh
index 8faf8b0176e935888fbcd2b14da16847d4d6aa1c..b543bd383b639455560a129aca33ad9c6f8f90b2 100755 (executable)
--- a/qa/workunits/rbd/cli_migration.sh
+++ b/qa/workunits/rbd/cli_migration.sh
@@ -400,7 +400,7 @@ EOF
     remove_image "${dest_image}"
 }
 
-test_import_nbd_stream() {
+test_import_nbd_stream_qcow2() {
     local base_image=$1
     local dest_image=$2
 
@@ -412,8 +412,7 @@ test_import_nbd_stream() {
   "type": "raw",
   "stream": {
     "type": "nbd",
-    "server": "localhost",
-    "port": "10809"
+    "uri": "nbd://localhost"
   }
 }
 EOF
@@ -433,6 +432,142 @@ EOF
     compare_images ${base_image} ${dest_image}
     remove_image "${dest_image}"
 
+    # shortest possible URI
+    rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd://"}}' \
+        ${dest_image}
+    rbd migration abort ${dest_image}
+
+    # non-existing export name
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd:///myexport"}}' \
+        ${dest_image}
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd://localhost/myexport"}}' \
+        ${dest_image}
+
+    kill_nbd_server
+    qemu-nbd --export-name myexport -f qcow2 --read-only --shared 10 --persistent --fork \
+        ${TEMPDIR}/${base_image}.qcow2
+
+    rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd:///myexport"}}' \
+        ${dest_image}
+    rbd migration abort ${dest_image}
+
+    rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd://localhost/myexport"}}' \
+        ${dest_image}
+    rbd migration abort ${dest_image}
+
+    kill_nbd_server
+
+    # server not running
+    expect_false rbd migration prepare --import-only \
+        --source-spec-path ${TEMPDIR}/spec.json ${dest_image}
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd://"}}' \
+        ${dest_image}
+
+    # no URI
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd"}}' \
+        ${dest_image}
+
+    # invalid URI
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": 123456}}' \
+        ${dest_image}
+
+    # libnbd - nbd_get_errno() returns an error
+    # nbd_connect_uri: unknown URI scheme: NULL: Invalid argument (errno = 22)
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": ""}}' \
+        ${dest_image}
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "foo.example.com"}}' \
+        ${dest_image}
+
+    # libnbd - nbd_get_errno() returns 0, EIO fallback
+    # nbd_connect_uri: getaddrinfo: foo.example.com:10809: Name or service not known (errno = 0)
+    expect_false rbd migration prepare --import-only \
+        --source-spec '{"type": "raw", "stream": {"type": "nbd", "uri": "nbd://foo.example.com"}}' \
+        ${dest_image}
+}
+
+test_import_nbd_stream_raw() {
+    local base_image=$1
+    local dest_image=$2
+
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        --socket ${TEMPDIR}/qemu-nbd-${base_image} ${TEMPDIR}/${base_image}
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        --socket ${TEMPDIR}/qemu-nbd-${base_image}@1 ${TEMPDIR}/${base_image}@1
+    qemu-nbd -f raw --read-only --shared 10 --persistent --fork \
+        --socket ${TEMPDIR}/qemu-nbd-${base_image}@2 ${TEMPDIR}/${base_image}@2
+
+    cat > ${TEMPDIR}/spec.json <<EOF
+{
+  "type": "raw",
+  "stream": {
+    "type": "nbd",
+    "uri": "nbd+unix:///?socket=${TEMPDIR}/qemu-nbd-${base_image}"
+  },
+  "snapshots": [{
+    "type": "raw",
+    "name": "snap1",
+    "stream": {
+      "type": "nbd",
+      "uri": "nbd+unix:///?socket=${TEMPDIR}/qemu-nbd-${base_image}@1"
+     }
+  }, {
+    "type": "raw",
+    "name": "snap2",
+    "stream": {
+      "type": "nbd",
+      "uri": "nbd+unix:///?socket=${TEMPDIR}/qemu-nbd-${base_image}@2"
+     }
+  }]
+}
+EOF
+    cat ${TEMPDIR}/spec.json
+
+    rbd migration prepare --import-only \
+        --source-spec-path ${TEMPDIR}/spec.json ${dest_image}
+
+    rbd snap create ${dest_image}@head
+    rbd bench --io-type write --io-pattern rand --io-size 32K --io-total 4M ${dest_image}
+
+    compare_images "${base_image}@1" "${dest_image}@snap1"
+    compare_images "${base_image}@2" "${dest_image}@snap2"
+    compare_images "${base_image}" "${dest_image}@head"
+
+    rbd migration abort ${dest_image}
+
+    cat ${TEMPDIR}/spec.json | rbd migration prepare --import-only \
+        --source-spec-path - ${dest_image}
+
+    rbd snap create ${dest_image}@head
+    rbd bench --io-type write --io-pattern rand --io-size 64K --io-total 8M ${dest_image}
+
+    compare_images "${base_image}@1" "${dest_image}@snap1"
+    compare_images "${base_image}@2" "${dest_image}@snap2"
+    compare_images "${base_image}" "${dest_image}@head"
+
+    rbd migration execute ${dest_image}
+
+    compare_images "${base_image}@1" "${dest_image}@snap1"
+    compare_images "${base_image}@2" "${dest_image}@snap2"
+    compare_images "${base_image}" "${dest_image}@head"
+
+    rbd migration commit ${dest_image}
+
+    compare_images "${base_image}@1" "${dest_image}@snap1"
+    compare_images "${base_image}@2" "${dest_image}@snap2"
+    compare_images "${base_image}" "${dest_image}@head"
+
+    remove_image "${dest_image}"
+
     kill_nbd_server
 }
 
@@ -449,8 +584,9 @@ test_import_native_format ${IMAGE1} ${IMAGE2}
 test_import_qcow_format ${IMAGE1} ${IMAGE2}
 
 test_import_qcow2_format ${IMAGE2} ${IMAGE3}
-test_import_nbd_stream ${IMAGE2} ${IMAGE3}
+test_import_nbd_stream_qcow2 ${IMAGE2} ${IMAGE3}
 
 test_import_raw_format ${IMAGE1} ${IMAGE2}
+test_import_nbd_stream_raw ${IMAGE1} ${IMAGE2}
 
 echo OK

diff --git a/src/librbd/migration/NBDStream.cc b/src/librbd/migration/NBDStream.cc
index b4d30206c631bcfef86bc2cc287f2faa5ef72a8e..c9eb618a24bfc3f4779ae388f899f80407b50dad 100644 (file)
--- a/src/librbd/migration/NBDStream.cc
+++ b/src/librbd/migration/NBDStream.cc
@@ -14,8 +14,7 @@ namespace migration {
 
 namespace {
 
-const std::string SERVER_KEY {"server"};
-const std::string PORT_KEY {"port"};
+const std::string URI_KEY{"uri"};
 
 int from_nbd_errno(int rc) {
   // nbd_get_errno() needs a default/fallback error:
@@ -209,24 +208,25 @@ NBDStream<I>::~NBDStream() {
 
 template <typename I>
 void NBDStream<I>::open(Context* on_finish) {
+  std::string uri;
   int rc;
 
-  auto& server_value = m_json_object[SERVER_KEY];
-  if (server_value.type() != json_spirit::str_type) {
-    lderr(m_cct) << "failed to locate '" << SERVER_KEY << "' key" << dendl;
-    on_finish->complete(-EINVAL);
-    return;
-  }
-
-  auto& port_value = m_json_object[PORT_KEY];
-  if (port_value.type() != json_spirit::str_type) {
-    lderr(m_cct) << "failed to locate '" << PORT_KEY << "' key" << dendl;
+  if (auto it = m_json_object.find(URI_KEY);
+      it != m_json_object.end()) {
+    if (it->second.type() == json_spirit::str_type) {
+      uri = it->second.get_str();
+    } else {
+      lderr(m_cct) << "invalid URI" << dendl;
+      on_finish->complete(-EINVAL);
+      return;
+    }
+  } else {
+    lderr(m_cct) << "missing URI" << dendl;
     on_finish->complete(-EINVAL);
     return;
   }
 
-  const char *m_server = &(server_value.get_str())[0];
-  const char *m_port = &(port_value.get_str())[0];
+  ldout(m_cct, 10) << "uri=" << uri << dendl;
 
   m_nbd = nbd_create();
   if (m_nbd == nullptr) {
@@ -246,18 +246,15 @@ void NBDStream<I>::open(Context* on_finish) {
     return;
   }
 
-  rc = nbd_connect_tcp(m_nbd, m_server, m_port);
+  rc = nbd_connect_uri(m_nbd, uri.c_str());
   if (rc == -1) {
     rc = nbd_get_errno();
-    lderr(m_cct) << "connect_tcp: " << nbd_get_error()
+    lderr(m_cct) << "connect_uri: " << nbd_get_error()
                  << " (errno = " << rc << ")" << dendl;
     on_finish->complete(from_nbd_errno(rc));
     return;
   }
 
-  ldout(m_cct, 20) << "server=" << m_server << ", "
-                   << "port=" << m_port << dendl;
-
   on_finish->complete(0);
 }
 

diff --git a/src/test/librbd/migration/test_mock_NBDStream.cc b/src/test/librbd/migration/test_mock_NBDStream.cc
index bf54b65c31484cc1c8ae9b1097263bbf1770c693..067749d76e352e4f64c9c7f9fd53a41800ab37fb 100644 (file)
--- a/src/test/librbd/migration/test_mock_NBDStream.cc
+++ b/src/test/librbd/migration/test_mock_NBDStream.cc
@@ -35,23 +35,37 @@ public:
     TestMockFixture::SetUp();
 
     ASSERT_EQ(0, open_image(m_image_name, &m_image_ctx));
-    json_object["url"] = "localhost";
-    json_object["port"] = "10809";
+    m_json_object["uri"] = "nbd://foo.example";
   }
 
   librbd::ImageCtx *m_image_ctx;
-  json_spirit::mObject json_object;
+  json_spirit::mObject m_json_object;
 };
 
-TEST_F(TestMockMigrationNBDStream, OpenClose) {
+TEST_F(TestMockMigrationNBDStream, OpenInvalidURI) {
   MockTestImageCtx mock_image_ctx(*m_image_ctx);
 
-  MockNBDStream mock_nbd_stream(&mock_image_ctx, json_object);
+  m_json_object["uri"] = 123;
+  MockNBDStream mock_nbd_stream(&mock_image_ctx, m_json_object);
 
   C_SaferCond ctx1;
   mock_nbd_stream.open(&ctx1);
-  // Since we don't have an nbd server running, we actually expect a failure.
-  ASSERT_EQ(-22, ctx1.wait());
+  ASSERT_EQ(-EINVAL, ctx1.wait());
+
+  C_SaferCond ctx2;
+  mock_nbd_stream.close(&ctx2);
+  ASSERT_EQ(0, ctx2.wait());
+}
+
+TEST_F(TestMockMigrationNBDStream, OpenMissingURI) {
+  MockTestImageCtx mock_image_ctx(*m_image_ctx);
+
+  m_json_object.clear();
+  MockNBDStream mock_nbd_stream(&mock_image_ctx, m_json_object);
+
+  C_SaferCond ctx1;
+  mock_nbd_stream.open(&ctx1);
+  ASSERT_EQ(-EINVAL, ctx1.wait());
 
   C_SaferCond ctx2;
   mock_nbd_stream.close(&ctx2);