Ceph » CephFS

It looks like MDSDaemon.cc uses CEPHFS_* error codes to initialize the MCommandReply:

...
if (!session->auth_caps.allow_all()) {
    dout(1) << __func__
      << ": received command from client without `tell` capability: " 
      << *m->get_connection()->peer_addrs << dendl;

    ss << "permission denied";
    r = -CEPHFS_EACCES;
  } else if (m->cmd.empty()) {
    r = -CEPHFS_EINVAL;
    ss << "no command given";
  } else if (!TOPNSPC::common::cmdmap_from_json(m->cmd, &cmdmap, ss)) {
    r = -CEPHFS_EINVAL;
  } else {
    cct->get_admin_socket()->queue_tell_command(m);
    return;
  }

  auto reply = make_message<MCommandReply>(r, ss.str());
...

Therefore, the usage of errorcode32_t in the MCommandReply doesn't change anything, since it's encode_payload method converts host error code to ceph error code, as shown above. But if the value is ALREADY set to be CEPHFS_, the effect is not clear.

Either the example of MCommandReply as a template for changing MClientReply is incorrect, or, the idea was to change ALL error codes in MDS to native (including for MCommandReply), and then it make sense to use errorcode32_t.

The receiver will get CEPHFS_* code from the wire, and will convert it to CEPHFS_* code (the get_result function described above), and it doesn't matter HOW the value on the wire was set, by explicitly using CEPH_FS_* in the handler code or by converting from the native value, while encoding.

Moreover, the whole MDS codebase uses CEPH_FS code, not only for the return values in the messages but for the internal function calls statuses:

void MDLog::_recovery_thread(MDSContext *completion)
{
  ceph_assert(journaler == NULL);
  if (g_conf()->mds_journal_format > JOURNAL_FORMAT_MAX) {
      dout(0) << "Configuration value for mds_journal_format is out of bounds, max is " 
              << JOURNAL_FORMAT_MAX << dendl;

      // Oh dear, something unreadable in the store for this rank: require
      // operator intervention.
      mds->damaged_unlocked();
      ceph_abort();  // damaged should not return
  }

  // First, read the pointer object.
  // If the pointer object is not present, then create it with
  // front = default ino and back = null
  JournalPointer jp(mds->get_nodeid(), mds->get_metadata_pool());
  const int read_result = jp.load(mds->objecter);

  // here
  *if (read_result == -CEPHFS_ENOENT) {*
  //

    inodeno_t const default_log_ino = MDS_INO_LOG_OFFSET + mds->get_nodeid();
    jp.front = default_log_ino;
    int write_result = jp.save(mds->objecter);
    if (write_result < 0) {
      std::lock_guard l(mds->mds_lock);
      if (mds->is_daemon_stopping()) {
        return;
      }
      mds->damaged();
      ceph_abort();  // damaged should never return
    }

If the idea was to COMPLETELY replace CEPHFS_* to host errno, it make sese. Otherwisze, I don't see any advantage of using one type of errors for the "internal" cases and another type for messages.

Igor Golikov wrote in #note-3:

It looks like MDSDaemon.cc uses CEPHFS_* error codes to initialize the MCommandReply:
[...]

Therefore, the usage of errorcode32_t in the MCommandReply doesn't change anything, since it's encode_payload method converts host error code to ceph error code, as shown above. But if the value is ALREADY set to be CEPHFS_, the effect is not clear.

Either the example of MCommandReply as a template for changing MClientReply is incorrect, or, the idea was to change ALL error codes in MDS to native (including for MCommandReply), and then it make sense to use errorcode32_t.

Yes, that's what this ticket is about. The CEPHFS_ errnos is an incorrect fix (see blame history). Unfortunately, the magic of errorcode32_t wasn't understood until Leonid ran into issues with wrong error codes on Mac.

The receiver will get CEPHFS_* code from the wire, and will convert it to CEPHFS_* code (the get_result function described above), and it doesn't matter HOW the value on the wire was set, by explicitly using CEPH_FS_* in the handler code or by converting from the native value, while encoding.

Moreover, the whole MDS codebase uses CEPH_FS code, not only for the return values in the messages but for the internal function calls statuses:

[...]

If the idea was to COMPLETELY replace CEPHFS_* to host errno, it make sese. Otherwisze, I don't see any advantage of using one type of errors for the "internal" cases and another type for messages.

Yes, let's use host errnos uniformly on client and MDS. Document heavily in MClientReply and MCommandReply that there is a conversion.

Thanks for the clarification, @Patrick Donnelly