Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rbd: fix rbd children listing when child is in trash and add rbd_list_children2 method #18483

Merged
merged 3 commits into from
Nov 14, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/include/rbd/librbd.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ extern "C" {
#define RBD_FLAG_OBJECT_MAP_INVALID (1<<0)
#define RBD_FLAG_FAST_DIFF_INVALID (1<<1)

typedef void *rbd_snap_t;
typedef void *rbd_image_t;
typedef void *rbd_image_options_t;

Expand All @@ -73,6 +72,13 @@ typedef struct {
const char *name;
} rbd_snap_info_t;

typedef struct {
const char *pool_name;
const char *image_name;
const char *image_id;
bool trash;
} rbd_child_info_t;

#define RBD_MAX_IMAGE_NAME_SIZE 96
#define RBD_MAX_BLOCK_NAME_SIZE 24

Expand Down Expand Up @@ -529,6 +535,12 @@ CEPH_RBD_API int rbd_flatten_with_progress(rbd_image_t image,
CEPH_RBD_API ssize_t rbd_list_children(rbd_image_t image, char *pools,
size_t *pools_len, char *images,
size_t *images_len);
CEPH_RBD_API int rbd_list_children2(rbd_image_t image,
rbd_child_info_t *children,
int *max_children);
CEPH_RBD_API void rbd_list_child_cleanup(rbd_child_info_t *child);
CEPH_RBD_API void rbd_list_children_cleanup(rbd_child_info_t *children,
size_t num_children);

/**
* @defgroup librbd_h_locking Advisory Locking
Expand Down
12 changes: 12 additions & 0 deletions src/include/rbd/librbd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ namespace librbd {
time_t deferment_end_time;
} trash_image_info_t;

typedef struct {
std::string pool_name;
std::string image_name;
std::string image_id;
bool trash;
} child_info_t;

class CEPH_RBD_API RBD
{
public:
Expand Down Expand Up @@ -306,6 +313,11 @@ class CEPH_RBD_API Image
* of this image at the currently set snapshot.
*/
int list_children(std::set<std::pair<std::string, std::string> > *children);
/**
* Returns a structure of poolname, imagename, imageid and trash flag
* for each clone of this image at the currently set snapshot.
*/
int list_children2(std::vector<librbd::child_info_t> *children);

/* advisory locking (see librbd.h for details) */
int list_lockers(std::list<locker_t> *lockers,
Expand Down
53 changes: 42 additions & 11 deletions src/librbd/internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,15 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
return 0;
}

bool compare_by_name(const child_info_t& c1, const child_info_t& c2)
{
if (c1.pool_name != c2.pool_name)
return c1.pool_name < c2.pool_name;
else if (c1.image_name != c2.image_name)
return c1.image_name < c2.image_name;
else
return false;
}

} // anonymous namespace

Expand Down Expand Up @@ -645,7 +654,8 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
return 0;
}

int list_children(ImageCtx *ictx, set<pair<string, string> >& names)
int list_children(ImageCtx *ictx,
vector<child_info_t> *names)
{
CephContext *cct = ictx->cct;
ldout(cct, 20) << "children list " << ictx->name << dendl;
Expand All @@ -665,7 +675,7 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
}

Rados rados(ictx->md_ctx);
for ( auto &info : image_info){
for (auto &info : image_info) {
IoCtx ioctx;
r = rados.ioctx_create2(info.first.first, ioctx);
if (r < 0) {
Expand All @@ -675,17 +685,38 @@ int validate_pool(IoCtx &io_ctx, CephContext *cct) {
}

for (auto &id_it : info.second) {
string name;
r = cls_client::dir_get_name(&ioctx, RBD_DIRECTORY, id_it, &name);
if (r < 0) {
lderr(cct) << "Error looking up name for image id " << id_it
<< " in pool " << info.first.second << dendl;
return r;
}
names.insert(make_pair(info.first.second, name));
string name;
bool trash = false;
r = cls_client::dir_get_name(&ioctx, RBD_DIRECTORY, id_it, &name);
if (r == -ENOENT) {
cls::rbd::TrashImageSpec trash_spec;
r = cls_client::trash_get(&ioctx, id_it, &trash_spec);
if (r < 0) {
if (r != -EOPNOTSUPP && r != -ENOENT) {
lderr(cct) << "Error looking up name for image id " << id_it
<< " in rbd trash" << dendl;
return r;
}
return -ENOENT;
}
name = trash_spec.name;
trash = true;
} else if (r < 0 && r != -ENOENT) {
lderr(cct) << "Error looking up name for image id " << id_it
<< " in pool " << info.first.second << dendl;
return r;
}
names->push_back(
child_info_t {
info.first.second,
name,
id_it,
trash
});
}
}

std::sort(names->begin(), names->end(), compare_by_name);

return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion src/librbd/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ namespace librbd {

int list(librados::IoCtx& io_ctx, std::vector<std::string>& names);
int list_children(ImageCtx *ictx,
std::set<std::pair<std::string, std::string> > & names);
std::vector<child_info_t> *names);
int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size,
int *order);
int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size,
Expand Down
103 changes: 98 additions & 5 deletions src/librbd/librbd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1261,11 +1261,30 @@ namespace librbd {
{
ImageCtx *ictx = (ImageCtx *)ctx;
tracepoint(librbd, list_children_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only);
int r = librbd::list_children(ictx, *children);
vector<librbd::child_info_t> children2;
int r = librbd::list_children(ictx, &children2);
if (r >= 0) {
for (set<pair<string, string> >::const_iterator it = children->begin();
it != children->end(); ++it) {
tracepoint(librbd, list_children_entry, it->first.c_str(), it->second.c_str());
for (std::vector<librbd::child_info_t>::iterator it = children2.begin();
it != children2.end(); ++it) {
if (!it->trash) {
children->insert(make_pair(it->pool_name, it->image_name));
tracepoint(librbd, list_children_entry, it->pool_name.c_str(), it->image_name.c_str());
}
}
}
tracepoint(librbd, list_children_exit, r);
return r;
}

int Image::list_children2(vector<librbd::child_info_t> *children)
{
ImageCtx *ictx = (ImageCtx *)ctx;
tracepoint(librbd, list_children_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only);
int r = librbd::list_children(ictx, children);
if (r >= 0) {
for (std::vector<librbd::child_info_t>::iterator it = children->begin();
it != children->end(); ++it) {
tracepoint(librbd, list_children_entry, it->pool_name.c_str(), it->image_name.c_str());
}
}
tracepoint(librbd, list_children_exit, r);
Expand Down Expand Up @@ -3295,13 +3314,21 @@ extern "C" ssize_t rbd_list_children(rbd_image_t image, char *pools,
librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
tracepoint(librbd, list_children_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only);
set<pair<string, string> > image_set;
vector<librbd::child_info_t> children;

int r = librbd::list_children(ictx, image_set);
int r = librbd::list_children(ictx, &children);
if (r < 0) {
tracepoint(librbd, list_children_exit, r);
return r;
}

for (std::vector<librbd::child_info_t>::iterator it = children.begin();
it != children.end(); ++it) {
if (!it->trash) {
image_set.insert(make_pair(it->pool_name, it->image_name));
}
}

size_t pools_total = 0;
size_t images_total = 0;
for (set<pair<string, string> >::const_iterator it = image_set.begin();
Expand Down Expand Up @@ -3340,6 +3367,72 @@ extern "C" ssize_t rbd_list_children(rbd_image_t image, char *pools,
return ret;
}

extern "C" int rbd_list_children2(rbd_image_t image,
rbd_child_info_t *children,
int *max_children)
{
vector<librbd::child_info_t> cpp_children;
librbd::ImageCtx *ictx = (librbd::ImageCtx *)image;
tracepoint(librbd, list_children_enter, ictx, ictx->name.c_str(), ictx->snap_name.c_str(), ictx->read_only);

if (!max_children) {
tracepoint(librbd, list_children_exit, -EINVAL);
return -EINVAL;
}

int r = librbd::list_children(ictx, &cpp_children);
if (r == -ENOENT) {
tracepoint(librbd, list_children_exit, *max_children);
r = 0;
}
if (r < 0) {
tracepoint(librbd, list_children_exit, *max_children);
return r;
}
if (*max_children < (int)cpp_children.size() + 1) {
*max_children = (int)cpp_children.size() + 1;
tracepoint(librbd, list_children_exit, *max_children);
return -ERANGE;
}

int i;
for (i = 0; i < (int)cpp_children.size(); i++) {
children[i].pool_name = strdup(cpp_children[i].pool_name.c_str());
children[i].image_name = strdup(cpp_children[i].image_name.c_str());
children[i].image_id = strdup(cpp_children[i].image_id.c_str());
children[i].trash = cpp_children[i].trash;
if (!children[i].pool_name || !children[i].image_name ||
!children[i].image_id) {
rbd_list_children_cleanup(&children[i], i);
tracepoint(librbd, list_children_exit, *max_children);
return -ENOMEM;
}
tracepoint(librbd, list_children_entry, children[i].pool_name, children[i].image_name);
}
children[i].pool_name = NULL;
children[i].image_name = NULL;
children[i].image_id = NULL;

r = (int)cpp_children.size();
tracepoint(librbd, list_children_exit, *max_children);
return r;
}

extern "C" void rbd_list_child_cleanup(rbd_child_info_t *child)
{
free((void *)child->pool_name);
free((void *)child->image_name);
free((void *)child->image_id);
}

extern "C" void rbd_list_children_cleanup(rbd_child_info_t *children,
size_t num_children)
{
for (size_t i=0; i < num_children; i++) {
rbd_list_child_cleanup(&children[i]);
}
}

extern "C" ssize_t rbd_list_lockers(rbd_image_t image, int *exclusive,
char *tag, size_t *tag_len,
char *clients, size_t *clients_len,
Expand Down
68 changes: 68 additions & 0 deletions src/pybind/rbd/rbd.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,12 @@ cdef extern from "rbd/librbd.h" nogil:
uint64_t size
char *name

ctypedef struct rbd_child_info_t:
char *pool_name
char *image_name
char *image_id
bint trash

ctypedef enum rbd_mirror_mode_t:
_RBD_MIRROR_MODE_DISABLED "RBD_MIRROR_MODE_DISABLED"
_RBD_MIRROR_MODE_IMAGE "RBD_MIRROR_MODE_IMAGE"
Expand Down Expand Up @@ -269,6 +275,10 @@ cdef extern from "rbd/librbd.h" nogil:
void *cbdata)
ssize_t rbd_list_children(rbd_image_t image, char *pools, size_t *pools_len,
char *images, size_t *images_len)
int rbd_list_children2(rbd_image_t image, rbd_child_info_t *children,
int *max_children)
void rbd_list_children_cleanup(rbd_child_info_t *children,
size_t num_children)
ssize_t rbd_list_lockers(rbd_image_t image, int *exclusive,
char *tag, size_t *tag_len,
char *clients, size_t *clients_len,
Expand Down Expand Up @@ -2232,6 +2242,14 @@ written." % (self.name, ret, length))
free(c_pools)
free(c_images)

def list_children2(self):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be nicer to switch to an iterator style and return a dict for each child.

"""
Iterate over the children of a snapshot.

:returns: :class:`ChildIterator`
"""
return ChildIterator(self)

def list_lockers(self):
"""
List clients that have locked the image and information
Expand Down Expand Up @@ -2958,3 +2976,53 @@ cdef class TrashIterator(object):
if self.entries:
free(self.entries)

cdef class ChildIterator(object):
"""
Iterator over child info for a snapshot.

Yields a dictionary containing information about a child.

Keys are:

* ``pool`` (str) - name of the pool

* ``image`` (str) - name of the child

* ``id`` (str) - id of the child

* ``trash`` (bool) - True if child is in trash bin
"""

cdef rbd_child_info_t *children
cdef int num_children
cdef object image

def __init__(self, Image image):
self.image = image
self.children = NULL
self.num_children = 10
while True:
self.children = <rbd_child_info_t*>realloc_chk(self.children,
self.num_children *
sizeof(rbd_child_info_t))
with nogil:
ret = rbd_list_children2(image.image, self.children, &self.num_children)
if ret >= 0:
self.num_children = ret
break
elif ret != -errno.ERANGE:
raise make_ex(ret, 'error listing children.')

def __iter__(self):
for i in range(self.num_children):
yield {
'pool' : decode_cstr(self.children[i].pool_name),
'image' : decode_cstr(self.children[i].image_name),
'id' : decode_cstr(self.children[i].image_id),
'trash' : self.children[i].trash
}

def __dealloc__(self):
if self.children:
rbd_list_children_cleanup(self.children, self.num_children)
free(self.children)
3 changes: 2 additions & 1 deletion src/test/cli/rbd/help.t
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ Skip test on FreeBSD as it generates different output there.
--io-type arg IO type (read , write, or readwrite(rw))

rbd help children
usage: rbd children [--pool <pool>] [--image <image>] [--snap <snap>]
usage: rbd children [--pool <pool>] [--image <image>] [--snap <snap>] [--all]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These (related) changes seem to be mixing and matching between commits. The title of this commit is to fix a bug re: listing children for trashed images so it should be limited to handling that fix. The next commit can add the new API methods and expand the rbd CLI to list trashed images.

[--format <format>] [--pretty-format]
<snap-spec>

Expand All @@ -158,6 +158,7 @@ Skip test on FreeBSD as it generates different output there.
-p [ --pool ] arg pool name
--image arg image name
--snap arg snapshot name
-a [ --all ] list all children of snapshot (include trash)
--format arg output format (plain, json, or xml) [default: plain]
--pretty-format pretty formatting (json and xml)

Expand Down
Loading