Skip to content

Commit 0bcb5cf

Browse files
committed
librbd/image: create rbd_trash object during RBD pool initialization
... and RBD namespace creation. It was not possible to remove a RBD image when OSDs were full and the 'rbd_trash' object was not already created in the image's pool or pool namespace. The 'rbd_trash' object was created in a pool or namespace during the first instance of image removal from the pool or namespace. If no images were ever removed from a RBD pool or namespace and the OSDs became full, removal of images using the CLI failed. The failure occured when trying to move the images to trash since the 'rbd_trash' object was missing in the pool or namespace. Fix this issue by creating the rbd_trash object in a pool when initalizing the pool as a RBD pool and when creating a RBD namespace. Fixes: https://tracker.ceph.com/issues/64800 Signed-off-by: Ramana Raja <[email protected]>
1 parent bdc50a9 commit 0bcb5cf

File tree

3 files changed

+102
-11
lines changed

3 files changed

+102
-11
lines changed

src/librbd/api/Namespace.cc

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,29 +64,47 @@ int Namespace<I>::create(librados::IoCtx& io_ctx, const std::string& name)
6464
return r;
6565
}
6666

67+
int ret_val;
6768
librados::IoCtx ns_ctx;
6869
ns_ctx.dup(io_ctx);
6970
ns_ctx.set_namespace(name);
7071

72+
r = ns_ctx.create(RBD_TRASH, false);
73+
if (r < 0) {
74+
lderr(cct) << "failed to create trash: " << cpp_strerror(r) << dendl;
75+
goto remove_namespace;
76+
}
77+
7178
r = cls_client::dir_state_set(&ns_ctx, RBD_DIRECTORY,
7279
cls::rbd::DIRECTORY_STATE_READY);
7380
if (r < 0) {
7481
lderr(cct) << "failed to initialize image directory: " << cpp_strerror(r)
7582
<< dendl;
76-
goto rollback;
83+
goto remove_dir_and_trash;
7784
}
7885

7986
return 0;
8087

81-
rollback:
82-
int ret_val = ns_ctx.remove(RBD_DIRECTORY);
88+
remove_dir_and_trash:
89+
// dir_state_set method may fail before or after implicitly creating
90+
// rbd_directory object
91+
ret_val = ns_ctx.remove(RBD_DIRECTORY);
8392
if (ret_val < 0 && ret_val != -ENOENT) {
84-
lderr(cct) << "failed to remove image directory: " << cpp_strerror(ret_val) << dendl;
93+
lderr(cct) << "failed to remove image directory: " << cpp_strerror(ret_val)
94+
<< dendl;
8595
}
8696

97+
ret_val = ns_ctx.remove(RBD_TRASH);
98+
if (ret_val < 0) {
99+
lderr(cct) << "failed to remove trash: " << cpp_strerror(ret_val)
100+
<< dendl;
101+
}
102+
103+
remove_namespace:
87104
ret_val = cls_client::namespace_remove(&default_ns_ctx, name);
88105
if (ret_val < 0) {
89-
lderr(cct) << "failed to remove namespace: " << cpp_strerror(ret_val) << dendl;
106+
lderr(cct) << "failed to remove namespace: " << cpp_strerror(ret_val)
107+
<< dendl;
90108
}
91109

92110
return r;

src/librbd/api/Pool.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,14 @@ int Pool<I>::init(librados::IoCtx& io_ctx, bool force) {
243243

244244
int r = io_ctx.application_enable(pg_pool_t::APPLICATION_NAME_RBD, force);
245245
if (r < 0) {
246+
lderr(cct) << "failed to enable RBD application: " << cpp_strerror(r)
247+
<< dendl;
248+
return r;
249+
}
250+
251+
r = io_ctx.create(RBD_TRASH, false);
252+
if (r < 0) {
253+
lderr(cct) << "failed to create trash: " << cpp_strerror(r) << dendl;
246254
return r;
247255
}
248256

src/test/librbd/test_librbd.cc

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2172,7 +2172,32 @@ static void remove_full_try(rados_ioctx_t ioctx, const std::string& image_name,
21722172

21732173
ASSERT_EQ(0, rbd_close(image));
21742174

2175-
// make sure we have latest map that marked the pool full
2175+
// If using a separate data pool, also make the metadata pool full
2176+
char pool_name[80];
2177+
ASSERT_GT(rados_ioctx_get_pool_name(ioctx, pool_name, sizeof(pool_name)), 0);
2178+
if (pool_name != data_pool_name) {
2179+
struct rados_pool_stat_t st;
2180+
ASSERT_EQ(0, rados_ioctx_pool_stat(ioctx, &st));
2181+
cmdstr = "{\"prefix\": \"osd pool set-quota\", \"pool\": \"" +
2182+
std::string(pool_name) + "\", \"field\": \"max_objects\", \"val\": \"" +
2183+
std::to_string(st.num_objects) + "\"}";
2184+
cmd[0] = (char *)cmdstr.c_str();
2185+
ASSERT_EQ(0, rados_mon_command(rados_ioctx_get_cluster(ioctx),
2186+
(const char **)cmd, 1, "", 0, nullptr, 0,
2187+
nullptr, 0));
2188+
2189+
for (int i = 0; i < 50; i++) {
2190+
auto temp_image_name = TestLibRBD::get_temp_image_name();
2191+
ret = create_image(ioctx, temp_image_name.c_str(), size, &order);
2192+
if (ret < 0) {
2193+
break;
2194+
}
2195+
sleep(1);
2196+
}
2197+
ASSERT_EQ(ret, -EDQUOT);
2198+
}
2199+
2200+
// make sure we have latest map that marked the pool(s) full
21762201
ASSERT_EQ(0, rados_wait_for_latest_osdmap(rados_ioctx_get_cluster(ioctx)));
21772202
ASSERT_EQ(0, rbd_remove(ioctx, image_name.c_str()));
21782203
}
@@ -2185,17 +2210,13 @@ TEST_F(TestLibRBD, RemoveFullTry)
21852210
rados_ioctx_t ioctx;
21862211
auto pool_name = create_pool(true);
21872212
ASSERT_EQ(0, rados_ioctx_create(_cluster, pool_name.c_str(), &ioctx));
2213+
ASSERT_EQ(0, rbd_pool_init(ioctx, true));
21882214
// cancel out rbd_default_data_pool -- we need an image without
21892215
// a separate data pool
21902216
ASSERT_EQ(0, rbd_pool_metadata_set(ioctx, "conf_rbd_default_data_pool",
21912217
pool_name.c_str()));
21922218

2193-
int order = 0;
21942219
auto image_name = get_temp_image_name();
2195-
// FIXME: this is a workaround for rbd_trash object being created
2196-
// on the first remove -- pre-create it to avoid bumping into quota
2197-
ASSERT_EQ(0, create_image(ioctx, image_name.c_str(), 0, &order));
2198-
ASSERT_EQ(0, rbd_remove(ioctx, image_name.c_str()));
21992220
remove_full_try(ioctx, image_name, pool_name);
22002221

22012222
rados_ioctx_destroy(ioctx);
@@ -2207,12 +2228,56 @@ TEST_F(TestLibRBD, RemoveFullTryDataPool)
22072228
REQUIRE(!is_rbd_pwl_enabled((CephContext *)_rados.cct()));
22082229
REQUIRE(!is_librados_test_stub(_rados));
22092230

2231+
rados_ioctx_t ioctx;
2232+
auto pool_name = create_pool(true);
2233+
auto data_pool_name = create_pool(true);
2234+
ASSERT_EQ(0, rados_ioctx_create(_cluster, pool_name.c_str(), &ioctx));
2235+
ASSERT_EQ(0, rbd_pool_init(ioctx, true));
2236+
ASSERT_EQ(0, rbd_pool_metadata_set(ioctx, "conf_rbd_default_data_pool",
2237+
data_pool_name.c_str()));
2238+
2239+
auto image_name = get_temp_image_name();
2240+
remove_full_try(ioctx, image_name, data_pool_name);
2241+
2242+
rados_ioctx_destroy(ioctx);
2243+
}
2244+
2245+
TEST_F(TestLibRBD, RemoveFullTryNamespace)
2246+
{
2247+
REQUIRE_FORMAT_V2();
2248+
REQUIRE(!is_rbd_pwl_enabled((CephContext *)_rados.cct()));
2249+
REQUIRE(!is_librados_test_stub(_rados));
2250+
2251+
rados_ioctx_t ioctx;
2252+
auto pool_name = create_pool(true);
2253+
ASSERT_EQ(0, rados_ioctx_create(_cluster, pool_name.c_str(), &ioctx));
2254+
// cancel out rbd_default_data_pool -- we need an image without
2255+
// a separate data pool
2256+
ASSERT_EQ(0, rbd_pool_metadata_set(ioctx, "conf_rbd_default_data_pool",
2257+
pool_name.c_str()));
2258+
ASSERT_EQ(0, rbd_namespace_create(ioctx, "name1"));
2259+
rados_ioctx_set_namespace(ioctx, "name1");
2260+
2261+
auto image_name = get_temp_image_name();
2262+
remove_full_try(ioctx, image_name, pool_name);
2263+
2264+
rados_ioctx_destroy(ioctx);
2265+
}
2266+
2267+
TEST_F(TestLibRBD, RemoveFullTryNamespaceDataPool)
2268+
{
2269+
REQUIRE_FORMAT_V2();
2270+
REQUIRE(!is_rbd_pwl_enabled((CephContext *)_rados.cct()));
2271+
REQUIRE(!is_librados_test_stub(_rados));
2272+
22102273
rados_ioctx_t ioctx;
22112274
auto pool_name = create_pool(true);
22122275
auto data_pool_name = create_pool(true);
22132276
ASSERT_EQ(0, rados_ioctx_create(_cluster, pool_name.c_str(), &ioctx));
22142277
ASSERT_EQ(0, rbd_pool_metadata_set(ioctx, "conf_rbd_default_data_pool",
22152278
data_pool_name.c_str()));
2279+
ASSERT_EQ(0, rbd_namespace_create(ioctx, "name1"));
2280+
rados_ioctx_set_namespace(ioctx, "name1");
22162281

22172282
auto image_name = get_temp_image_name();
22182283
remove_full_try(ioctx, image_name, data_pool_name);

0 commit comments

Comments
 (0)