Skip to content

Commit 3bbf1f5

Browse files
idryomovidryomov
committed
librbd/migration/NativeFormat: refactor source spec parsing
In preparation for not instantiating NativeFormat and losing a copy of the source spec JSON object in m_json_object, refactor the parsing code to use only const methods (which std::map's operator[] isn't) and local variables where possible. Signed-off-by: Ilya Dryomov <[email protected]>
1 parent 1ba9a32 commit 3bbf1f5

File tree

2 files changed

+100
-75
lines changed

2 files changed

+100
-75
lines changed

‎src/librbd/migration/NativeFormat.cc‎

Lines changed: 100 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -62,95 +62,124 @@ void NativeFormat<I>::open(Context* on_finish) {
6262
auto cct = m_image_ctx->cct;
6363
ldout(cct, 10) << dendl;
6464

65-
auto& pool_name_val = m_json_object[POOL_NAME_KEY];
66-
if (pool_name_val.type() == json_spirit::str_type) {
67-
librados::Rados rados(m_image_ctx->md_ctx);
68-
m_pool_id = rados.pool_lookup(pool_name_val.get_str().c_str());
69-
if (m_pool_id < 0) {
70-
lderr(cct) << "failed to lookup pool" << dendl;
71-
on_finish->complete(static_cast<int>(m_pool_id));
65+
int64_t pool_id = -1;
66+
std::string pool_namespace;
67+
std::string image_name;
68+
std::string image_id;
69+
70+
if (auto it = m_json_object.find(POOL_NAME_KEY);
71+
it != m_json_object.end()) {
72+
if (it->second.type() == json_spirit::str_type) {
73+
librados::Rados rados(m_image_ctx->md_ctx);
74+
pool_id = rados.pool_lookup(it->second.get_str().c_str());
75+
if (pool_id < 0) {
76+
lderr(cct) << "failed to lookup pool" << dendl;
77+
on_finish->complete(static_cast<int>(pool_id));
78+
return;
79+
}
80+
} else {
81+
lderr(cct) << "invalid pool name" << dendl;
82+
on_finish->complete(-EINVAL);
7283
return;
7384
}
74-
} else if (pool_name_val.type() != json_spirit::null_type) {
75-
lderr(cct) << "invalid pool name" << dendl;
76-
on_finish->complete(-EINVAL);
77-
return;
7885
}
7986

80-
auto& pool_id_val = m_json_object[POOL_ID_KEY];
81-
if (m_pool_id != -1 && pool_id_val.type() != json_spirit::null_type) {
82-
lderr(cct) << "cannot specify both pool name and pool id" << dendl;
83-
on_finish->complete(-EINVAL);
84-
return;
85-
} else if (pool_id_val.type() == json_spirit::int_type) {
86-
m_pool_id = pool_id_val.get_int64();
87-
} else if (pool_id_val.type() == json_spirit::str_type) {
88-
try {
89-
m_pool_id = boost::lexical_cast<int64_t>(pool_id_val.get_str());
90-
} catch (boost::bad_lexical_cast &) {
87+
if (auto it = m_json_object.find(POOL_ID_KEY);
88+
it != m_json_object.end()) {
89+
if (pool_id != -1) {
90+
lderr(cct) << "cannot specify both pool name and pool id" << dendl;
91+
on_finish->complete(-EINVAL);
92+
return;
93+
}
94+
if (it->second.type() == json_spirit::int_type) {
95+
pool_id = it->second.get_int64();
96+
} else if (it->second.type() == json_spirit::str_type) {
97+
try {
98+
pool_id = boost::lexical_cast<int64_t>(it->second.get_str());
99+
} catch (boost::bad_lexical_cast&) {
100+
}
101+
}
102+
if (pool_id == -1) {
103+
lderr(cct) << "invalid pool id" << dendl;
104+
on_finish->complete(-EINVAL);
105+
return;
91106
}
92107
}
93108

94-
if (m_pool_id == -1) {
95-
lderr(cct) << "missing or invalid pool id" << dendl;
96-
on_finish->complete(-EINVAL);
97-
return;
98-
}
99-
100-
auto& pool_namespace_val = m_json_object[POOL_NAMESPACE_KEY];
101-
if (pool_namespace_val.type() == json_spirit::str_type) {
102-
m_pool_namespace = pool_namespace_val.get_str();
103-
} else if (pool_namespace_val.type() != json_spirit::null_type) {
104-
lderr(cct) << "invalid pool namespace" << dendl;
109+
if (pool_id == -1) {
110+
lderr(cct) << "missing pool name or pool id" << dendl;
105111
on_finish->complete(-EINVAL);
106112
return;
107113
}
108114

109-
auto& image_name_val = m_json_object[IMAGE_NAME_KEY];
110-
if (image_name_val.type() != json_spirit::str_type) {
111-
lderr(cct) << "missing or invalid image name" << dendl;
112-
on_finish->complete(-EINVAL);
113-
return;
115+
if (auto it = m_json_object.find(POOL_NAMESPACE_KEY);
116+
it != m_json_object.end()) {
117+
if (it->second.type() == json_spirit::str_type) {
118+
pool_namespace = it->second.get_str();
119+
} else {
120+
lderr(cct) << "invalid pool namespace" << dendl;
121+
on_finish->complete(-EINVAL);
122+
return;
123+
}
114124
}
115-
m_image_name = image_name_val.get_str();
116125

117-
auto& image_id_val = m_json_object[IMAGE_ID_KEY];
118-
if (image_id_val.type() == json_spirit::str_type) {
119-
m_image_id = image_id_val.get_str();
120-
} else if (image_id_val.type() != json_spirit::null_type) {
121-
lderr(cct) << "invalid image id" << dendl;
126+
if (auto it = m_json_object.find(IMAGE_NAME_KEY);
127+
it != m_json_object.end()) {
128+
if (it->second.type() == json_spirit::str_type) {
129+
image_name = it->second.get_str();
130+
} else {
131+
lderr(cct) << "invalid image name" << dendl;
132+
on_finish->complete(-EINVAL);
133+
return;
134+
}
135+
} else {
136+
lderr(cct) << "missing image name" << dendl;
122137
on_finish->complete(-EINVAL);
123138
return;
124139
}
125140

126-
auto& snap_name_val = m_json_object[SNAP_NAME_KEY];
127-
if (snap_name_val.type() == json_spirit::str_type) {
128-
m_snap_name = snap_name_val.get_str();
129-
} else if (snap_name_val.type() != json_spirit::null_type) {
130-
lderr(cct) << "invalid snap name" << dendl;
131-
on_finish->complete(-EINVAL);
132-
return;
141+
if (auto it = m_json_object.find(IMAGE_ID_KEY);
142+
it != m_json_object.end()) {
143+
if (it->second.type() == json_spirit::str_type) {
144+
image_id = it->second.get_str();
145+
} else {
146+
lderr(cct) << "invalid image id" << dendl;
147+
on_finish->complete(-EINVAL);
148+
return;
149+
}
133150
}
134151

135-
auto& snap_id_val = m_json_object[SNAP_ID_KEY];
136-
if (!m_snap_name.empty() && snap_id_val.type() != json_spirit::null_type) {
137-
lderr(cct) << "cannot specify both snap name and snap id" << dendl;
138-
on_finish->complete(-EINVAL);
139-
return;
140-
} else if (snap_id_val.type() == json_spirit::str_type) {
141-
try {
142-
m_snap_id = boost::lexical_cast<uint64_t>(snap_id_val.get_str());
143-
} catch (boost::bad_lexical_cast &) {
152+
if (auto it = m_json_object.find(SNAP_NAME_KEY);
153+
it != m_json_object.end()) {
154+
if (it->second.type() == json_spirit::str_type) {
155+
m_snap_name = it->second.get_str();
156+
} else {
157+
lderr(cct) << "invalid snap name" << dendl;
158+
on_finish->complete(-EINVAL);
159+
return;
144160
}
145-
} else if (snap_id_val.type() == json_spirit::int_type) {
146-
m_snap_id = snap_id_val.get_uint64();
147161
}
148162

149-
if (snap_id_val.type() != json_spirit::null_type &&
150-
m_snap_id == CEPH_NOSNAP) {
151-
lderr(cct) << "invalid snap id" << dendl;
152-
on_finish->complete(-EINVAL);
153-
return;
163+
if (auto it = m_json_object.find(SNAP_ID_KEY);
164+
it != m_json_object.end()) {
165+
if (!m_snap_name.empty()) {
166+
lderr(cct) << "cannot specify both snap name and snap id" << dendl;
167+
on_finish->complete(-EINVAL);
168+
return;
169+
}
170+
if (it->second.type() == json_spirit::int_type) {
171+
m_snap_id = it->second.get_uint64();
172+
} else if (it->second.type() == json_spirit::str_type) {
173+
try {
174+
m_snap_id = boost::lexical_cast<uint64_t>(it->second.get_str());
175+
} catch (boost::bad_lexical_cast&) {
176+
}
177+
}
178+
if (m_snap_id == CEPH_NOSNAP) {
179+
lderr(cct) << "invalid snap id" << dendl;
180+
on_finish->complete(-EINVAL);
181+
return;
182+
}
154183
}
155184

156185
// snapshot is required for import to keep source read-only
@@ -163,21 +192,21 @@ void NativeFormat<I>::open(Context* on_finish) {
163192
// TODO add support for external clusters
164193
librados::IoCtx io_ctx;
165194
int r = util::create_ioctx(m_image_ctx->md_ctx, "source image",
166-
m_pool_id, m_pool_namespace, &io_ctx);
195+
pool_id, pool_namespace, &io_ctx);
167196
if (r < 0) {
168197
on_finish->complete(r);
169198
return;
170199
}
171200

172201
m_image_ctx->md_ctx.dup(io_ctx);
173202
m_image_ctx->data_ctx.dup(io_ctx);
174-
m_image_ctx->name = m_image_name;
203+
m_image_ctx->name = image_name;
175204

176205
uint64_t flags = 0;
177-
if (m_image_id.empty() && !m_import_only) {
206+
if (image_id.empty() && !m_import_only) {
178207
flags |= OPEN_FLAG_OLD_FORMAT;
179208
} else {
180-
m_image_ctx->id = m_image_id;
209+
m_image_ctx->id = image_id;
181210
}
182211

183212
if (m_image_ctx->child != nullptr) {

‎src/librbd/migration/NativeFormat.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,6 @@ class NativeFormat : public FormatInterface {
6262
json_spirit::mObject m_json_object;
6363
bool m_import_only;
6464

65-
int64_t m_pool_id = -1;
66-
std::string m_pool_namespace;
67-
std::string m_image_name;
68-
std::string m_image_id;
6965
std::string m_snap_name;
7066
uint64_t m_snap_id = CEPH_NOSNAP;
7167

0 commit comments

Comments
 (0)