Merge "Add Windows volume backup support"

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
 

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 "
@@ -441,12 +445,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
@@ -561,15 +565,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
@@ -1046,7 +1051,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}

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)

cinder/tests/unit/backup/test_backup.py

@@ -611,7 +611,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):
@@ -636,7 +636,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)
 
@@ -650,6 +649,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'])
@@ -1084,10 +1084,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',
@@ -1097,7 +1101,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
@@ -1109,8 +1112,10 @@ class BackupTestCase(BaseBackupTest):
                                               '_attach_device')
         mock_attach_device.return_value = attach_info
 
+        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)

cinder/tests/unit/test_utils.py

@@ -355,6 +355,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')

cinder/utils.py

@@ -437,6 +437,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()
 

cinder/volume/driver.py

@@ -1258,6 +1258,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
 

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

cinder/volume/drivers/windows/smbfs.py

@@ -655,3 +655,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

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.