Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[vmware] Improve the order of attaching/detaching volumes #514

Open
wants to merge 1 commit into
base: stable/xena-m3
Choose a base branch
from
Open

[vmware] Improve the order of attaching/detaching volumes #514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 32 additions & 4 deletions nova/tests/unit/virt/vmwareapi/test_vmops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,17 +1269,45 @@ def test_finish_revert_in_place_migration_relocate_fails(self):
def test_attach_volumes(self, fake_attach_volume):
block_device_info = {
'block_device_mapping': [
{'mount_device': '/dev/sda', 'connection_info': {'id': 'c'}},
{'mount_device': '/dev/sda', 'connection_info': {'id': 'a'}},
{'mount_device': '/dev/sdb', 'connection_info': {'id': 'b'}},
{'mount_device': '/dev/sdc', 'connection_info': {'id': 'a'}},
{'mount_device': '/dev/sdc', 'connection_info': {'id': 'c'}},
{'mount_device': '/dev/sdd', 'boot_index': 1,
'connection_info': {'id': 'd'}},
{'mount_device': '/dev/sde', 'boot_index': 0,
'connection_info': {'id': 'e'}},
]
}
self._vmops._attach_volumes(self._instance, block_device_info,
mock.sentinel.adapter_type)
fake_attach_volume.assert_has_calls([
mock.call({'id': 'c'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'b'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'e'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'd'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'a'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'b'}, self._instance, mock.sentinel.adapter_type),
mock.call({'id': 'c'}, self._instance, mock.sentinel.adapter_type),
])

@mock.patch.object(volumeops.VMwareVolumeOps, 'detach_volume')
def test_detach_volumes(self, fake_attach_volume):
block_device_info = {
'block_device_mapping': [
{'mount_device': '/dev/sda', 'connection_info': {'id': 'a'}},
{'mount_device': '/dev/sdb', 'connection_info': {'id': 'b'}},
{'mount_device': '/dev/sdc', 'connection_info': {'id': 'c'}},
{'mount_device': '/dev/sdd', 'boot_index': 1,
'connection_info': {'id': 'd'}},
{'mount_device': '/dev/sde', 'boot_index': 0,
'connection_info': {'id': 'e'}},
]
}
self._vmops._detach_volumes(self._instance, block_device_info)
fake_attach_volume.assert_has_calls([
mock.call({'id': 'c'}, self._instance),
mock.call({'id': 'b'}, self._instance),
mock.call({'id': 'a'}, self._instance),
mock.call({'id': 'd'}, self._instance),
mock.call({'id': 'e'}, self._instance),
])

@mock.patch.object(vmops.VMwareVMOps, '_get_instance_metadata')
Expand Down
25 changes: 20 additions & 5 deletions nova/virt/vmwareapi/vmops.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3285,9 +3285,7 @@ def _detach_volumes(self, instance, block_device_info):
disks = driver.block_device_info_get_mapping(block_device_info)
# Detach the volumes in reverse order, so if we roll it back
# that the device order will still be preserved
for disk in sorted(disks,
reverse=True,
key=itemgetter('mount_device')):
for disk in self._sort_disks(disks, reverse=True):
try:
self._volumeops.detach_volume(disk['connection_info'],
instance)
Expand All @@ -3299,15 +3297,32 @@ def _attach_volumes(self, instance, block_device_info, adapter_type,
existing_disks=None):
disks = driver.block_device_info_get_mapping(block_device_info)
# make sure the disks are attached by the device_name order
for disk in sorted(disks,
key=itemgetter('mount_device')):
for disk in self._sort_disks(disks):
if existing_disks and disk['volume_id'] in existing_disks:
continue

adapter_type = disk.get('disk_bus') or adapter_type
self._volumeops.attach_volume(disk['connection_info'], instance,
adapter_type)

def _sort_disks(self, disks, reverse=False):
"""Since mount_device can be controlled from outside, we want to
make sure the boot_index >= 0 devices are always at the top.
"""
boot_disks = sorted(
(d for d in disks if d.get('boot_index') is not None),
key=itemgetter('boot_index'),
reverse=reverse)
other_disks = sorted(
(d for d in disks if d.get('boot_index') is None),
key=itemgetter('mount_device'),
reverse=reverse)

if reverse:
return other_disks + boot_disks
else:
return boot_disks + other_disks

def _find_esx_host(self, cluster_ref, ds_ref):
"""Find ESX host in the specified cluster which is also connected to
the specified datastore.
Expand Down