Sorry I've been slow to review this! On Tue, 23 Dec 2014, Loic Dachary wrote: > Hi Andreas, > > I took a closer look at https://github.com/ceph/ceph/pull/2730 implementing rados whereis [--dns] and I think it deserves a discussion here. If I understand correctly, it relies on a new function of the rados API: > > typedef struct whereis { > int64_t osd_id; //< ID of the OSD hosting this object Perhaps it would be better to have int32_t primary_osd; vector<int32_t> acting_osds; > std::string osd_state; //< state of the OSD - either 'active' or 'inactive' I don't think this makes here. Only 'up' OSDs are eligible for the acting set (or the primary). > int64_t pg_seed; //< Seed of the PG hosting this object Yes > std::string ip_string; //< Ip as string I think we should use sockaddr_storage here, if we include the address at all. But perhaps it makes more sense to get the address for a given OSD id with a separate call... > std::vector<std::string> host_names; //< optional reverse DNS HostNames > std::map<std::string, std::string> user_map; //< optional user KV map > void resolve(); //< reverse DNS OSD IPs and store in HostNames I don't think these belong in the interface at all. > } whereis_t; > > static int whereis(IoCtx &ioctx, const std::string &oid, std::vector<whereis_t> &locations); get_object_location? map_object_location? and return a single struct. sage > which needs to be added there because the rados API does not expose some details that are needed to fill the fields of the whereis_t structure. > > It looks fine to me but ... I'm not used to maintaining or developing the rados API and someone else may have a more informed opinion. > > There is a technical detail that also needs to be sorted out : the current implementation exposes the RadosWhereis class (for dump) and this should either be moved to rados.cc or be part of the rados API (which probably is not the best option because it would also expose Formatter as a consequence). > > Cheers > -- > Lo?c Dachary, Artisan Logiciel Libre > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
