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>

diff --git a/doc/rbd/rbd-live-migration.rst b/doc/rbd/rbd-live-migration.rst
index aaaf8d7a2d112d7e67ac84f197f5e5654e4ec7af..c2e0915b21bb09e53df03ab4d43b8e95b378b09c 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 f278837338ca8be2e042004fd690a4cfbb5cd262..771d6bf8f1af7b46825e09e40ad5c84273a3e3bc 100755 (executable)
--- a/qa/workunits/rbd/cli_migration.sh
+++ b/qa/workunits/rbd/cli_migration.sh
@@ -395,7 +395,7 @@ EOF
     remove_image "${dest_image}"
 }
 
-test_import_nbd_stream() {
+test_import_nbd_stream_qcow2() {
     local base_image=$1
     local dest_image=$2
 
@@ -407,8 +407,7 @@ test_import_nbd_stream() {
   "type": "raw",
   "stream": {
     "type": "nbd",
-    "server": "localhost",
-    "port": "10809"
+    "uri": "nbd://localhost"
   }
 }
 EOF
@@ -428,6 +427,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
 }
 
@@ -444,8 +579,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);