From 99287a1e552387f8eec505bffdffae7f4674cc8b Mon Sep 17 00:00:00 2001
From: Mykola Golub <mgolub@suse.com>
Date: Mon, 23 Mar 2020 10:28:52 +0000
Subject: [PATCH] pybind/rbd: ensure image is open before permitting operations

Fixes: https://tracker.ceph.com/issues/44610
Signed-off-by: Mykola Golub <mgolub@suse.com>
---
 src/pybind/rbd/rbd.pyx      | 148 +++++++++++++++++++++++++++++++-----
 src/test/pybind/test_rbd.py |   4 +
 2 files changed, 134 insertions(+), 18 deletions(-)

diff --git a/src/pybind/rbd/rbd.pyx b/src/pybind/rbd/rbd.pyx
index b71458b5cb936..3c6884167d21c 100644
--- a/src/pybind/rbd/rbd.pyx
+++ b/src/pybind/rbd/rbd.pyx
@@ -3292,6 +3292,13 @@ cdef class Group(object):
         if ret != 0:
             raise make_ex(ret, 'error rolling back group to snapshot', group_errno_to_exception)
 
+def requires_not_closed(f):
+    def wrapper(self, *args, **kwargs):
+        self.require_not_closed()
+        return f(self, *args, **kwargs)
+
+    return wrapper
+
 cdef class Image(object):
     """
     This class represents an RBD image. It is used to perform I/O on
@@ -3406,6 +3413,15 @@ cdef class Image(object):
         completion_obj.rbd_comp = completion
         return completion_obj
 
+    def require_not_closed(self):
+        """
+        Checks if the Image is not closed
+
+        :raises: :class:`InvalidArgument`
+        """
+        if self.closed:
+            raise InvalidArgument("image is closed")
+
     def close(self):
         """
         Release the resources used by this image object.
@@ -3426,6 +3442,7 @@ cdef class Image(object):
     def __repr__(self):
         return "rbd.Image(ioctx, %r)" % self.name
 
+    @requires_not_closed
     def resize(self, size, allow_shrink=True):
         """
         Change the size of the image, allow shrink.
@@ -3449,6 +3466,7 @@ cdef class Image(object):
         if ret < 0:
             raise make_ex(ret, 'error resizing image %s' % self.name)
 
+    @requires_not_closed
     def stat(self):
         """
         Get information about the image. Currently parent pool and
@@ -3490,6 +3508,7 @@ cdef class Image(object):
             'parent_name'       : info.parent_name
             }
 
+    @requires_not_closed
     def get_name(self):
         """
         Get the RBD image name
@@ -3512,6 +3531,7 @@ cdef class Image(object):
         finally:
             free(image_name)
 
+    @requires_not_closed
     def id(self):
         """
         Get the RBD v2 internal image id
@@ -3536,6 +3556,7 @@ cdef class Image(object):
         finally:
             free(image_id)
 
+    @requires_not_closed
     def block_name_prefix(self):
         """
         Get the RBD block name prefix
@@ -3560,6 +3581,7 @@ cdef class Image(object):
         finally:
             free(prefix)
 
+    @requires_not_closed
     def data_pool_id(self):
         """
         Get the pool id of the pool where the data of this RBD image is stored.
@@ -3568,6 +3590,7 @@ cdef class Image(object):
         """
         return rbd_get_data_pool_id(self.image)
 
+    @requires_not_closed
     def get_parent_image_spec(self):
         """
         Get spec of the cloned image's parent
@@ -3597,6 +3620,7 @@ cdef class Image(object):
         rbd_snap_spec_cleanup(&snap_spec)
         return result
 
+    @requires_not_closed
     def parent_info(self):
         """
         Deprecated. Use `get_parent_image_spec` instead.
@@ -3610,6 +3634,7 @@ cdef class Image(object):
         parent = self.get_parent_image_spec()
         return (parent['pool_name'], parent['image_name'], parent['snap_name'])
 
+    @requires_not_closed
     def parent_id(self):
         """
         Get image id of a cloned image's parent (if any)
@@ -3631,6 +3656,7 @@ cdef class Image(object):
         rbd_snap_spec_cleanup(&snap_spec)
         return result
 
+    @requires_not_closed
     def old_format(self):
         """
         Find out whether the image uses the old RBD format.
@@ -3644,6 +3670,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting old_format for image %s' % (self.name))
         return old != 0
 
+    @requires_not_closed
     def size(self):
         """
         Get the size of the image. If open to a snapshot, returns the
@@ -3658,6 +3685,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting size for image %s' % (self.name))
         return image_size
 
+    @requires_not_closed
     def features(self):
         """
         Get the features bitmask of the image.
@@ -3671,6 +3699,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting features for image %s' % (self.name))
         return features
 
+    @requires_not_closed
     def update_features(self, features, enabled):
         """
         Update the features bitmask of the image by enabling/disabling
@@ -3692,6 +3721,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error updating features for image %s' %
                                (self.name))
 
+    @requires_not_closed
     def op_features(self):
         """
         Get the op features bitmask of the image.
@@ -3705,6 +3735,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting op features for image %s' % (self.name))
         return op_features
 
+    @requires_not_closed
     def overlap(self):
         """
         Get the number of overlapping bytes between the image and its parent
@@ -3721,6 +3752,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting overlap for image %s' % (self.name))
         return overlap
 
+    @requires_not_closed
     def flags(self):
         """
         Get the flags bitmask of the image.
@@ -3734,6 +3766,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting flags for image %s' % (self.name))
         return flags
 
+    @requires_not_closed
     def group(self):
         """
         Get information about the image's group.
@@ -3757,6 +3790,7 @@ cdef class Image(object):
         rbd_group_info_cleanup(&info, sizeof(info))
         return result
 
+    @requires_not_closed
     def is_exclusive_lock_owner(self):
         """
         Get the status of the image exclusive lock.
@@ -3770,6 +3804,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting lock status for image %s' % (self.name))
         return owner == 1
 
+    @requires_not_closed
     def copy(self, dest_ioctx, dest_name, features=None, order=None,
              stripe_unit=None, stripe_count=None, data_pool=None):
         """
@@ -3826,6 +3861,7 @@ cdef class Image(object):
         if ret < 0:
             raise make_ex(ret, 'error copying image %s to %s' % (self.name, dest_name))
 
+    @requires_not_closed
     def deep_copy(self, dest_ioctx, dest_name, features=None, order=None,
                   stripe_unit=None, stripe_count=None, data_pool=None):
         """
@@ -3882,6 +3918,7 @@ cdef class Image(object):
         if ret < 0:
             raise make_ex(ret, 'error copying image %s to %s' % (self.name, dest_name))
 
+    @requires_not_closed
     def list_snaps(self):
         """
         Iterate over the snapshots of an image.
@@ -3890,6 +3927,7 @@ cdef class Image(object):
         """
         return SnapIterator(self)
 
+    @requires_not_closed
     def create_snap(self, name):
         """
         Create a snapshot of the image.
@@ -3905,6 +3943,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error creating snapshot %s from %s' % (name, self.name))
 
+    @requires_not_closed
     def rename_snap(self, srcname, dstname):
         """
         rename a snapshot of the image.
@@ -3925,6 +3964,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error renaming snapshot of %s from %s to %s' % (self.name, srcname, dstname))
 
+    @requires_not_closed
     def remove_snap(self, name):
         """
         Delete a snapshot of the image.
@@ -3940,6 +3980,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error removing snapshot %s from %s' % (name, self.name))
 
+    @requires_not_closed
     def remove_snap2(self, name, flags):
         """
         Delete a snapshot of the image.
@@ -3949,6 +3990,8 @@ cdef class Image(object):
         :type name: str
         :raises: :class:`IOError`, :class:`ImageBusy`
         """
+        self.require_not_closed()
+
         name = cstr(name, 'name')
         cdef:
             char *_name = name
@@ -3959,6 +4002,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error removing snapshot %s from %s with flags %lx' % (name, self.name, flags))
 
+    @requires_not_closed
     def remove_snap_by_id(self, snap_id):
         """
         Delete a snapshot of the image by its id.
@@ -3974,6 +4018,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error removing snapshot %s from %s' % (snap_id, self.name))
 
+    @requires_not_closed
     def rollback_to_snap(self, name):
         """
         Revert the image to its contents at a snapshot. This is a
@@ -3991,6 +4036,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error rolling back image %s to snapshot %s' % (self.name, name))
 
+    @requires_not_closed
     def protect_snap(self, name):
         """
         Mark a snapshot as protected. This means it can't be deleted
@@ -4007,6 +4053,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error protecting snapshot %s@%s' % (self.name, name))
 
+    @requires_not_closed
     def unprotect_snap(self, name):
         """
         Mark a snapshot unprotected. This allows it to be deleted if
@@ -4023,6 +4070,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error unprotecting snapshot %s@%s' % (self.name, name))
 
+    @requires_not_closed
     def is_protected_snap(self, name):
         """
         Find out whether a snapshot is protected from deletion.
@@ -4042,6 +4090,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error checking if snapshot %s@%s is protected' % (self.name, name))
         return is_protected == 1
 
+    @requires_not_closed
     def snap_exists(self, name):
         """
         Find out whether a snapshot is exists.
@@ -4060,13 +4109,13 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting snapshot exists for %s' % self.name)
         return bool(_exists != 0)
 
+    @requires_not_closed
     def get_snap_limit(self):
         """
         Get the snapshot limit for an image.
 
         :returns: int - the snapshot limit for an image
         """
-
         cdef:
             uint64_t limit
         with nogil:
@@ -4075,13 +4124,13 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting snapshot limit for %s' % self.name)
         return limit
 
+    @requires_not_closed
     def set_snap_limit(self, limit):
         """
         Set the snapshot limit for an image.
 
         :param limit: the new limit to set
         """
-
         cdef:
             uint64_t _limit = limit
         with nogil:
@@ -4090,6 +4139,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error setting snapshot limit for %s' % self.name)
         return ret
 
+    @requires_not_closed
     def get_snap_timestamp(self, snap_id):
         """
         Get the snapshot timestamp for an image.
@@ -4105,6 +4155,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error getting snapshot timestamp for image: %s, snap_id: %d' % (self.name, snap_id))
         return datetime.utcfromtimestamp(timestamp.tv_sec)
 
+    @requires_not_closed
     def remove_snap_limit(self):
         """
         Remove the snapshot limit for an image, essentially setting
@@ -4116,6 +4167,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error removing snapshot limit for %s' % self.name)
         return ret
 
+    @requires_not_closed
     def set_snap(self, name):
         """
         Set the snapshot to read from. Writes will raise ReadOnlyImage
@@ -4132,6 +4184,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error setting image %s to snapshot %s' % (self.name, name))
 
+    @requires_not_closed
     def set_snap_by_id(self, snap_id):
         """
         Set the snapshot to read from. Writes will raise ReadOnlyImage
@@ -4149,6 +4202,7 @@ cdef class Image(object):
         if ret != 0:
             raise make_ex(ret, 'error setting image %s to snapshot %d' % (self.name, snap_id))
 
+    @requires_not_closed
     def snap_get_name(self, snap_id):
         """
         Get snapshot name by id.
@@ -4175,6 +4229,7 @@ cdef class Image(object):
         finally:
             free(image_name)
 
+    @requires_not_closed
     def snap_get_id(self, snap_name):
         """
         Get snapshot id by name.
@@ -4184,7 +4239,6 @@ cdef class Image(object):
         :returns: int - snapshot id
         :raises: :class:`ImageNotFound`
         """
-
         snap_name = cstr(snap_name, 'snap_name')
         cdef:
             const char *_snap_name = snap_name
@@ -4195,6 +4249,7 @@ cdef class Image(object):
             raise make_ex(ret, 'error snap_get_id.')
         return snap_id
 
+    @requires_not_closed
     def read(self, offset, length, fadvise_flags=0):
         """
         Read data from the image. Raises :class:`InvalidArgument` if
@@ -4240,6 +4295,7 @@ cdef class Image(object):
             # itself and set ret_s to NULL, hence XDECREF).
             ref.Py_XDECREF(ret_s)
 
+    @requires_not_closed
     def diff_iterate(self, offset, length, from_snapshot, iterate_cb,
                      include_parent = True, whole_object = False):
         """
@@ -4298,6 +4354,7 @@ cdef class Image(object):
             msg = 'error generating diff from snapshot %s' % from_snapshot
             raise make_ex(ret, msg)
 
+    @requires_not_closed
     def write(self, data, offset, fadvise_flags=0):
         """
         Write data to the image. Raises :class:`InvalidArgument` if
@@ -4333,6 +4390,7 @@ cdef class Image(object):
 returned %d, but %d was the maximum number of bytes it could have \
 written." % (self.name, ret, length))
 
+    @requires_not_closed
     def discard(self, offset, length):
         """
         Trim the range from the image. It will be logically filled
@@ -4345,6 +4403,7 @@ written." % (self.name, ret, length))
             msg = 'error discarding region %d~%d' % (offset, length)
             raise make_ex(ret, msg)
 
+    @requires_not_closed
     def flush(self):
         """
         Block until all writes are fully flushed if caching is enabled.
@@ -4354,6 +4413,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error flushing image')
 
+    @requires_not_closed
     def invalidate_cache(self):
         """
         Drop any cached data for the image.
@@ -4363,6 +4423,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error invalidating cache')
 
+    @requires_not_closed
     def stripe_unit(self):
         """
         Return the stripe unit used for the image.
@@ -4374,6 +4435,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting stripe unit for image %s' % (self.name))
         return stripe_unit
 
+    @requires_not_closed
     def stripe_count(self):
         """
         Return the stripe count used for the image.
@@ -4385,6 +4447,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting stripe count for image %s' % (self.name))
         return stripe_count
 
+    @requires_not_closed
     def create_timestamp(self):
         """
         Return the create timestamp for the image.
@@ -4397,6 +4460,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting create timestamp for image: %s' % (self.name))
         return datetime.utcfromtimestamp(timestamp.tv_sec)
 
+    @requires_not_closed
     def access_timestamp(self):
         """
         Return the access timestamp for the image.
@@ -4409,6 +4473,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting access timestamp for image: %s' % (self.name))
         return datetime.fromtimestamp(timestamp.tv_sec)
 
+    @requires_not_closed
     def modify_timestamp(self):
         """
         Return the modify timestamp for the image.
@@ -4421,6 +4486,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting modify timestamp for image: %s' % (self.name))
         return datetime.fromtimestamp(timestamp.tv_sec)
 
+    @requires_not_closed
     def flatten(self, on_progress=None):
         """
         Flatten clone image (copy all blocks from parent to child)
@@ -4438,6 +4504,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, "error flattening %s" % self.name)
 
+    @requires_not_closed
     def sparsify(self, sparse_size):
         """
         Reclaim space for zeroed image extents
@@ -4449,6 +4516,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, "error sparsifying %s" % self.name)
 
+    @requires_not_closed
     def rebuild_object_map(self):
         """
         Rebuild the object map for the image HEAD or currently set snapshot
@@ -4459,6 +4527,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, "error rebuilding object map %s" % self.name)
 
+    @requires_not_closed
     def list_children(self):
         """
         List children of the currently set snapshot (set via set_snap()).
@@ -4488,6 +4557,7 @@ written." % (self.name, ret, length))
                 rbd_linked_image_spec_list_cleanup(children, num_children)
                 free(children)
 
+    @requires_not_closed
     def list_children2(self):
         """
         Iterate over the children of the image or its snapshot.
@@ -4496,6 +4566,7 @@ written." % (self.name, ret, length))
         """
         return ChildIterator(self)
 
+    @requires_not_closed
     def list_descendants(self):
         """
         Iterate over the descendants of the image.
@@ -4504,6 +4575,7 @@ written." % (self.name, ret, length))
         """
         return ChildIterator(self, True)
 
+    @requires_not_closed
     def list_lockers(self):
         """
         List clients that have locked the image and information
@@ -4559,6 +4631,7 @@ written." % (self.name, ret, length))
             free(c_addrs)
             free(c_tag)
 
+    @requires_not_closed
     def lock_acquire(self, lock_mode):
         """
         Acquire a managed lock on the image.
@@ -4574,6 +4647,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error acquiring lock on image')
 
+    @requires_not_closed
     def lock_release(self):
         """
         Release a managed lock on the image that was previously acquired.
@@ -4583,6 +4657,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error releasing lock on image')
 
+    @requires_not_closed
     def lock_get_owners(self):
         """
         Iterate over the lock owners of an image.
@@ -4591,6 +4666,7 @@ written." % (self.name, ret, length))
         """
         return LockOwnerIterator(self)
 
+    @requires_not_closed
     def lock_break(self, lock_mode, lock_owner):
         """
         Break the image lock held by a another client.
@@ -4607,6 +4683,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error breaking lock on image')
 
+    @requires_not_closed
     def lock_exclusive(self, cookie):
         """
         Take an exclusive lock on the image.
@@ -4621,6 +4698,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error acquiring exclusive lock on image')
 
+    @requires_not_closed
     def lock_shared(self, cookie, tag):
         """
         Take a shared lock on the image. The tag must match
@@ -4639,6 +4717,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error acquiring shared lock on image')
 
+    @requires_not_closed
     def unlock(self, cookie):
         """
         Release a lock on the image that was locked by this rados client.
@@ -4650,6 +4729,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error unlocking image')
 
+    @requires_not_closed
     def break_lock(self, client, cookie):
         """
         Release a lock held by another rados client.
@@ -4664,6 +4744,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error unlocking image')
 
+    @requires_not_closed
     def mirror_image_enable(self, mode=RBD_MIRROR_IMAGE_MODE_JOURNAL):
         """
         Enable mirroring for the image.
@@ -4674,6 +4755,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error enabling mirroring for image %s' % self.name)
 
+    @requires_not_closed
     def mirror_image_disable(self, force):
         """
         Disable mirroring for the image.
@@ -4687,6 +4769,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error disabling mirroring for image %s' % self.name)
 
+    @requires_not_closed
     def mirror_image_promote(self, force):
         """
         Promote the image to primary for mirroring.
@@ -4700,6 +4783,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error promoting image %s to primary' % self.name)
 
+    @requires_not_closed
     def mirror_image_demote(self):
         """
         Demote the image to secondary for mirroring.
@@ -4709,6 +4793,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error demoting image %s to secondary' % self.name)
 
+    @requires_not_closed
     def mirror_image_resync(self):
         """
         Flag the image to resync.
@@ -4718,6 +4803,7 @@ written." % (self.name, ret, length))
         if ret < 0:
             raise make_ex(ret, 'error to resync image %s' % self.name)
 
+    @requires_not_closed
     def mirror_image_create_snapshot(self):
         """
         Create mirror snapshot.
@@ -4735,6 +4821,7 @@ written." % (self.name, ret, length))
                           self.name)
         return snap_id
 
+    @requires_not_closed
     def mirror_image_get_info(self):
         """
         Get mirror info for the image.
@@ -4760,6 +4847,7 @@ written." % (self.name, ret, length))
         rbd_mirror_image_get_info_cleanup(&c_info)
         return info
 
+    @requires_not_closed
     def mirror_image_get_mode(self):
         """
         Get mirror mode for the image.
@@ -4773,6 +4861,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error getting mirror mode for image %s' % self.name)
         return int(c_mode)
 
+    @requires_not_closed
     def mirror_image_get_status(self):
         """
         Get mirror status for the image.
@@ -4847,6 +4936,7 @@ written." % (self.name, ret, length))
             rbd_mirror_image_global_status_cleanup(&c_status)
         return status
 
+    @requires_not_closed
     def mirror_image_get_instance_id(self):
         """
         Get mirror instance id for the image.
@@ -4871,6 +4961,7 @@ written." % (self.name, ret, length))
         finally:
             free(instance_id)
 
+    @requires_not_closed
     def aio_read(self, offset, length, oncomplete, fadvise_flags=0):
         """
         Asynchronously read data from the image
@@ -4894,7 +4985,6 @@ written." % (self.name, ret, length))
         :returns: :class:`Completion` - the completion object
         :raises: :class:`InvalidArgument`, :class:`IOError`
         """
-
         cdef:
             char *ret_buf
             uint64_t _offset = offset
@@ -4926,6 +5016,7 @@ written." % (self.name, ret, length))
 
         return completion
 
+    @requires_not_closed
     def aio_write(self, data, offset, oncomplete, fadvise_flags=0):
         """
         Asynchronously write data to the image
@@ -4948,7 +5039,6 @@ written." % (self.name, ret, length))
         :returns: :class:`Completion` - the completion object
         :raises: :class:`InvalidArgument`, :class:`IOError`
         """
-
         cdef:
             uint64_t _offset = offset
             char *_data = data
@@ -4971,12 +5061,12 @@ written." % (self.name, ret, length))
 
         return completion
 
+    @requires_not_closed
     def aio_discard(self, offset, length, oncomplete):
         """
         Asynchronously trim the range from the image. It will be logically
         filled with zeroes.
         """
-
         cdef:
             uint64_t _offset = offset
             size_t _length = length
@@ -4997,12 +5087,12 @@ written." % (self.name, ret, length))
 
         return completion
 
+    @requires_not_closed
     def aio_flush(self, oncomplete):
         """
         Asynchronously wait until all writes are fully flushed if caching is
         enabled.
         """
-
         cdef Completion completion = self.__get_completion(oncomplete)
         try:
             completion.__persist()
@@ -5016,6 +5106,7 @@ written." % (self.name, ret, length))
 
         return completion
 
+    @requires_not_closed
     def metadata_get(self, key):
         """
         Get image metadata for the given key.
@@ -5046,6 +5137,7 @@ written." % (self.name, ret, length))
         finally:
             free(value)
 
+    @requires_not_closed
     def metadata_set(self, key, value):
         """
         Set image metadata for the given key.
@@ -5067,7 +5159,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error setting metadata %s for image %s' %
                           (key, self.name))
 
-
+    @requires_not_closed
     def metadata_remove(self, key):
         """
         Remove image metadata for the given key.
@@ -5087,6 +5179,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error removing metadata %s for image %s' %
                           (key, self.name))
 
+    @requires_not_closed
     def metadata_list(self):
         """
         List image metadata.
@@ -5095,6 +5188,7 @@ written." % (self.name, ret, length))
         """
         return MetadataIterator(self)
 
+    @requires_not_closed
     def watchers_list(self):
         """
         List image watchers.
@@ -5103,6 +5197,7 @@ written." % (self.name, ret, length))
         """
         return WatcherIterator(self)
 
+    @requires_not_closed
     def config_list(self):
         """
         List image-level config overrides.
@@ -5111,7 +5206,7 @@ written." % (self.name, ret, length))
         """
         return ConfigImageIterator(self)
 
-
+    @requires_not_closed
     def config_set(self, key, value):
         """
         Set an image-level configuration override.
@@ -5134,7 +5229,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error setting config %s for image %s' %
                           (key, self.name))
 
-
+    @requires_not_closed
     def config_get(self, key):
         """
         Get an image-level configuration override.
@@ -5166,7 +5261,7 @@ written." % (self.name, ret, length))
         finally:
             free(value)
 
-
+    @requires_not_closed
     def config_remove(self, key):
         """
         Remove an image-level configuration override.
@@ -5187,7 +5282,7 @@ written." % (self.name, ret, length))
             raise make_ex(ret, 'error removing config %s for image %s' %
                           (key, self.name))
 
-
+    @requires_not_closed
     def snap_get_namespace_type(self, snap_id):
         """
         Get the snapshot namespace type.
@@ -5204,6 +5299,7 @@ written." % (self.name, ret, length))
 
         return namespace_type
 
+    @requires_not_closed
     def snap_get_group_namespace(self, snap_id):
         """
         get the group namespace details.
@@ -5236,6 +5332,7 @@ written." % (self.name, ret, length))
                                          sizeof(rbd_snap_group_namespace_t))
         return info
 
+    @requires_not_closed
     def snap_get_trash_namespace(self, snap_id):
         """
         get the trash namespace details.
@@ -5266,6 +5363,7 @@ written." % (self.name, ret, length))
         finally:
             free(_name)
 
+    @requires_not_closed
     def snap_get_mirror_namespace(self, snap_id):
         """
         get the mirror namespace details.
@@ -5380,6 +5478,8 @@ cdef class LockOwnerIterator(object):
         object image
 
     def __init__(self, Image image):
+        image.require_not_closed()
+
         self.image = image
         self.lock_owners = NULL
         self.num_lock_owners = 8
@@ -5419,15 +5519,17 @@ cdef class MetadataIterator(object):
     """
 
     cdef:
-        object image_name
-        rbd_image_t image
+        cdef object image
+        rbd_image_t c_image
         char *last_read
         uint64_t max_read
         object next_chunk
 
     def __init__(self, Image image):
-        self.image_name = image.name
-        self.image = image.image
+        image.require_not_closed()
+
+        self.image = image
+        self.c_image = image.image
         self.last_read = strdup("")
         self.max_read = 32
         self.get_next_chunk()
@@ -5445,6 +5547,8 @@ cdef class MetadataIterator(object):
             free(self.last_read)
 
     def get_next_chunk(self):
+        self.image.require_not_closed()
+
         cdef:
             char *c_keys = NULL
             size_t keys_size = 4096
@@ -5455,14 +5559,14 @@ cdef class MetadataIterator(object):
                 c_keys = <char *>realloc_chk(c_keys, keys_size)
                 c_vals = <char *>realloc_chk(c_vals, vals_size)
                 with nogil:
-                    ret = rbd_metadata_list(self.image, self.last_read,
+                    ret = rbd_metadata_list(self.c_image, self.last_read,
                                             self.max_read, c_keys, &keys_size,
                                             c_vals, &vals_size)
                 if ret >= 0:
                     break
                 elif ret != -errno.ERANGE:
                     raise make_ex(ret, 'error listing metadata for image %s' %
-                                  self.image_name)
+                                  self.image.name)
             keys = [decode_cstr(key) for key in
                         c_keys[:keys_size].split(b'\0') if key]
             vals = [decode_cstr(val) for val in
@@ -5504,6 +5608,8 @@ cdef class SnapIterator(object):
     cdef object image
 
     def __init__(self, Image image):
+        image.require_not_closed()
+
         self.image = image
         self.snaps = NULL
         self.num_snaps = 10
@@ -5635,6 +5741,8 @@ cdef class ChildIterator(object):
     cdef object image
 
     def __init__(self, Image image, descendants=False):
+        image.require_not_closed()
+
         self.image = image
         self.children = NULL
         self.num_children = 10
@@ -5689,6 +5797,8 @@ cdef class WatcherIterator(object):
     cdef object image
 
     def __init__(self, Image image):
+        image.require_not_closed()
+
         self.image = image
         self.watchers = NULL
         self.num_watchers = 10
@@ -5736,6 +5846,8 @@ cdef class ConfigImageIterator(object):
         int num_options
 
     def __init__(self, Image image):
+        image.require_not_closed()
+
         self.options = NULL
         self.num_options = 32
         while True:
diff --git a/src/test/pybind/test_rbd.py b/src/test/pybind/test_rbd.py
index 1bd84e81f84ad..72d43e44aea04 100644
--- a/src/test/pybind/test_rbd.py
+++ b/src/test/pybind/test_rbd.py
@@ -588,6 +588,10 @@ def test_flags(self):
     def test_image_auto_close(self):
         image = Image(ioctx, image_name)
 
+    def test_use_after_close(self):
+        self.image.close()
+        assert_raises(InvalidArgument, self.image.stat)
+
     def test_write(self):
         data = rand_data(256)
         self.image.write(data, 0)