pybind: Add Python 3 support for rados and rbd modules

Python 3 explicitly distinguishes between strings (which can contain any Unicode
character) and 8-bit byte strings. This means that we must explicitly encode and
decode strings that use C-style char* string APIs.

Functions that take a true-strings now have their values encoded to UTF-8 before
being passed to the C API, while functions that accept binary data only accept
the bytes type. To help with this the `cstr` helper function has been introduced
as a way of easily a string into a C-string compatible form.

There is also a number of general Python 3 compatibility fixes:

    - dict.iteritems() is replaced with just dict.items()
    - xrange() is replaced with just range
    - Iterators now use the __next__ magic method
    - zip() and map() now return iterators, rather than lists
    - print() is now a function
    - contextlib.nexted() is replaced with multiple manager with-statement (from Python 2.7)

This also required updating of the unit-tests for these modules, mostly due to
explicitly distinguishing between strings and byte strings.

Signed-off-by: David Coles <dcoles@gaikai.com>

2 files changed, 168 insertions, 122 deletions

diff --git a/src/pybind/rados.py b/src/pybind/rados.py
index 804a1692020..30be4f93789 100644
--- a/src/pybind/rados.py
+++ b/src/pybind/rados.py
@@ -12,6 +12,7 @@ import errno
 import threading
 import time
 
+from collections import Iterator
 from datetime import datetime
 from functools import wraps
 from itertools import chain
@@ -261,6 +262,19 @@ def requires(*types):
     return wrapper
 
 
+def cstr(val, encoding="utf-8"):
+    """
+    Create a C-style string from a Python string
+
+    :param str val: Python string
+    :rtype: c_char_p
+    """
+    if val is None:
+        return c_char_p(None)
+
+    return c_char_p(val.encode(encoding))
+
+
 class Rados(object):
     """librados python wrapper"""
     def require_state(self, *args):
@@ -297,15 +311,15 @@ Rados object in state %s." % self.state)
         if clustername is None:
             clustername = 'ceph'
         ret = run_in_thread(self.librados.rados_create2,
-                            (byref(self.cluster), c_char_p(clustername),
-                             c_char_p(name), c_uint64(flags)))
+                            (byref(self.cluster), cstr(clustername),
+                            cstr(name), c_uint64(flags)))
 
         if ret != 0:
             raise Error("rados_initialize failed with error code: %d" % ret)
         self.state = "configuring"
         # order is important: conf_defaults, then conffile, then conf
         if conf_defaults:
-            for key, value in conf_defaults.iteritems():
+            for key, value in conf_defaults.items():
                 self.conf_set(key, value)
         if conffile is not None:
             # read the default conf file when '' is given
@@ -313,7 +327,7 @@ Rados object in state %s." % self.state)
                 conffile = None
             self.conf_read_file(conffile)
         if conf:
-            for key, value in conf.iteritems():
+            for key, value in conf.items():
                 self.conf_set(key, value)
 
     def shutdown(self):
@@ -358,7 +372,7 @@ Rados object in state %s." % self.state)
         """
         self.require_state("configuring", "connected")
         ret = run_in_thread(self.librados.rados_conf_read_file,
-                            (self.cluster, c_char_p(path)))
+                            (self.cluster, cstr(path)))
         if (ret != 0):
             raise make_ex(ret, "error calling conf_read_file")
 
@@ -372,7 +386,7 @@ Rados object in state %s." % self.state)
             return
         # create instances of arrays of c_char_p's, both len(args) long
         # cretargs will always be a subset of cargs (perhaps identical)
-        cargs = (c_char_p * len(args))(*args)
+        cargs = (c_char_p * len(args))(*map(cstr, args))
         cretargs = (c_char_p * len(args))()
         ret = run_in_thread(self.librados.rados_conf_parse_argv_remainder,
                             (self.cluster, len(args), cargs, cretargs))
@@ -395,7 +409,7 @@ Rados object in state %s." % self.state)
         if not var:
             return
         ret = run_in_thread(self.librados.rados_conf_parse_env,
-                            (self.cluster, c_char_p(var)))
+                            (self.cluster, cstr(var)))
         if (ret != 0):
             raise make_ex(ret, "error calling conf_parse_env")
 
@@ -415,10 +429,10 @@ Rados object in state %s." % self.state)
         while True:
             ret_buf = create_string_buffer(length)
             ret = run_in_thread(self.librados.rados_conf_get,
-                                (self.cluster, c_char_p(option), ret_buf,
-                                 c_size_t(length)))
+                                (self.cluster, cstr(option), ret_buf,
+                                c_size_t(length)))
             if (ret == 0):
-                return ret_buf.value
+                return ret_buf.value.decode("utf-8")
             elif (ret == -errno.ENAMETOOLONG):
                 length = length * 2
             elif (ret == -errno.ENOENT):
@@ -440,7 +454,7 @@ Rados object in state %s." % self.state)
         """
         self.require_state("configuring", "connected")
         ret = run_in_thread(self.librados.rados_conf_set,
-                            (self.cluster, c_char_p(option), c_char_p(val)))
+                            (self.cluster, cstr(option), cstr(val)))
         if (ret != 0):
             raise make_ex(ret, "error calling conf_set")
 
@@ -463,7 +477,7 @@ Rados object in state %s." % self.state)
         outstrlen = c_long()
 
         ret = run_in_thread(self.librados.rados_ping_monitor,
-                            (self.cluster, c_char_p(mon_id),
+                            (self.cluster, cstr(mon_id),
                              outstrp, byref(outstrlen)))
 
         my_outstr = outstrp.contents[:(outstrlen.value)]
@@ -528,7 +542,7 @@ Rados object in state %s." % self.state)
         """
         self.require_state("connected")
         ret = run_in_thread(self.librados.rados_pool_lookup,
-                            (self.cluster, c_char_p(pool_name)))
+                            (self.cluster, cstr(pool_name)))
         if (ret >= 0):
             return True
         elif (ret == -errno.ENOENT):
@@ -549,7 +563,7 @@ Rados object in state %s." % self.state)
         """
         self.require_state("connected")
         ret = run_in_thread(self.librados.rados_pool_lookup,
-                            (self.cluster, c_char_p(pool_name)))
+                            (self.cluster, cstr(pool_name)))
         if (ret >= 0):
             return int(ret)
         elif (ret == -errno.ENOENT):
@@ -606,20 +620,20 @@ Rados object in state %s." % self.state)
         if auid is None:
             if crush_rule is None:
                 ret = run_in_thread(self.librados.rados_pool_create,
-                                    (self.cluster, c_char_p(pool_name)))
+                                    (self.cluster, cstr(pool_name)))
             else:
                 ret = run_in_thread(self.librados.
                                     rados_pool_create_with_crush_rule,
-                                    (self.cluster, c_char_p(pool_name),
+                                    (self.cluster, cstr(pool_name),
                                      c_ubyte(crush_rule)))
 
         elif crush_rule is None:
             ret = run_in_thread(self.librados.rados_pool_create_with_auid,
-                                (self.cluster, c_char_p(pool_name),
+                                (self.cluster, cstr(pool_name),
                                  c_uint64(auid)))
         else:
             ret = run_in_thread(self.librados.rados_pool_create_with_all,
-                                (self.cluster, c_char_p(pool_name),
+                                (self.cluster, cstr(pool_name),
                                  c_uint64(auid), c_ubyte(crush_rule)))
         if ret < 0:
             raise make_ex(ret, "error creating pool '%s'" % pool_name)
@@ -654,7 +668,7 @@ Rados object in state %s." % self.state)
         """
         self.require_state("connected")
         ret = run_in_thread(self.librados.rados_pool_delete,
-                            (self.cluster, c_char_p(pool_name)))
+                            (self.cluster, cstr(pool_name)))
         if ret < 0:
             raise make_ex(ret, "error deleting pool '%s'" % pool_name)
 
@@ -674,7 +688,8 @@ Rados object in state %s." % self.state)
                 size = c_size_t(ret)
             else:
                 break
-        return filter(lambda name: name != '', c_names.raw.split('\0'))
+
+        return [name for name in c_names.raw.decode("utf-8").split('\0') if len(name) > 0]
 
     def get_fsid(self):
         """
@@ -709,7 +724,7 @@ Rados object in state %s." % self.state)
         self.require_state("connected")
         ioctx = c_void_p()
         ret = run_in_thread(self.librados.rados_ioctx_create,
-                            (self.cluster, c_char_p(ioctx_name), byref(ioctx)))
+                            (self.cluster, cstr(ioctx_name), byref(ioctx)))
         if ret < 0:
             raise make_ex(ret, "error opening pool '%s'" % ioctx_name)
         return Ioctx(ioctx_name, self.librados, ioctx)
@@ -724,11 +739,11 @@ Rados object in state %s." % self.state)
         outbuflen = c_long()
         outsp = pointer(pointer(c_char()))
         outslen = c_long()
-        cmdarr = (c_char_p * len(cmd))(*cmd)
+        cmdarr = (c_char_p * len(cmd))(*map(cstr, cmd))
 
         if target:
             ret = run_in_thread(self.librados.rados_mon_command_target,
-                                (self.cluster, c_char_p(target), cmdarr,
+                                (self.cluster, cstr(target), cmdarr,
                                  len(cmd), c_char_p(inbuf), len(inbuf),
                                  outbufp, byref(outbuflen), outsp,
                                  byref(outslen)), timeout)
@@ -761,7 +776,7 @@ Rados object in state %s." % self.state)
         outbuflen = c_long()
         outsp = pointer(pointer(c_char()))
         outslen = c_long()
-        cmdarr = (c_char_p * len(cmd))(*cmd)
+        cmdarr = (c_char_p * len(cmd))(*map(cstr, cmd))
         ret = run_in_thread(self.librados.rados_osd_command,
                             (self.cluster, osdid, cmdarr, len(cmd),
                              c_char_p(inbuf), len(inbuf),
@@ -790,9 +805,9 @@ Rados object in state %s." % self.state)
         outbuflen = c_long()
         outsp = pointer(pointer(c_char()))
         outslen = c_long()
-        cmdarr = (c_char_p * len(cmd))(*cmd)
+        cmdarr = (c_char_p * len(cmd))(*map(cstr, cmd))
         ret = run_in_thread(self.librados.rados_pg_command,
-                            (self.cluster, c_char_p(pgid), cmdarr, len(cmd),
+                            (self.cluster, cstr(pgid), cmdarr, len(cmd),
                              c_char_p(inbuf), len(inbuf),
                              outbufp, byref(outbuflen), outsp, byref(outslen)),
                             timeout)
@@ -826,13 +841,13 @@ Rados object in state %s." % self.state)
         """
         self.require_state("connected")
         ret = run_in_thread(self.librados.rados_blacklist_add,
-                            (self.cluster, c_char_p(client_address),
+                            (self.cluster, cstr(client_address),
                              c_uint32(expire_seconds)))
         if ret < 0:
             raise make_ex(ret, "error blacklisting client '%s'" % client_address)
 
 
-class OmapIterator(object):
+class OmapIterator(Iterator):
     """Omap iterator"""
     def __init__(self, ioctx, ctx):
         self.ioctx = ioctx
@@ -842,6 +857,9 @@ class OmapIterator(object):
         return self
 
     def next(self):
+        return self.__next__()
+
+    def __next__(self):
         """
         Get the next key-value pair in the object
         :returns: next rados.OmapItem
@@ -855,7 +873,7 @@ class OmapIterator(object):
             raise make_ex(ret, "error iterating over the omap")
         if key_.value is None:
             raise StopIteration()
-        key = ctypes.string_at(key_)
+        key = key_.value.decode("utf-8")
         val = None
         if val_.value is not None:
             val = ctypes.string_at(val_, len_)
@@ -865,7 +883,7 @@ class OmapIterator(object):
         run_in_thread(self.ioctx.librados.rados_omap_get_end, (self.ctx,))
 
 
-class ObjectIterator(object):
+class ObjectIterator(Iterator):
     """rados.Ioctx Object iterator"""
     def __init__(self, ioctx):
         self.ioctx = ioctx
@@ -880,26 +898,33 @@ class ObjectIterator(object):
         return self
 
     def next(self):
+        return self.__next__()
+
+    def __next__(self):
         """
         Get the next object name and locator in the pool
 
         :raises: StopIteration
         :returns: next rados.Ioctx Object
         """
-        key = c_char_p()
-        locator = c_char_p()
-        nspace = c_char_p()
+        key_ = c_char_p()
+        locator_ = c_char_p()
+        nspace_ = c_char_p()
         ret = run_in_thread(self.ioctx.librados.rados_nobjects_list_next,
-                            (self.ctx, byref(key), byref(locator), byref(nspace)))
+                            (self.ctx, byref(key_), byref(locator_), byref(nspace_)))
         if ret < 0:
             raise StopIteration()
-        return Object(self.ioctx, key.value, locator.value, nspace.value)
+
+        key = None if key_.value is None else key_.value.decode("utf-8")
+        locator = None if locator_.value is None else locator_.value.decode("utf-8")
+        nspace = None if nspace_.value is None else nspace_.value.decode("utf-8")
+        return Object(self.ioctx, key, locator, nspace)
 
     def __del__(self):
         run_in_thread(self.ioctx.librados.rados_nobjects_list_close, (self.ctx,))
 
 
-class XattrIterator(object):
+class XattrIterator(Iterator):
     """Extended attribute iterator"""
     def __init__(self, ioctx, it, oid):
         self.ioctx = ioctx
@@ -910,6 +935,9 @@ class XattrIterator(object):
         return self
 
     def next(self):
+        return self.__next__()
+
+    def __next__(self):
         """
         Get the next xattr on the object
 
@@ -926,7 +954,7 @@ class XattrIterator(object):
 in '%s'" % self.oid)
         if name_.value is None:
             raise StopIteration()
-        name = ctypes.string_at(name_)
+        name = ctypes.string_at(name_).decode("utf-8")
         val = ctypes.string_at(val_, len_)
         return (name, val)
 
@@ -934,7 +962,7 @@ in '%s'" % self.oid)
         run_in_thread(self.ioctx.librados.rados_getxattrs_end, (self.it,))
 
 
-class SnapIterator(object):
+class SnapIterator(Iterator):
     """Snapshot iterator"""
     def __init__(self, ioctx):
         self.ioctx = ioctx
@@ -958,6 +986,9 @@ ioctx '%s'" % self.ioctx.name)
         return self
 
     def next(self):
+        return self.__next__()
+
+    def __next__(self):
         """
         Get the next Snapshot
 
@@ -979,7 +1010,7 @@ ioctx '%s'" % self.ioctx.name)
             elif (ret != -errno.ERANGE):
                 raise make_ex(ret, "rados_snap_get_name error")
             name_len = name_len * 2
-        snap = Snap(self.ioctx, name.value, snap_id)
+        snap = Snap(self.ioctx, name.value.decode("utf-8"), snap_id)
         self.cur_snap = self.cur_snap + 1
         return snap
 
@@ -1244,7 +1275,7 @@ class Ioctx(object):
         """
         completion = self.__get_completion(oncomplete, onsafe)
         ret = run_in_thread(self.librados.rados_aio_write,
-                            (self.io, c_char_p(object_name),
+                            (self.io, cstr(object_name),
                              completion.rados_comp, c_char_p(to_write),
                              c_size_t(len(to_write)), c_uint64(offset)))
         if ret < 0:
@@ -1276,7 +1307,7 @@ class Ioctx(object):
         """
         completion = self.__get_completion(oncomplete, onsafe)
         ret = run_in_thread(self.librados.rados_aio_write_full,
-                            (self.io, c_char_p(object_name),
+                            (self.io, cstr(object_name),
                              completion.rados_comp, c_char_p(to_write),
                              c_size_t(len(to_write))))
         if ret < 0:
@@ -1307,7 +1338,7 @@ class Ioctx(object):
         """
         completion = self.__get_completion(oncomplete, onsafe)
         ret = run_in_thread(self.librados.rados_aio_append,
-                            (self.io, c_char_p(object_name),
+                            (self.io, cstr(object_name),
                              completion.rados_comp, c_char_p(to_append),
                              c_size_t(len(to_append))))
         if ret < 0:
@@ -1354,7 +1385,7 @@ class Ioctx(object):
 
         completion = self.__get_completion(oncomplete_, None)
         ret = run_in_thread(self.librados.rados_aio_read,
-                            (self.io, c_char_p(object_name),
+                            (self.io, cstr(object_name),
                              completion.rados_comp, buf, c_size_t(length),
                              c_uint64(offset)))
         if ret < 0:
@@ -1379,7 +1410,7 @@ class Ioctx(object):
         """
         completion = self.__get_completion(oncomplete, onsafe)
         ret = run_in_thread(self.librados.rados_aio_remove,
-                            (self.io, c_char_p(object_name),
+                            (self.io, cstr(object_name),
                              completion.rados_comp))
         if ret < 0:
             raise make_ex(ret, "error removing %s" % object_name)
@@ -1428,7 +1459,7 @@ class Ioctx(object):
         """
         self.require_ioctx_open()
         run_in_thread(self.librados.rados_ioctx_locator_set_key,
-                      (self.io, c_char_p(loc_key)))
+                      (self.io, cstr(loc_key)))
         self.locator_key = loc_key
 
     def get_locator_key(self):
@@ -1459,7 +1490,7 @@ class Ioctx(object):
         if nspace is None:
             nspace = ""
         run_in_thread(self.librados.rados_ioctx_set_namespace,
-                      (self.io, c_char_p(nspace)))
+                      (self.io, cstr(nspace)))
         self.nspace = nspace
 
     def get_namespace(self):
@@ -1485,7 +1516,7 @@ class Ioctx(object):
             self.state = "closed"
 
 
-    @requires(('key', str), ('data', str))
+    @requires(('key', str), ('data', bytes))
     def write(self, key, data, offset=0):
         """
         Write data to an object synchronously
@@ -1493,7 +1524,7 @@ class Ioctx(object):
         :param key: name of the object
         :type key: str
         :param data: data to write
-        :type data: str
+        :type data: bytes
         :param offset: byte offset in the object to begin writing at
         :type offset: int
 
@@ -1504,7 +1535,7 @@ class Ioctx(object):
         self.require_ioctx_open()
         length = len(data)
         ret = run_in_thread(self.librados.rados_write,
-                            (self.io, c_char_p(key), c_char_p(data),
+                            (self.io, cstr(key), c_char_p(data),
                              c_size_t(length), c_uint64(offset)))
         if ret == 0:
             return ret
@@ -1515,7 +1546,7 @@ class Ioctx(object):
             raise LogicError("Ioctx.write(%s): rados_write \
 returned %d, but should return zero on success." % (self.name, ret))
 
-    @requires(('key', str), ('data', str))
+    @requires(('key', str), ('data', bytes))
     def write_full(self, key, data):
         """
         Write an entire object synchronously.
@@ -1526,7 +1557,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         :param key: name of the object
         :type key: str
         :param data: data to write
-        :type data: str
+        :type data: bytes
 
         :raises: :class:`TypeError`
         :raises: :class:`Error`
@@ -1535,7 +1566,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
         length = len(data)
         ret = run_in_thread(self.librados.rados_write_full,
-                            (self.io, c_char_p(key), c_char_p(data),
+                            (self.io, cstr(key), c_char_p(data),
                              c_size_t(length)))
         if ret == 0:
             return ret
@@ -1546,7 +1577,7 @@ returned %d, but should return zero on success." % (self.name, ret))
             raise LogicError("Ioctx.write_full(%s): rados_write_full \
 returned %d, but should return zero on success." % (self.name, ret))
 
-    @requires(('key', str), ('data', str))
+    @requires(('key', str), ('data', bytes))
     def append(self, key, data):
         """
         Append data to an object synchronously
@@ -1554,7 +1585,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         :param key: name of the object
         :type key: str
         :param data: data to write
-        :type data: str
+        :type data: bytes
 
         :raises: :class:`TypeError`
         :raises: :class:`LogicError`
@@ -1563,7 +1594,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
         length = len(data)
         ret = run_in_thread(self.librados.rados_append,
-                            (self.io, c_char_p(key), c_char_p(data),
+                            (self.io, cstr(key), c_char_p(data),
                              c_size_t(length)))
         if ret == 0:
             return ret
@@ -1593,7 +1624,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
         ret_buf = create_string_buffer(length)
         ret = run_in_thread(self.librados.rados_read,
-                            (self.io, c_char_p(key), ret_buf, c_size_t(length),
+                            (self.io, cstr(key), ret_buf, c_size_t(length),
                              c_uint64(offset)))
         if ret < 0:
             raise make_ex(ret, "Ioctx.read(%s): failed to read %s" % (self.name, key))
@@ -1665,7 +1696,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         """
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_remove,
-                            (self.io, c_char_p(key)))
+                            (self.io, cstr(key)))
         if ret < 0:
             raise make_ex(ret, "Failed to remove '%s'" % key)
         return True
@@ -1690,7 +1721,7 @@ returned %d, but should return zero on success." % (self.name, ret))
 
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_trunc,
-                            (self.io, c_char_p(key), c_uint64(size)))
+                            (self.io, cstr(key), c_uint64(size)))
         if ret < 0:
             raise make_ex(ret, "Ioctx.trunc(%s): failed to truncate %s" % (self.name, key))
         return ret
@@ -1712,7 +1743,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         pmtime = c_uint64()
 
         ret = run_in_thread(self.librados.rados_stat,
-                            (self.io, c_char_p(key), pointer(psize),
+                            (self.io, cstr(key), pointer(psize),
                              pointer(pmtime)))
         if ret < 0:
             raise make_ex(ret, "Failed to stat %r" % key)
@@ -1737,7 +1768,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         while ret_length < 4096 * 1024 * 1024:
             ret_buf = create_string_buffer(ret_length)
             ret = run_in_thread(self.librados.rados_getxattr,
-                                (self.io, c_char_p(key), c_char_p(xattr_name),
+                                (self.io, cstr(key), cstr(xattr_name),
                                  ret_buf, c_size_t(ret_length)))
             if (ret == -errno.ERANGE):
                 ret_length *= 2
@@ -1753,7 +1784,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         Start iterating over xattrs on an object.
 
         :param oid: the name of the object to get xattrs from
-        :type key: str
+        :type oid: str
 
         :raises: :class:`TypeError`
         :raises: :class:`Error`
@@ -1762,12 +1793,12 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
         it = c_void_p(0)
         ret = run_in_thread(self.librados.rados_getxattrs,
-                            (self.io, oid, byref(it)))
+                            (self.io, cstr(oid), byref(it)))
         if ret != 0:
             raise make_ex(ret, "Failed to get rados xattrs for object %r" % oid)
         return XattrIterator(self, it, oid)
 
-    @requires(('key', str), ('xattr_name', str), ('xattr_value', str))
+    @requires(('key', str), ('xattr_name', str), ('xattr_value', bytes))
     def set_xattr(self, key, xattr_name, xattr_value):
         """
         Set an extended attribute on an object.
@@ -1777,7 +1808,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         :param xattr_name: which extended attribute to set
         :type xattr_name: str
         :param xattr_value: the value of the  extended attribute
-        :type xattr_value: str
+        :type xattr_value: bytes
 
         :raises: :class:`TypeError`
         :raises: :class:`Error`
@@ -1785,7 +1816,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         """
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_setxattr,
-                            (self.io, c_char_p(key), c_char_p(xattr_name),
+                            (self.io, cstr(key), cstr(xattr_name),
                              c_char_p(xattr_value), c_size_t(len(xattr_value))))
         if ret < 0:
             raise make_ex(ret, "Failed to set xattr %r" % xattr_name)
@@ -1807,7 +1838,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         """
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_rmxattr,
-                            (self.io, c_char_p(key), c_char_p(xattr_name)))
+                            (self.io, cstr(key), cstr(xattr_name)))
         if ret < 0:
             raise make_ex(ret, "Failed to delete key %r xattr %r" %
                           (key, xattr_name))
@@ -1844,7 +1875,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         """
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_ioctx_snap_create,
-                            (self.io, c_char_p(snap_name)))
+                            (self.io, cstr(snap_name)))
         if (ret != 0):
             raise make_ex(ret, "Failed to create snap %s" % snap_name)
 
@@ -1861,7 +1892,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         """
         self.require_ioctx_open()
         ret = run_in_thread(self.librados.rados_ioctx_snap_remove,
-                            (self.io, c_char_p(snap_name)))
+                            (self.io, cstr(snap_name)))
         if (ret != 0):
             raise make_ex(ret, "Failed to remove snap %s" % snap_name)
 
@@ -1880,7 +1911,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
         snap_id = c_uint64()
         ret = run_in_thread(self.librados.rados_ioctx_snap_lookup,
-                            (self.io, c_char_p(snap_name), byref(snap_id)))
+                            (self.io, cstr(snap_name), byref(snap_id)))
         if (ret != 0):
             raise make_ex(ret, "Failed to lookup snap %s" % snap_name)
         return Snap(self, snap_name, snap_id)
@@ -1945,7 +1976,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         key_num = len(keys)
         key_array_type = c_char_p*key_num
         key_array = key_array_type()
-        key_array[:] = keys
+        key_array[:] = [cstr(key) for key in keys]
 
         value_array_type = c_char_p*key_num
         value_array = value_array_type()
@@ -1974,7 +2005,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         :type flags: int
         """
         run_in_thread(self.librados.rados_write_op_operate,
-                      (c_void_p(write_op), self.io, c_char_p(oid),
+                      (c_void_p(write_op), self.io, cstr(oid),
                        c_long(mtime), c_int(flags),))
 
     @requires(('read_op', int), ('oid', str), ('flag', opt(int)))
@@ -1989,7 +2020,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         :type flag: int
         """
         run_in_thread(self.librados.rados_read_op_operate,
-                      (c_void_p(read_op), self.io, c_char_p(oid), c_int(flag),))
+                      (c_void_p(read_op), self.io, cstr(oid), c_int(flag),))
 
     @requires(('read_op', int), ('start_after', str), ('filter_prefix', str), ('max_return', int))
     def get_omap_vals(self, read_op, start_after, filter_prefix, max_return):
@@ -2008,8 +2039,8 @@ returned %d, but should return zero on success." % (self.name, ret))
         prval = c_int()
         iter_addr = c_void_p()
         run_in_thread(self.librados.rados_read_op_omap_get_vals,
-                      (c_void_p(read_op), c_char_p(start_after),
-                       c_char_p(filter_prefix), c_int(max_return),
+                      (c_void_p(read_op), cstr(start_after),
+                       cstr(filter_prefix), c_int(max_return),
                        byref(iter_addr), pointer(prval)))
         return OmapIterator(self, iter_addr), prval.value
 
@@ -2028,7 +2059,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         prval = c_int()
         iter_addr = c_void_p()
         run_in_thread(self.librados.rados_read_op_omap_get_keys,
-                      (c_void_p(read_op), c_char_p(start_after),
+                      (c_void_p(read_op), cstr(start_after),
                        c_int(max_return), byref(iter_addr), pointer(prval)))
         return OmapIterator(self, iter_addr), prval.value
 
@@ -2047,7 +2078,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         key_num = len(keys)
         key_array_type = c_char_p*key_num
         key_array = key_array_type()
-        key_array[:] = keys
+        key_array[:] = [cstr(key) for key in keys]
         run_in_thread(self.librados.rados_read_op_omap_get_vals_by_keys,
                       (c_void_p(read_op), byref(key_array), c_int(key_num),
                        byref(iter_addr), pointer(prval)))
@@ -2065,7 +2096,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         key_num = len(keys)
         key_array_type = c_char_p*key_num
         key_array = key_array_type()
-        key_array[:] = keys
+        key_array[:] = [cstr(key) for key in keys]
         run_in_thread(self.librados.rados_write_op_omap_rm_keys,
                       (c_void_p(write_op), byref(key_array), c_int(key_num)))
 
@@ -2105,8 +2136,8 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
 
         ret = run_in_thread(self.librados.rados_lock_exclusive,
-                            (self.io, c_char_p(key), c_char_p(name), c_char_p(cookie),
-                             c_char_p(desc),
+                            (self.io, cstr(key), cstr(name), cstr(cookie),
+                             cstr(desc),
                              timeval(duration, None) if duration is None else None,
                              c_uint8(flags)))
         if ret < 0:
@@ -2140,8 +2171,8 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
 
         ret = run_in_thread(self.librados.rados_lock_shared,
-                            (self.io, c_char_p(key), c_char_p(name), c_char_p(cookie),
-                             c_char_p(tag), c_char_p(desc),
+                            (self.io, cstr(key), cstr(name), cstr(cookie),
+                             cstr(tag), cstr(desc),
                              timeval(duration, None) if duration is None else None,
                              c_uint8(flags)))
         if ret < 0:
@@ -2166,7 +2197,7 @@ returned %d, but should return zero on success." % (self.name, ret))
         self.require_ioctx_open()
 
         ret = run_in_thread(self.librados.rados_unlock,
-                            (self.io, c_char_p(key), c_char_p(name), c_char_p(cookie)))
+                            (self.io, cstr(key), cstr(name), cstr(cookie)))
         if ret < 0:
             raise make_ex(ret, "Ioctx.rados_lock_exclusive(%s): failed to set lock %s on %s" % (self.name, name, key))
 
diff --git a/src/pybind/rbd.py b/src/pybind/rbd.py
index b570a00ebd6..b04e8453f25 100644
--- a/src/pybind/rbd.py
+++ b/src/pybind/rbd.py
@@ -15,6 +15,7 @@ to interact correctly with librbd. If unicode is passed to these
 methods, a :class:`TypeError` will be raised.
 """
 # Copyright 2011 Josh Durgin
+from collections import Iterable
 from ctypes import CDLL, c_char, c_char_p, c_size_t, c_void_p, c_int, \
     create_string_buffer, byref, Structure, c_uint64, c_int64, c_uint8, \
     CFUNCTYPE
@@ -194,6 +195,19 @@ def load_librbd():
         raise EnvironmentError("Unable to load librbd: %s" % e)
 
 
+def cstr(val, encoding="utf-8"):
+    """
+    Create a C-style string from a Python string
+
+    :param str val: Python string
+    :rtype: c_char_p
+    """
+    if val is None:
+        return c_char_p(None)
+
+    return c_char_p(val.encode(encoding))
+
+
 class RBD(object):
     """
     This class wraps librbd CRUD functions.
@@ -250,7 +264,7 @@ class RBD(object):
             if features != 0 or stripe_unit != 0 or stripe_count != 0:
                 raise InvalidArgument('format 1 images do not support feature'
                                       ' masks or non-default striping')
-            ret = self.librbd.rbd_create(ioctx.io, c_char_p(name),
+            ret = self.librbd.rbd_create(ioctx.io, cstr(name),
                                          c_uint64(size),
                                          byref(c_int(order)))
         else:
@@ -262,14 +276,14 @@ class RBD(object):
                 raise FunctionNotSupported('installed version of librbd does'
                                            ' not support stripe unit or count')
             if has_create3:
-                ret = self.librbd.rbd_create3(ioctx.io, c_char_p(name),
+                ret = self.librbd.rbd_create3(ioctx.io, cstr(name),
                                               c_uint64(size),
                                               c_uint64(features),
                                               byref(c_int(order)),
                                               c_uint64(stripe_unit),
                                               c_uint64(stripe_count))
             else:
-                ret = self.librbd.rbd_create2(ioctx.io, c_char_p(name),
+                ret = self.librbd.rbd_create2(ioctx.io, cstr(name),
                                               c_uint64(size),
                                               c_uint64(features),
                                               byref(c_int(order)))
@@ -308,9 +322,9 @@ class RBD(object):
         if not isinstance(c_name, str):
             raise TypeError('child name must be a string')
 
-        ret = self.librbd.rbd_clone(p_ioctx.io, c_char_p(p_name),
-                                    c_char_p(p_snapname),
-                                    c_ioctx.io, c_char_p(c_name),
+        ret = self.librbd.rbd_clone(p_ioctx.io, cstr(p_name),
+                                    cstr(p_snapname),
+                                    c_ioctx.io, cstr(c_name),
                                     c_uint64(features),
                                     byref(c_int(order)))
         if ret < 0:
@@ -332,7 +346,8 @@ class RBD(object):
                 break
             elif ret != -errno.ERANGE:
                 raise make_ex(ret, 'error listing images')
-        return filter(lambda name: name != '', c_names.raw.split('\0'))
+
+        return [name for name in c_names.raw.decode("utf-8").split('\0') if len(name) > 0]
 
     def remove(self, ioctx, name):
         """
@@ -353,7 +368,7 @@ class RBD(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_remove(ioctx.io, c_char_p(name))
+        ret = self.librbd.rbd_remove(ioctx.io, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error removing image')
 
@@ -371,7 +386,7 @@ class RBD(object):
         """
         if not isinstance(src, str) or not isinstance(dest, str):
             raise TypeError('src and dest must be strings')
-        ret = self.librbd.rbd_rename(ioctx.io, c_char_p(src), c_char_p(dest))
+        ret = self.librbd.rbd_rename(ioctx.io, cstr(src), cstr(dest))
         if ret != 0:
             raise make_ex(ret, 'error renaming image')
 
@@ -420,12 +435,12 @@ class Image(object):
             if not hasattr(self.librbd, 'rbd_open_read_only'):
                 raise FunctionNotSupported('installed version of librbd does '
                                            'not support open in read-only mode')
-            ret = self.librbd.rbd_open_read_only(ioctx.io, c_char_p(name),
+            ret = self.librbd.rbd_open_read_only(ioctx.io, cstr(name),
                                                  byref(self.image),
-                                                 c_char_p(snapshot))
+                                                 cstr(snapshot))
         else:
-            ret = self.librbd.rbd_open(ioctx.io, c_char_p(name),
-                                       byref(self.image), c_char_p(snapshot))
+            ret = self.librbd.rbd_open(ioctx.io, cstr(name),
+                                       byref(self.image), cstr(snapshot))
         if ret != 0:
             raise make_ex(ret, 'error opening image %s at snapshot %s' % (name, snapshot))
         self.closed = False
@@ -644,7 +659,7 @@ class Image(object):
         """
         if not isinstance(dest_name, str):
             raise TypeError('dest_name must be a string')
-        ret = self.librbd.rbd_copy(self.image, dest_ioctx.io, c_char_p(dest_name))
+        ret = self.librbd.rbd_copy(self.image, dest_ioctx.io, cstr(dest_name))
         if ret < 0:
             raise make_ex(ret, 'error copying image %s to %s' % (self.name, dest_name))
 
@@ -666,7 +681,7 @@ class Image(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_create(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_create(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error creating snapshot %s from %s' % (name, self.name))
 
@@ -680,7 +695,7 @@ class Image(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_remove(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_remove(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error removing snapshot %s from %s' % (name, self.name))
 
@@ -696,7 +711,7 @@ class Image(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_rollback(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_rollback(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error rolling back image %s to snapshot %s' % (self.name, name))
 
@@ -711,7 +726,7 @@ class Image(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_protect(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_protect(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error protecting snapshot %s@%s' % (self.name, name))
 
@@ -726,7 +741,7 @@ class Image(object):
         """
         if not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_unprotect(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_unprotect(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error unprotecting snapshot %s@%s' % (self.name, name))
 
@@ -742,7 +757,7 @@ class Image(object):
         if not isinstance(name, str):
             raise TypeError('name must be a string')
         is_protected = c_int()
-        ret = self.librbd.rbd_snap_is_protected(self.image, c_char_p(name),
+        ret = self.librbd.rbd_snap_is_protected(self.image, cstr(name),
                                                 byref(is_protected))
         if ret != 0:
             raise make_ex(ret, 'error checking if snapshot %s@%s is protected' % (self.name, name))
@@ -759,7 +774,7 @@ class Image(object):
         """
         if name is not None and not isinstance(name, str):
             raise TypeError('name must be a string')
-        ret = self.librbd.rbd_snap_set(self.image, c_char_p(name))
+        ret = self.librbd.rbd_snap_set(self.image, cstr(name))
         if ret != 0:
             raise make_ex(ret, 'error setting image %s to snapshot %s' % (self.name, name))
 
@@ -838,7 +853,7 @@ class Image(object):
         cb_holder = DiffIterateCB(iterate_cb)
         cb = RBD_DIFF_CB(cb_holder.callback)
         ret = self.librbd.rbd_diff_iterate2(self.image,
-                                            c_char_p(from_snapshot),
+                                            cstr(from_snapshot),
                                             c_uint64(offset),
                                             c_uint64(length),
                                             c_uint8(include_parent),
@@ -855,7 +870,7 @@ class Image(object):
         part of the write would fall outside the image.
 
         :param data: the data to be written
-        :type data: str
+        :type data: bytes
         :param offset: where to start writing data
         :type offset: int
         :param fadvise_flags: fadvise flags for this write
@@ -864,8 +879,8 @@ class Image(object):
         :raises: :class:`IncompleteWriteError`, :class:`LogicError`,
                  :class:`InvalidArgument`, :class:`IOError`
         """
-        if not isinstance(data, str):
-            raise TypeError('data must be a string')
+        if not isinstance(data, bytes):
+            raise TypeError('data must be a byte string')
         length = len(data)
 
         if fadvise_flags == 0:
@@ -967,7 +982,7 @@ written." % (self.name, ret, length))
             return []
         pools = c_pools.raw[:pools_size.value - 1].split('\0')
         images = c_images.raw[:images_size.value - 1].split('\0')
-        return zip(pools, images)
+        return list(zip(pools, images))
 
     def list_lockers(self):
         """
@@ -1010,13 +1025,13 @@ written." % (self.name, ret, length))
                 raise make_ex(ret, 'error listing images')
         if ret == 0:
             return []
-        clients = c_clients.raw[:clients_size.value - 1].split('\0')
-        cookies = c_cookies.raw[:cookies_size.value - 1].split('\0')
-        addrs = c_addrs.raw[:addrs_size.value - 1].split('\0')
+        clients = [client.decode("utf-8") for client in c_clients.raw[:clients_size.value - 1].split(b'\0')]
+        cookies = [cookie.decode("utf-8") for cookie in c_cookies.raw[:cookies_size.value - 1].split(b'\0')]
+        addrs = [addr.decode("utf-8") for addr in c_addrs.raw[:addrs_size.value - 1].split(b'\0')]
         return {
-            'tag'       : c_tag.value,
+            'tag'       : c_tag.value.decode("utf-8"),
             'exclusive' : exclusive.value == 1,
-            'lockers'   : zip(clients, cookies, addrs),
+            'lockers'   : list(zip(clients, cookies, addrs)),
             }
 
     def lock_exclusive(self, cookie):
@@ -1028,7 +1043,7 @@ written." % (self.name, ret, length))
         """
         if not isinstance(cookie, str):
             raise TypeError('cookie must be a string')
-        ret = self.librbd.rbd_lock_exclusive(self.image, c_char_p(cookie))
+        ret = self.librbd.rbd_lock_exclusive(self.image, cstr(cookie))
         if ret < 0:
             raise make_ex(ret, 'error acquiring exclusive lock on image')
 
@@ -1044,8 +1059,8 @@ written." % (self.name, ret, length))
             raise TypeError('cookie must be a string')
         if not isinstance(tag, str):
             raise TypeError('tag must be a string')
-        ret = self.librbd.rbd_lock_shared(self.image, c_char_p(cookie),
-                                          c_char_p(tag))
+        ret = self.librbd.rbd_lock_shared(self.image, cstr(cookie),
+                                          cstr(tag))
         if ret < 0:
             raise make_ex(ret, 'error acquiring shared lock on image')
 
@@ -1055,7 +1070,7 @@ written." % (self.name, ret, length))
         """
         if not isinstance(cookie, str):
             raise TypeError('cookie must be a string')
-        ret = self.librbd.rbd_unlock(self.image, c_char_p(cookie))
+        ret = self.librbd.rbd_unlock(self.image, cstr(cookie))
         if ret < 0:
             raise make_ex(ret, 'error unlocking image')
 
@@ -1067,8 +1082,8 @@ written." % (self.name, ret, length))
             raise TypeError('client must be a string')
         if not isinstance(cookie, str):
             raise TypeError('cookie must be a string')
-        ret = self.librbd.rbd_break_lock(self.image, c_char_p(client),
-                                         c_char_p(cookie))
+        ret = self.librbd.rbd_break_lock(self.image, cstr(client),
+                                         cstr(cookie))
         if ret < 0:
             raise make_ex(ret, 'error unlocking image')
 
@@ -1082,7 +1097,7 @@ class DiffIterateCB(object):
         return 0
 
 
-class SnapIterator(object):
+class SnapIterator(Iterable):
     """
     Iterator over snapshot info for an image.
 
@@ -1110,11 +1125,11 @@ class SnapIterator(object):
                 raise make_ex(ret, 'error listing snapshots for image %s' % (image.name,))
 
     def __iter__(self):
-        for i in xrange(self.num_snaps):
+        for i in range(self.num_snaps):
             yield {
                 'id'   : self.snaps[i].id,
                 'size' : self.snaps[i].size,
-                'name' : self.snaps[i].name,
+                'name' : self.snaps[i].name.decode("utf-8"),
                 }
 
     def __del__(self):