From: Victor Stinner Date: Tue, 23 Jun 2015 09:55:40 +0000 (+0200) Subject: Fix Python 3 issues in ceph and rbd drivers X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c967b44110e1338449f07663fb3e4b0fc6ff5913;p=openstack-build%2Fcinder-build.git Fix Python 3 issues in ceph and rbd drivers * Add cinder.utils.convert_str() function: helper to convert a string to a native string: convert to bytes on Python 2, convert to Unicode on Python 3. Function needed by ceph and rbd drivers. * Replace encodeutils.safe_encode() with utils.convert_str() * test_backup_ceph: get the size of the volume file using os.fstat() to fix a test. Before the volume size was a mock, but comparison between integers and mocks raise a TypeError on Python 3. * Fix RBDDriver._update_volume_stats(): replace a/b with a//b to get an integer on Python 2 and Python 3 (a/b always returns a float on Python 3) * Replace map() with a list-comprehension in RBDDriver because a list is expected. On Python 3, map() returns an iterator. * tox.ini: add to following tests to Python 3.4 - cinder.tests.unit.test_backup_ceph - cinder.tests.unit.test_rbd Blueprint cinder-python3 Change-Id: I7acb0d9eed0d351798133e0c6ef55408fed925dd --- diff --git a/cinder/backup/drivers/ceph.py b/cinder/backup/drivers/ceph.py index 207e09977..089a5add7 100644 --- a/cinder/backup/drivers/ceph.py +++ b/cinder/backup/drivers/ceph.py @@ -51,7 +51,6 @@ import time import eventlet from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import encodeutils from oslo_utils import excutils from oslo_utils import units from six.moves import range @@ -104,7 +103,7 @@ class VolumeMetadataBackup(object): @property def name(self): - return encodeutils.safe_encode("backup.%s.meta" % self._backup_id) + return utils.convert_str("backup.%s.meta" % self._backup_id) @property def exists(self): @@ -181,9 +180,9 @@ class CephBackupDriver(driver.BackupDriver): self.rbd_stripe_count = 0 self.rbd_stripe_unit = 0 - self._ceph_backup_user = encodeutils.safe_encode(CONF.backup_ceph_user) - self._ceph_backup_pool = encodeutils.safe_encode(CONF.backup_ceph_pool) - self._ceph_backup_conf = encodeutils.safe_encode(CONF.backup_ceph_conf) + self._ceph_backup_user = utils.convert_str(CONF.backup_ceph_user) + self._ceph_backup_pool = utils.convert_str(CONF.backup_ceph_pool) + self._ceph_backup_conf = utils.convert_str(CONF.backup_ceph_conf) def _validate_string_args(self, *args): """Ensure all args are non-None and non-empty.""" @@ -239,8 +238,7 @@ class CephBackupDriver(driver.BackupDriver): conffile=self._ceph_backup_conf) try: client.connect() - pool_to_open = encodeutils.safe_encode(pool or - self._ceph_backup_pool) + pool_to_open = utils.convert_str(pool or self._ceph_backup_pool) ioctx = client.open_ioctx(pool_to_open) return client, ioctx except self.rados.Error: @@ -263,13 +261,13 @@ class CephBackupDriver(driver.BackupDriver): """ # Ensure no unicode if diff_format: - return encodeutils.safe_encode("volume-%s.backup.base" % volume_id) + return utils.convert_str("volume-%s.backup.base" % volume_id) else: if backup_id is None: msg = _("Backup id required") raise exception.InvalidParameterValue(msg) - return encodeutils.safe_encode("volume-%s.backup.%s" % - (volume_id, backup_id)) + return utils.convert_str("volume-%s.backup.%s" + % (volume_id, backup_id)) def _discard_bytes(self, volume, offset, length): """Trim length bytes from offset. @@ -473,7 +471,7 @@ class CephBackupDriver(driver.BackupDriver): # Since we have deleted the base image we can delete the source # volume backup snapshot. - src_name = encodeutils.safe_encode(volume_id) + src_name = utils.convert_str(volume_id) if src_name in self.rbd.RBD().list(client.ioctx): LOG.debug("Deleting source volume snapshot '%(snapshot)s' " "for backup %(basename)s.", @@ -538,15 +536,14 @@ class CephBackupDriver(driver.BackupDriver): if from_snap is not None: cmd1.extend(['--from-snap', from_snap]) if src_snap: - path = encodeutils.safe_encode("%s/%s@%s" % - (src_pool, src_name, src_snap)) + path = utils.convert_str("%s/%s@%s" + % (src_pool, src_name, src_snap)) else: - path = encodeutils.safe_encode("%s/%s" % (src_pool, src_name)) - + path = utils.convert_str("%s/%s" % (src_pool, src_name)) cmd1.extend([path, '-']) cmd2 = ['rbd', 'import-diff'] + dest_ceph_args - rbd_path = encodeutils.safe_encode("%s/%s" % (dest_pool, dest_name)) + rbd_path = utils.convert_str("%s/%s" % (dest_pool, dest_name)) cmd2.extend(['-', rbd_path]) ret, stderr = self._piped_execute(cmd1, cmd2) @@ -757,8 +754,8 @@ class CephBackupDriver(driver.BackupDriver): return backup_snaps def _get_new_snap_name(self, backup_id): - return encodeutils.safe_encode("backup.%s.snap.%s" % - (backup_id, time.time())) + return utils.convert_str("backup.%s.snap.%s" + % (backup_id, time.time())) def _get_backup_snap_name(self, rbd_image, name, backup_id): """Return the name of the snapshot associated with backup_id. @@ -932,7 +929,7 @@ class CephBackupDriver(driver.BackupDriver): with rbd_driver.RADOSClient(self, self._ceph_backup_pool) as client: adjust_size = 0 base_image = self.rbd.Image(client.ioctx, - encodeutils.safe_encode(backup_base), + utils.convert_str(backup_base), read_only=True) try: if restore_length != base_image.size(): @@ -942,7 +939,7 @@ class CephBackupDriver(driver.BackupDriver): if adjust_size: with rbd_driver.RADOSClient(self, src_pool) as client: - restore_vol_encode = encodeutils.safe_encode(restore_vol) + restore_vol_encode = utils.convert_str(restore_vol) dest_image = self.rbd.Image(client.ioctx, restore_vol_encode) try: LOG.debug("Adjusting restore vol size") diff --git a/cinder/tests/unit/test_backup_ceph.py b/cinder/tests/unit/test_backup_ceph.py index c497df22d..4ae3c64db 100644 --- a/cinder/tests/unit/test_backup_ceph.py +++ b/cinder/tests/unit/test_backup_ceph.py @@ -307,6 +307,7 @@ class BackupCephTestCase(test.TestCase): rbd1 = mock.Mock() rbd1.read.side_effect = fake_read + rbd1.size.return_value = os.fstat(self.volume_file.fileno()).st_size rbd2 = mock.Mock() rbd2.write.side_effect = mock_write_data diff --git a/cinder/utils.py b/cinder/utils.py index b204a62e8..e2b1082ec 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -40,6 +40,7 @@ from oslo_concurrency import lockutils from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import encodeutils from oslo_utils import importutils from oslo_utils import timeutils import retrying @@ -816,3 +817,21 @@ def convert_version_to_str(version_int): def convert_version_to_tuple(version_str): return tuple(int(part) for part in version_str.split('.')) + + +def convert_str(text): + """Convert to native string. + + Convert bytes and Unicode strings to native strings: + + * convert to bytes on Python 2: + encode Unicode using encodeutils.safe_encode() + * convert to Unicode on Python 3: decode bytes from UTF-8 + """ + if six.PY2: + return encodeutils.safe_encode(text) + else: + if isinstance(text, bytes): + return text.decode('utf-8') + else: + return text diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py index ed665751b..80683f16a 100644 --- a/cinder/volume/drivers/rbd.py +++ b/cinder/volume/drivers/rbd.py @@ -23,7 +23,6 @@ import tempfile from eventlet import tpool from oslo_config import cfg from oslo_log import log as logging -from oslo_utils import encodeutils from oslo_utils import units import six from six.moves import urllib @@ -101,9 +100,9 @@ class RBDImageMetadata(object): """RBD image metadata to be used with RBDImageIOWrapper.""" def __init__(self, image, pool, user, conf): self.image = image - self.pool = encodeutils.safe_encode(pool) - self.user = encodeutils.safe_encode(user) - self.conf = encodeutils.safe_encode(conf) + self.pool = utils.convert_str(pool) + self.user = utils.convert_str(user) + self.conf = utils.convert_str(conf) class RBDImageIOWrapper(io.RawIOBase): @@ -218,11 +217,11 @@ class RBDVolumeProxy(object): read_only=False): client, ioctx = driver._connect_to_rados(pool) if snapshot is not None: - snapshot = encodeutils.safe_encode(snapshot) + snapshot = utils.convert_str(snapshot) try: self.volume = driver.rbd.Image(ioctx, - encodeutils.safe_encode(name), + utils.convert_str(name), snapshot=snapshot, read_only=read_only) except driver.rbd.Error: @@ -287,7 +286,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, 'rbd_ceph_conf', 'rbd_pool']: val = getattr(self.configuration, attr) if val is not None: - setattr(self.configuration, attr, encodeutils.safe_encode(val)) + setattr(self.configuration, attr, utils.convert_str(val)) def check_for_setup_error(self): """Returns an error if prerequisites aren't met.""" @@ -330,7 +329,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, clustername=self.configuration.rbd_cluster_name, conffile=self.configuration.rbd_ceph_conf)) if pool is not None: - pool = encodeutils.safe_encode(pool) + pool = utils.convert_str(pool) else: pool = self.configuration.rbd_pool @@ -408,8 +407,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, pool['name'] == self.configuration.rbd_pool][0]['stats'] stats['free_capacity_gb'] = ( - pool_stats['max_avail'] / units.Gi) - used_capacity_gb = pool_stats['bytes_used'] / units.Gi + pool_stats['max_avail'] // units.Gi) + used_capacity_gb = pool_stats['bytes_used'] // units.Gi stats['total_capacity_gb'] = (stats['free_capacity_gb'] + used_capacity_gb) except self.rados.Error: @@ -458,8 +457,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, and that clone has rbd_max_clone_depth clones behind it, the source volume will be flattened. """ - src_name = encodeutils.safe_encode(src_vref['name']) - dest_name = encodeutils.safe_encode(volume['name']) + src_name = utils.convert_str(src_vref['name']) + dest_name = utils.convert_str(volume['name']) flatten_parent = False # Do full copy if requested @@ -544,7 +543,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, with RADOSClient(self) as client: self.RBDProxy().create(client.ioctx, - encodeutils.safe_encode(volume['name']), + utils.convert_str(volume['name']), size, order, old_format=False, @@ -563,10 +562,10 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, with RADOSClient(self, src_pool) as src_client: with RADOSClient(self) as dest_client: self.RBDProxy().clone(src_client.ioctx, - encodeutils.safe_encode(src_image), - encodeutils.safe_encode(src_snap), + utils.convert_str(src_image), + utils.convert_str(src_snap), dest_client.ioctx, - encodeutils.safe_encode(volume['name']), + utils.convert_str(volume['name']), features=src_client.features) def _resize(self, volume, **kwargs): @@ -655,7 +654,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, """Deletes a logical volume.""" # NOTE(dosaboy): this was broken by commit cbe1d5f. Ensure names are # utf-8 otherwise librbd will barf. - volume_name = encodeutils.safe_encode(volume['name']) + volume_name = utils.convert_str(volume['name']) with RADOSClient(self) as client: try: rbd_image = self.rbd.Image(client.ioctx, volume_name) @@ -725,7 +724,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, def create_snapshot(self, snapshot): """Creates an rbd snapshot.""" with RBDVolumeProxy(self, snapshot['volume_name']) as volume: - snap = encodeutils.safe_encode(snapshot['name']) + snap = utils.convert_str(snapshot['name']) volume.create_snap(snap) volume.protect_snap(snap) @@ -733,8 +732,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, """Deletes an rbd snapshot.""" # NOTE(dosaboy): this was broken by commit cbe1d5f. Ensure names are # utf-8 otherwise librbd will barf. - volume_name = encodeutils.safe_encode(snapshot['volume_name']) - snap_name = encodeutils.safe_encode(snapshot['name']) + volume_name = utils.convert_str(snapshot['volume_name']) + snap_name = utils.convert_str(snapshot['name']) with RBDVolumeProxy(self, volume_name) as volume: try: volume.unprotect_snap(snap_name) @@ -806,7 +805,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, if not location.startswith(prefix): reason = _('Not stored in rbd') raise exception.ImageUnacceptable(image_id=location, reason=reason) - pieces = map(urllib.parse.unquote, location[len(prefix):].split('/')) + pieces = [urllib.parse.unquote(loc) + for loc in location[len(prefix):].split('/')] if any(map(lambda p: p == '', pieces)): reason = _('Blank components') raise exception.ImageUnacceptable(image_id=location, reason=reason) @@ -985,8 +985,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, with RADOSClient(self) as client: rbd_name = existing_ref['source-name'] self.RBDProxy().rename(client.ioctx, - encodeutils.safe_encode(rbd_name), - encodeutils.safe_encode(volume['name'])) + utils.convert_str(rbd_name), + utils.convert_str(volume['name'])) def manage_existing_get_size(self, volume, existing_ref): """Return size of an existing image for manage_existing. @@ -1004,7 +1004,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD, raise exception.ManageExistingInvalidReference( existing_ref=existing_ref, reason=reason) - rbd_name = encodeutils.safe_encode(existing_ref['source-name']) + rbd_name = utils.convert_str(existing_ref['source-name']) with RADOSClient(self) as client: # Raise an exception if we didn't find a suitable rbd image. diff --git a/tox.ini b/tox.ini index b6e0f3994..5501e4d22 100644 --- a/tox.ini +++ b/tox.ini @@ -31,6 +31,7 @@ commands = python -m testtools.run \ cinder.tests.unit.test_api_urlmap \ cinder.tests.unit.test_backup \ + cinder.tests.unit.test_backup_ceph \ cinder.tests.unit.test_backup_driver_base \ cinder.tests.unit.test_block_device \ cinder.tests.unit.test_cloudbyte \ @@ -56,6 +57,7 @@ commands = cinder.tests.unit.test_nimble \ cinder.tests.unit.test_openvstorage \ cinder.tests.unit.test_qos_specs \ + cinder.tests.unit.test_rbd \ cinder.tests.unit.test_remotefs \ cinder.tests.unit.test_replication \ cinder.tests.unit.test_san \