From e9e2ceefc9a89b8d3ff080ff7649feb0017396a5 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 2 Sep 2024 22:17:40 +0200 Subject: [PATCH] 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 (cherry picked from commit 90b55cfdc25e9ad5247f076ff48a80e91d907adb) --- doc/rbd/rbd-live-migration.rst | 19 ++- qa/workunits/rbd/cli_migration.sh | 144 +++++++++++++++++- src/librbd/migration/NBDStream.cc | 35 ++--- .../librbd/migration/test_mock_NBDStream.cc | 28 +++- 4 files changed, 194 insertions(+), 32 deletions(-) diff --git a/doc/rbd/rbd-live-migration.rst b/doc/rbd/rbd-live-migration.rst index 7584338ab35..a117bed3623 100644 --- 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 "stream": { "type": "nbd", - "server": "", - "port": "" + "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 8faf8b0176e..b543bd383b6 100755 --- 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 <::~NBDStream() { template void NBDStream::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::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 bf54b65c314..067749d76e3 100644 --- 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); -- 2.39.5