From: Joao Eduardo Luis Date: Fri, 17 Oct 2014 17:59:51 +0000 (+0100) Subject: mon: MDSMonitor: have management_command() returning int instead of bool X-Git-Tag: v0.88~56^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0dd473cbad4f9ea403fe60badffdc6da4dd3aa3c;p=ceph.git mon: MDSMonitor: have management_command() returning int instead of bool We can more easily differentiate between errors we get out of this function, which makes the code a bit more versatile and readable. Signed-off-by: Joao Eduardo Luis --- diff --git a/src/mon/MDSMonitor.cc b/src/mon/MDSMonitor.cc index 0d32eaabe9e5..2984cf96ecc1 100644 --- a/src/mon/MDSMonitor.cc +++ b/src/mon/MDSMonitor.cc @@ -929,20 +929,30 @@ bool MDSMonitor::prepare_command(MMonCommand *m) } /* Execute filesystem add/remove, or pass through to filesystem_command */ - bool const handled = management_command(prefix, cmdmap, ss, r); - if (!handled) { - if (!pending_mdsmap.get_enabled()) { - ss << "No filesystem configured: use `ceph fs new` to create a filesystem"; - r = -ENOENT; - } else { - bool const completed = filesystem_command(m, prefix, cmdmap, ss, r); - if (!completed) { - // Do not reply, the message has been enqueued for retry - return false; - } + r = management_command(prefix, cmdmap, ss); + if (r >= 0 || r != -ENOSYS) + // MDSMonitor::management_command() returns -ENOSYS if it knows nothing + // about the command passed to it, in which case we will check whether + // MDSMonitor::filesystem_command() knows about it. If on the other hand + // the error code is different from -ENOSYS, we will treat it as is and + // behave accordingly. + goto out; + } + + if (!pending_mdsmap.get_enabled()) { + ss << "No filesystem configured: use `ceph fs new` to create a filesystem"; + r = -ENOENT; + } else { + bool const completed = filesystem_command(m, prefix, cmdmap, ss, r); + if (!completed) { + // Do not reply, the message has been enqueued for retry + return false; } + // we expect MDSMonitor::filesystem_command() to set 'r = 0' in + // case of success. } +out: /* Compose response */ string rs; getline(ss, rs); @@ -1005,11 +1015,10 @@ int MDSMonitor::_check_pool( * @return true if such a command was found, else false to * fall through and look for other types of command. */ -bool MDSMonitor::management_command( +int MDSMonitor::management_command( std::string const &prefix, map &cmdmap, - std::stringstream &ss, - int &r) + std::stringstream &ss) { if (prefix == "mds newfs") { /* Legacy `newfs` command, takes pool numbers instead of @@ -1021,24 +1030,22 @@ bool MDSMonitor::management_command( if (!cmd_getval(g_ceph_context, cmdmap, "metadata", metadata)) { ss << "error parsing 'metadata' value '" << cmd_vartype_stringify(cmdmap["metadata"]) << "'"; - r = -EINVAL; - return true; + return -EINVAL; } if (!cmd_getval(g_ceph_context, cmdmap, "data", data)) { ss << "error parsing 'data' value '" << cmd_vartype_stringify(cmdmap["data"]) << "'"; - r = -EINVAL; - return true; + return -EINVAL; } - r = _check_pool(data, &ss); - if (r) { - return true; + int r = _check_pool(data, &ss); + if (r < 0) { + return r; } r = _check_pool(metadata, &ss); - if (r) { - return true; + if (r < 0) { + return r; } // be idempotent.. success if it already exists and matches @@ -1047,43 +1054,37 @@ bool MDSMonitor::management_command( mdsmap.get_first_data_pool() == data && mdsmap.fs_name == MDS_FS_NAME_DEFAULT) { ss << "filesystem '" << MDS_FS_NAME_DEFAULT << "' already exists"; - r = 0; - return true; + return 0; } string sure; cmd_getval(g_ceph_context, cmdmap, "sure", sure); if (pending_mdsmap.get_enabled() && sure != "--yes-i-really-mean-it") { ss << "this is DANGEROUS and will wipe out the mdsmap's fs, and may clobber data in the new pools you specify. add --yes-i-really-mean-it if you do."; - r = -EPERM; - - return true; + return -EPERM; } else { newmap.inc = pending_mdsmap.inc; pending_mdsmap = newmap; pending_mdsmap.epoch = mdsmap.epoch + 1; create_new_fs(pending_mdsmap, MDS_FS_NAME_DEFAULT, metadata, data); ss << "new fs with metadata pool " << metadata << " and data pool " << data; - r = 0; - return true; + return 0; } } else if (prefix == "fs new") { string metadata_name; cmd_getval(g_ceph_context, cmdmap, "metadata", metadata_name); int64_t metadata = mon->osdmon()->osdmap.lookup_pg_pool_name(metadata_name); if (metadata < 0) { - r = -ENOENT; ss << "pool '" << metadata_name << "' does not exist"; - return true; + return -ENOENT; } string data_name; cmd_getval(g_ceph_context, cmdmap, "data", data_name); int64_t data = mon->osdmon()->osdmap.lookup_pg_pool_name(data_name); if (data < 0) { - r = -ENOENT; ss << "pool '" << data_name << "' does not exist"; - return true; + return -ENOENT; } string fs_name; @@ -1092,8 +1093,7 @@ bool MDSMonitor::management_command( // Ensure fs name is not empty so that we can implement // commmands that refer to FS by name in future. ss << "Filesystem name may not be empty"; - r = -EINVAL; - return true; + return -EINVAL; } if (pending_mdsmap.get_enabled() @@ -1101,16 +1101,14 @@ bool MDSMonitor::management_command( && *(pending_mdsmap.data_pools.begin()) == data && pending_mdsmap.metadata_pool == metadata) { // Identical FS created already, this is a no-op - r = 0; ss << "filesystem '" << fs_name << "' already exists"; - return true; + return 0; } if (pending_mdsmap.get_enabled()) { /* We currently only support one filesystem, so cannot create a second */ ss << "A filesystem already exists, use `ceph fs rm` if you wish to delete it"; - r = -EINVAL; - return true; + return -EINVAL; } pg_pool_t const *data_pool = mon->osdmon()->osdmap.get_pg_pool(data); @@ -1123,14 +1121,14 @@ bool MDSMonitor::management_command( // we believe we must. bailing out *after* we request the proposal is // bad business as we could have changed the osdmon's state and ending up // returning an error to the user. - r = _check_pool(data, &ss); - if (r) { - return true; + int r = _check_pool(data, &ss); + if (r < 0) { + return r; } r = _check_pool(metadata, &ss); - if (r) { - return true; + if (r < 0) { + return r; } // Automatically set crash_replay_interval on data pool if it @@ -1149,8 +1147,7 @@ bool MDSMonitor::management_command( pending_mdsmap.last_failure_osd_epoch = mdsmap.last_failure_osd_epoch; create_new_fs(pending_mdsmap, fs_name, metadata, data); ss << "new fs with metadata pool " << metadata << " and data pool " << data; - r = 0; - return true; + return 0; } else if (prefix == "fs rm") { // Check caller has correctly named the FS to delete // (redundant while there is only one FS, but command @@ -1160,15 +1157,13 @@ bool MDSMonitor::management_command( if (!pending_mdsmap.get_enabled() || fs_name != pending_mdsmap.fs_name) { // Consider absence success to make deletes idempotent ss << "filesystem '" << fs_name << "' does not exist"; - r = 0; - return true; + return 0; } // Check that no MDS daemons are active if (!pending_mdsmap.up.empty()) { ss << "all MDS daemons must be inactive before removing filesystem"; - r = -EINVAL; - return true; + return -EINVAL; } // Check for confirmation flag @@ -1177,8 +1172,7 @@ bool MDSMonitor::management_command( if (sure != "--yes-i-really-mean-it") { ss << "this is a DESTRUCTIVE operation and will make data in your filesystem permanently" \ "inaccessible. Add --yes-i-really-mean-it if you are sure you wish to continue."; - r = -EPERM; - return true; + return -EPERM; } MDSMap newmap; @@ -1190,10 +1184,9 @@ bool MDSMonitor::management_command( pending_mdsmap.cas_pool = -1; pending_mdsmap.created = ceph_clock_now(g_ceph_context); - r = 0; - return true; + return 0; } else { - return false; + return -ENOSYS; } } diff --git a/src/mon/MDSMonitor.h b/src/mon/MDSMonitor.h index da6239648ff7..2824f08eeb89 100644 --- a/src/mon/MDSMonitor.h +++ b/src/mon/MDSMonitor.h @@ -101,11 +101,10 @@ class MDSMonitor : public PaxosService { bool preprocess_command(MMonCommand *m); bool prepare_command(MMonCommand *m); - bool management_command( + int management_command( std::string const &prefix, map &cmdmap, - std::stringstream &ss, - int &r); + std::stringstream &ss); bool filesystem_command( MMonCommand *m, std::string const &prefix,