From 07ff70a23b1f5f98e10f795af9954a47313a0a74 Mon Sep 17 00:00:00 2001 From: Danny Al-Gaaf Date: Tue, 10 Feb 2015 16:43:21 +0100 Subject: [PATCH] quobyte: remove dependency to xattr The Quobyte driver introduced an new dependency to xattr python binding. This driver is the only consumer of xattr. Instead of introducing new dependencies call getfattr binary as already done in the quobyte nova volume driver. Closes-Bug: 1420332 Co-Authored-By: Marc Koderer Change-Id: I83f97310313cf7a603996ae64327002cdf0ad5fc Signed-off-by: Danny Al-Gaaf --- cinder/tests/test_quobyte.py | 24 ++++++++++++------------ cinder/volume/drivers/quobyte.py | 29 +++++++++++++++++------------ requirements.txt | 1 - 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/cinder/tests/test_quobyte.py b/cinder/tests/test_quobyte.py index d4f0547c617..90b73f7e814 100644 --- a/cinder/tests/test_quobyte.py +++ b/cinder/tests/test_quobyte.py @@ -130,8 +130,7 @@ class QuobyteDriverTestCase(test.TestCase): mock.patch.object(self._driver, '_execute'), mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' '.read_proc_mount'), - mock.patch('xattr.getxattr') - ) as (mock_execute, mock_open, mock_getxattr): + ) as (mock_execute, mock_open): # Content of /proc/mount (not mounted yet). mock_open.return_value = StringIO.StringIO( "/dev/sda5 / ext4 rw,relatime,data=ordered 0 0") @@ -144,31 +143,32 @@ class QuobyteDriverTestCase(test.TestCase): mount_call = mock.call( 'mount.quobyte', self.TEST_QUOBYTE_VOLUME, self.TEST_MNT_POINT, run_as_root=False) - mock_execute.assert_has_calls([mkdir_call, mount_call], - any_order=False) - mock_getxattr.assert_called_once_with(self.TEST_MNT_POINT, - 'quobyte.info') + + getfattr_call = mock.call( + 'getfattr', '-n', 'quobyte.info', self.TEST_MNT_POINT, + run_as_root=False) + + mock_execute.assert_has_calls( + [mkdir_call, mount_call, getfattr_call], any_order=False) def test_mount_quobyte_already_mounted_detected_seen_in_proc_mount(self): with contextlib.nested( mock.patch.object(self._driver, '_execute'), mock.patch('cinder.volume.drivers.quobyte.QuobyteDriver' '.read_proc_mount'), - mock.patch('xattr.getxattr') - ) as (mock_execute, mock_open, mock_getxattr): + ) as (mock_execute, mock_open): # Content of /proc/mount (already mounted). mock_open.return_value = StringIO.StringIO( "quobyte@%s %s fuse rw,nosuid,nodev,noatime,user_id=1000" ",group_id=100,default_permissions,allow_other 0 0" % (self.TEST_QUOBYTE_VOLUME, self.TEST_MNT_POINT)) - mock_getxattr.return_value = "non-empty string" self._driver._mount_quobyte(self.TEST_QUOBYTE_VOLUME, self.TEST_MNT_POINT) - self.assertFalse(mock_execute.called) - mock_getxattr.assert_called_once_with(self.TEST_MNT_POINT, - 'quobyte.info') + mock_execute.assert_called_once_with( + 'getfattr', '-n', 'quobyte.info', self.TEST_MNT_POINT, + run_as_root=False) def test_mount_quobyte_should_suppress_and_log_already_mounted_error(self): """Based on /proc/mount, the file system is not mounted yet. However, diff --git a/cinder/volume/drivers/quobyte.py b/cinder/volume/drivers/quobyte.py index 2119a0d78f9..caf3a78e261 100644 --- a/cinder/volume/drivers/quobyte.py +++ b/cinder/volume/drivers/quobyte.py @@ -19,11 +19,10 @@ import os from oslo_concurrency import processutils from oslo_config import cfg -import xattr from cinder import compute from cinder import exception -from cinder.i18n import _, _LE, _LI, _LW +from cinder.i18n import _, _LI, _LW from cinder.image import image_utils from cinder.openstack.common import fileutils from cinder.openstack.common import log as logging @@ -418,13 +417,19 @@ class QuobyteDriver(remotefs_drv.RemoteFSSnapDriver): raise if mounted: - try: - xattr.getxattr(mount_path, 'quobyte.info') - except Exception as exc: - msg = _LE("The mount %(mount_path)s is not a valid" - " Quobyte USP volume. Error: %(exc)s") \ - % {'mount_path': mount_path, 'exc': exc} - raise exception.VolumeDriverException(msg) - if not os.access(mount_path, os.W_OK | os.X_OK): - LOG.warn(_LW("Volume is not writable. Please broaden the file" - " permissions. Mount: %s"), mount_path) + self._validate_volume(mount_path) + + def _validate_volume(self, mount_path): + """Wraps execute calls for checking validity of a Quobyte volume""" + command = ['getfattr', "-n", "quobyte.info", mount_path] + try: + self._execute(*command, run_as_root=False) + except processutils.ProcessExecutionError as exc: + msg = (_("The mount %(mount_path)s is not a valid" + " Quobyte USP volume. Error: %(exc)s") + % {'mount_path': mount_path, 'exc': exc}) + raise exception.VolumeDriverException(msg) + + if not os.access(mount_path, os.W_OK | os.X_OK): + LOG.warn(_LW("Volume is not writable. Please broaden the file" + " permissions. Mount: %s"), mount_path) diff --git a/requirements.txt b/requirements.txt index 26c1227f226..7e53a3f7b5b 100644 --- a/requirements.txt +++ b/requirements.txt @@ -44,4 +44,3 @@ WebOb>=1.2.3 wsgiref>=0.1.2 oslo.i18n>=1.3.0 # Apache-2.0 oslo.vmware>=0.9.0 # Apache-2.0 -xattr>=0.4