openstack
/
cinder
Code Issues Proposed changes

Add Windows volume backup support 

This patch makes a few small changes that are required in order to
have the Cinder Backup service working on Windows.

- all physial disks must be open in byte mode. 'rb+' must be used
when writing.
- reading passed the disk size boundary will not return an empty
string, raising an IOError instead. For this reason, we're avoiding
doing it.
- we ensure that the chunk size is a multiple of the sector size.
- the chmod command is not available on Windows. Although changing
owners is possible, it is not needed. For this reason, the
'temporary_chown' helper will be a noop on Windows. It's easier to
do it here rather than do platform checks wherever this gets called.
- when connecting the volumes, we pass the 'expect_raw_disk' argument,
which provides a hint to the connector about what we expect. This
allows the SMBFS connector to return a mounted raw disk path instead
of a virtual image path.
- when the driver provides temporary snapshots to be used during
the backup process, the API is bypassed. For this reason, we need to
ensure that the snapshot state and progress gets updated accordingly.
Otherwise, this breaks the nova assisted snapshot workflow.

We're doing platform checks, ensuring that we don't break/change
the current workflow.

The Swift and Posix backup drivers are known to be working on Windows.

Implements: blueprint windows-smb-backup

Depends-On: #I20f791482fb0912772fa62d2949fa5becaec5675

Change-Id: I8769a135974240fdf7cebd4b6d74aaa439ba1f27
This commit is contained in:
Lucian Petrut 2017-12-15 14:37:14 +02:00
parent 32a08e4d6a
commit 302402df33
10 changed files with 156 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
cinder/backup/chunkeddriver.py
View File

@ -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

16
cinder/backup/manager.py
View File

@ -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}

69
cinder/tests/unit/backup/drivers/test_backup_swift.py
View File

@ -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)

17
cinder/tests/unit/backup/test_backup.py
View File

@ -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)

10
cinder/tests/unit/test_utils.py
View File

@ -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')

7
cinder/utils.py
View File

@ -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()

1
cinder/volume/driver.py
View File

@ -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

3
cinder/volume/drivers/windows/iscsi.py
View File

@ -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

3
cinder/volume/drivers/windows/smbfs.py
View File

@ -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

9
releasenotes/notes/windows-volume-backup-b328858a20f5a499.yaml Normal file
View File

@ -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.