diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py index a5436154244..9c528b455db 100644 --- a/cinder/backup/chunkeddriver.py +++ b/cinder/backup/chunkeddriver.py @@ -25,6 +25,7 @@ import abc import hashlib import json import os +import sys import eventlet from oslo_config import cfg @@ -41,6 +42,9 @@ from cinder import objects from cinder.objects import fields from cinder.volume import utils as volume_utils +if sys.platform == 'win32': + from os_win import utilsfactory as os_win_utilsfactory + LOG = logging.getLogger(__name__) chunkedbackup_service_opts = [ @@ -116,6 +120,13 @@ class ChunkedBackupDriver(driver.BackupDriver): self._get_compressor(CONF.backup_compression_algorithm) self.support_force_delete = True + if sys.platform == 'win32' and self.chunk_size_bytes % 4096: + # The chunk size must be a multiple of the sector size. In order + # to fail out early and avoid attaching the disks, we'll just + # enforce the chunk size to be a multiple of 4096. + err = _("Invalid chunk size. It must be a multiple of 4096.") + raise exception.InvalidConfigurationValue(message=err) + def _get_object_writer(self, container, object_name, extra_metadata=None): """Return writer proxy-wrapped to execute methods in native thread.""" writer = self.get_object_writer(container, object_name, extra_metadata) @@ -453,6 +464,12 @@ class ChunkedBackupDriver(driver.BackupDriver): extra_usage_info= object_meta) + def _get_win32_phys_disk_size(self, disk_path): + win32_diskutils = os_win_utilsfactory.get_diskutils() + disk_number = win32_diskutils.get_device_number_from_device_name( + disk_path) + return win32_diskutils.get_disk_size(disk_number) + def backup(self, backup, volume_file, backup_metadata=True): """Backup the given volume. @@ -488,6 +505,13 @@ class ChunkedBackupDriver(driver.BackupDriver): 'backup. Do a full backup.') raise exception.InvalidBackup(reason=err) + if sys.platform == 'win32': + # When dealing with Windows physical disks, we need the exact + # size of the disk. Attempting to read passed this boundary will + # lead to an IOError exception. At the same time, we cannot + # seek to the end of file. + win32_disk_size = self._get_win32_phys_disk_size(volume_file.name) + (object_meta, object_sha256, extra_metadata, container, volume_size_bytes) = self._prepare_backup(backup) @@ -526,7 +550,14 @@ class ChunkedBackupDriver(driver.BackupDriver): LOG.debug('Cancel the backup process of %s.', backup.id) break data_offset = volume_file.tell() - data = volume_file.read(self.chunk_size_bytes) + + if sys.platform == 'win32': + read_bytes = min(self.chunk_size_bytes, + win32_disk_size - data_offset) + else: + read_bytes = self.chunk_size_bytes + data = volume_file.read(read_bytes) + if data == b'': break diff --git a/cinder/backup/manager.py b/cinder/backup/manager.py index 0cb3900b65b..d0b1ae3c1c1 100644 --- a/cinder/backup/manager.py +++ b/cinder/backup/manager.py @@ -275,6 +275,10 @@ class BackupManager(manager.ThreadPoolManager): try: temp_snapshot = objects.Snapshot.get_by_id( ctxt, backup.temp_snapshot_id) + # We may want to consider routing those calls through the + # cinder API. + temp_snapshot.status = fields.SnapshotStatus.DELETING + temp_snapshot.save() self.volume_rpcapi.delete_snapshot(ctxt, temp_snapshot) except exception.SnapshotNotFound: LOG.debug("Could not find temp snapshot %(snap)s to clean " @@ -432,12 +436,12 @@ class BackupManager(manager.ThreadPoolManager): if (isinstance(device_path, six.string_types) and not os.path.isdir(device_path)): if backup_device.secure_enabled: - with open(device_path) as device_file: + with open(device_path, 'rb') as device_file: updates = backup_service.backup( backup, tpool.Proxy(device_file)) else: with utils.temporary_chown(device_path): - with open(device_path) as device_file: + with open(device_path, 'rb') as device_file: updates = backup_service.backup( backup, tpool.Proxy(device_file)) # device_path is already file-like so no need to open it @@ -551,15 +555,16 @@ class BackupManager(manager.ThreadPoolManager): # with native threads proxy-wrapping the device file object. try: device_path = attach_info['device']['path'] + open_mode = 'rb+' if os.name == 'nt' else 'wb' if (isinstance(device_path, six.string_types) and not os.path.isdir(device_path)): if secure_enabled: - with open(device_path, 'wb') as device_file: + with open(device_path, open_mode) as device_file: backup_service.restore(backup, volume.id, tpool.Proxy(device_file)) else: with utils.temporary_chown(device_path): - with open(device_path, 'wb') as device_file: + with open(device_path, open_mode) as device_file: backup_service.restore(backup, volume.id, tpool.Proxy(device_file)) # device_path is already file-like so no need to open it @@ -987,7 +992,8 @@ class BackupManager(manager.ThreadPoolManager): protocol, use_multipath=use_multipath, device_scan_attempts=device_scan_attempts, - conn=conn) + conn=conn, + expect_raw_disk=True) vol_handle = connector.connect_volume(conn['data']) return {'conn': conn, 'device': vol_handle, 'connector': connector} diff --git a/cinder/tests/unit/backup/drivers/test_backup_swift.py b/cinder/tests/unit/backup/drivers/test_backup_swift.py index d7faf0a4b04..c31d4f9bc82 100644 --- a/cinder/tests/unit/backup/drivers/test_backup_swift.py +++ b/cinder/tests/unit/backup/drivers/test_backup_swift.py @@ -32,6 +32,7 @@ import mock from oslo_config import cfg from swiftclient import client as swift +from cinder.backup import chunkeddriver from cinder.backup.drivers import swift as swift_dr from cinder import context from cinder import db @@ -873,3 +874,71 @@ class BackupSwiftTestCase(test.TestCase): self.assertEqual('none', result[0]) self.assertEqual(already_compressed_data, result[1]) + + +class WindowsBackupSwiftTestCase(BackupSwiftTestCase): + # We're running all the parent class tests, while doing + # some patching in order to simulate Windows behavior. + def setUp(self): + self._mock_utilsfactory = mock.Mock() + + platform_patcher = mock.patch('sys.platform', 'win32') + platform_patcher.start() + self.addCleanup(platform_patcher.stop) + + super(WindowsBackupSwiftTestCase, self).setUp() + + read = self.volume_file.read + + def win32_read(sz): + # We're simulating the Windows behavior. + if self.volume_file.tell() > fake_get_size(): + raise IOError() + return read(sz) + + read_patcher = mock.patch.object( + self.volume_file, 'read', win32_read) + read_patcher.start() + self.addCleanup(read_patcher.stop) + + def fake_get_size(*args, **kwargs): + pos = self.volume_file.tell() + sz = self.volume_file.seek(0, 2) + self.volume_file.seek(pos) + return sz + + self._disk_size_getter_mocker = mock.patch.object( + swift_dr.SwiftBackupDriver, + '_get_win32_phys_disk_size', + fake_get_size) + + self._disk_size_getter_mocker.start() + self.addCleanup(self._disk_size_getter_mocker.stop) + + def test_invalid_chunk_size(self): + self.flags(backup_swift_object_size=1000) + # We expect multiples of 4096 + self.assertRaises(exception.InvalidConfigurationValue, + swift_dr.SwiftBackupDriver, + self.ctxt) + + @mock.patch.object(chunkeddriver, 'os_win_utilsfactory', create=True) + def test_get_phys_disk_size(self, mock_utilsfactory): + # We're patching this method in setUp, so we need to + # retrieve the original one. Note that we'll get an unbound + # method. + service = swift_dr.SwiftBackupDriver(self.ctxt) + get_disk_size = self._disk_size_getter_mocker.temp_original + + disk_utils = mock_utilsfactory.get_diskutils.return_value + disk_utils.get_device_number_from_device_name.return_value = ( + mock.sentinel.dev_num) + disk_utils.get_disk_size.return_value = mock.sentinel.disk_size + + disk_size = get_disk_size(service, mock.sentinel.disk_path) + + self.assertEqual(mock.sentinel.disk_size, disk_size) + disk_utils.get_device_number_from_device_name.assert_called_once_with( + mock.sentinel.disk_path) + disk_utils.get_disk_size.assert_called_once_with( + mock.sentinel.dev_num) diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 21ac6cdd5ee..0ce5de24681 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -591,7 +591,7 @@ class BackupTestCase(BaseBackupTest): @mock.patch('cinder.utils.brick_get_connector_properties') @mock.patch('cinder.volume.rpcapi.VolumeAPI.get_backup_device') @mock.patch('cinder.utils.temporary_chown') - @mock.patch('six.moves.builtins.open') + @mock.patch('six.moves.builtins.open', wraps=open) @mock.patch.object(os.path, 'isdir', return_value=False) def test_create_backup(self, mock_isdir, mock_open, mock_temporary_chown, mock_get_backup_device, mock_get_conn): @@ -616,7 +616,6 @@ class BackupTestCase(BaseBackupTest): mock_attach_device.return_value = attach_info properties = {} mock_get_conn.return_value = properties - mock_open.return_value = open('/dev/null', 'rb') self.backup_mgr.create_backup(self.ctxt, backup) @@ -630,6 +629,7 @@ class BackupTestCase(BaseBackupTest): force=True, ignore_errors=True) + mock_open.assert_called_once_with('/dev/null', 'rb') vol = objects.Volume.get_by_id(self.ctxt, vol_id) self.assertEqual('available', vol['status']) self.assertEqual('backing-up', vol['previous_status']) @@ -1011,10 +1011,14 @@ class BackupTestCase(BaseBackupTest): @mock.patch('cinder.utils.brick_get_connector_properties') @mock.patch('cinder.utils.temporary_chown') - @mock.patch('six.moves.builtins.open') + @mock.patch('six.moves.builtins.open', wraps=open) @mock.patch.object(os.path, 'isdir', return_value=False) + @ddt.data({'os_name': 'nt', 'exp_open_mode': 'rb+'}, + {'os_name': 'posix', 'exp_open_mode': 'wb'}) + @ddt.unpack def test_restore_backup(self, mock_isdir, mock_open, - mock_temporary_chown, mock_get_conn): + mock_temporary_chown, mock_get_conn, + os_name, exp_open_mode): """Test normal backup restoration.""" vol_size = 1 vol_id = self._create_volume_db_entry(status='restoring-backup', @@ -1024,7 +1028,6 @@ class BackupTestCase(BaseBackupTest): properties = {} mock_get_conn.return_value = properties - mock_open.return_value = open('/dev/null', 'wb') mock_secure_enabled = ( self.volume_mocks['secure_file_operations_enabled']) mock_secure_enabled.return_value = False @@ -1036,8 +1039,10 @@ class BackupTestCase(BaseBackupTest): '_attach_device') mock_attach_device.return_value = attach_info - self.backup_mgr.restore_backup(self.ctxt, backup, vol_id) + with mock.patch('os.name', os_name): + self.backup_mgr.restore_backup(self.ctxt, backup, vol_id) + mock_open.assert_called_once_with('/dev/null', exp_open_mode) mock_temporary_chown.assert_called_once_with('/dev/null') mock_get_conn.assert_called_once_with() mock_secure_enabled.assert_called_once_with(self.ctxt, vol) diff --git a/cinder/tests/unit/test_utils.py b/cinder/tests/unit/test_utils.py index f7ab2dd35d1..195eb3cd1fe 100644 --- a/cinder/tests/unit/test_utils.py +++ b/cinder/tests/unit/test_utils.py @@ -342,6 +342,16 @@ class TemporaryChownTestCase(test.TestCase): mock_stat.assert_called_once_with(test_filename) self.assertFalse(mock_exec.called) + @mock.patch('os.name', 'nt') + @mock.patch('os.stat') + @mock.patch('cinder.utils.execute') + def test_temporary_chown_win32(self, mock_exec, mock_stat): + with utils.temporary_chown(mock.sentinel.path): + pass + + mock_exec.assert_not_called() + mock_stat.assert_not_called() + class TempdirTestCase(test.TestCase): @mock.patch('tempfile.mkdtemp') diff --git a/cinder/utils.py b/cinder/utils.py index 4e941144180..d2e87ee079c 100644 --- a/cinder/utils.py +++ b/cinder/utils.py @@ -431,6 +431,13 @@ def temporary_chown(path, owner_uid=None): :params owner_uid: UID of temporary owner (defaults to current user) """ + + if os.name == 'nt': + LOG.debug("Skipping chown for %s as this operation is " + "not available on Windows.", path) + yield + return + if owner_uid is None: owner_uid = os.getuid() diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 3ef1904d67c..44f1320c603 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -1255,6 +1255,7 @@ class BaseVD(object): temp_snap_ref.destroy() temp_snap_ref.status = fields.SnapshotStatus.AVAILABLE + temp_snap_ref.progress = '100%' temp_snap_ref.save() return temp_snap_ref diff --git a/cinder/volume/drivers/windows/iscsi.py b/cinder/volume/drivers/windows/iscsi.py index 11fb9c399ac..0e618135034 100644 --- a/cinder/volume/drivers/windows/iscsi.py +++ b/cinder/volume/drivers/windows/iscsi.py @@ -345,3 +345,6 @@ class WindowsISCSIDriver(driver.ISCSIDriver): additional_size_mb = (new_size - old_size) * 1024 self._tgt_utils.extend_wt_disk(volume.name, additional_size_mb) + + def backup_use_temp_snapshot(self): + return False diff --git a/cinder/volume/drivers/windows/smbfs.py b/cinder/volume/drivers/windows/smbfs.py index 3f240a0d89f..4abea70f249 100644 --- a/cinder/volume/drivers/windows/smbfs.py +++ b/cinder/volume/drivers/windows/smbfs.py @@ -656,3 +656,6 @@ class WindowsSmbfsDriver(remotefs_drv.RevertToSnapshotMixin, # The SMBFS driver does not manage file permissions. We chose # to let this up to the deployer. pass + + def backup_use_temp_snapshot(self): + return True diff --git a/releasenotes/notes/windows-volume-backup-b328858a20f5a499.yaml b/releasenotes/notes/windows-volume-backup-b328858a20f5a499.yaml new file mode 100644 index 00000000000..e05601c5691 --- /dev/null +++ b/releasenotes/notes/windows-volume-backup-b328858a20f5a499.yaml @@ -0,0 +1,9 @@ +--- +features: + - | + The Cinder Volume Backup service can now be run on Windows. It supports + backing up volumes exposed by SMBFS/iSCSI Windows Cinder Volume backends, + as well as any other Cinder backend that's accessible on Windows (e.g. + SANs exposing volumes via iSCSI/FC). + + The Swift and Posix backup drivers are known to be working on Windows.