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

Reorder root disk as first disk in OSMorphing stage #202

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Reorder root disk as first disk in OSMorphing stage #202

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
8 changes: 7 additions & 1 deletion coriolis/osmorphing/osmount/windows.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,13 @@ def mount_os(self):

for fs_root in [r for r in fs_roots if not r[:-1] == system_drive]:
if self._conn.test_path("%sWindows\\System32" % fs_root):
return fs_root, None
root_disk_id = None
root_disk_ids = self._get_disk_ids_from_drive_letters(
[fs_root.split(':')[0]])
if root_disk_ids:
root_disk_id = root_disk_ids[0]

return fs_root, root_disk_id

raise exception.OperatingSystemNotFound("root partition not found")

Expand Down
65 changes: 62 additions & 3 deletions coriolis/tasks/osmorphing_tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Copyright 2016 Cloudbase Solutions Srl
# All Rights Reserved.

import re

from oslo_log import log as logging

from coriolis import constants
Expand All @@ -14,6 +16,55 @@
LOG = logging.getLogger(__name__)


def _reorder_root_disk(volumes_info, root_device, os_type):
"""
Reorders volumes_info so that the root disk will always be the first volume

root_device is returned by the OSMount Tools as the root partition device
(i.e. /dev/vdd2 for linux).

For Linux, we need to strip the trailing digits
to get the actual disk device. After that, we convert the last letter of
the disk device name into the equivalent index by alphabetical order.

Windows OSMount Tools should directly return the root disk index extracted
from diskpart (as string; i.e. '1').

Reordering is done by swapping the indexes of the volumes_info list.
"""
if not root_device:
LOG.warn('os_root_dev was not returned by OSMount Tools. '
'Skipping root disk reordering')
return

# the default disk device of the migrated VM should be /dev/Xdb, because
# /dev/Xda should be the worker's root disk. Same for Windows: Disk 0
# should be the worker's root disk, and Disk 1 the migrated VM's root disk
linux_root_default = 'b'
windows_root_default = 1

supported_os_types = [constants.OS_TYPE_LINUX, constants.OS_TYPE_WINDOWS]
if os_type == constants.OS_TYPE_LINUX:
pattern = r'[0-9]'
root_disk = re.sub(pattern, '', root_device)
disk_index = ord(root_disk[-1]) - ord(linux_root_default)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please declare a default value for disk_index before these if statements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may seem like a stretch but this code will completely miss its mark in case of clouds where separate device models would dictate a separate device name prefix (e.g. a setup where 2-3 disks are "/dev/vdN" and 2-3 are "/dev/sdn".

From my PoV there is no sane way of handling this without having the providers knowing and passing the disk device mapping (i.e. "Disk ID %s is /dev/sdc") from the DEPLOY_OS_MORPHING_RESOURCES task directly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code will also completely miss the mark if the worker image itself has multiple disks, so a better solution is clearly required.

elif os_type == constants.OS_TYPE_WINDOWS:
disk_index = int(root_device) - windows_root_default
else:
LOG.warn('Root disk reordering only supported for %s. Got OS type: %s.'
'Skipping root disk reordering', supported_os_types, os_type)
return

if disk_index > 0:
if disk_index < len(volumes_info):
volumes_info[0], volumes_info[disk_index] = (
volumes_info[disk_index], volumes_info[0])
else:
LOG.warn('Disk device name index out of range: %s for device %s'
'Skipping root disk reordering', disk_index, root_device)



class OSMorphingTask(base.TaskRunner):

@classmethod
Expand All @@ -28,7 +79,7 @@ def get_required_task_info_properties(cls):

@classmethod
def get_returned_task_info_properties(cls):
return []
return ["instance_deployment_info"]

@classmethod
def get_required_provider_types(cls):
Expand All @@ -53,13 +104,14 @@ def _run(self, ctxt, instance, origin, destination, task_info,
osmorphing_connection_info = base.unmarshal_migr_conn_info(
task_info['osmorphing_connection_info'])
osmorphing_info = task_info.get('osmorphing_info', {})
instance_deployment_info = task_info.get('instance_deployment_info', {})

user_scripts = task_info.get("user_scripts")
os_type = osmorphing_info.get("os_type")
instance_script = None
if user_scripts:
instance_script = user_scripts.get("instances", {}).get(instance)
if not instance_script:
os_type = osmorphing_info.get("os_type")
if os_type:
instance_script = user_scripts.get(
"global", {}).get(os_type)
Expand All @@ -72,7 +124,14 @@ def _run(self, ctxt, instance, origin, destination, task_info,
instance_script,
event_handler)

return {}
volumes_info = instance_deployment_info.get('volumes_info', [])
LOG.debug('Volumes info before root disk reordering: %s', volumes_info)
_reorder_root_disk(volumes_info, osmorphing_info.get('os_root_dev'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having us randomly depend on osmorphing_manager.morph_image setting the os_root_dev within the mutable osmorphing_info is not exactly safe.

Please make osmorphing_manager.morph_image explicitly return the root device.
Additionally, please ensure within osmorphing_manager.morph_image that a copy of the osmorphing_info is made/mutated before being passed to the actual OSMount/OSMorphing tools.

os_type)
LOG.debug('Volumes info after root disk reordering: %s', volumes_info)

return {
'instance_deployment_info': instance_deployment_info}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simply putting the volumes_info back into the instance_deployment_info and hoping that the plugins will handle it isn't a safe way to go about things...

Please make the OSMorphingTask both require and return the volumes_info and manipulate it directly instead.
Out goal should be to not have to care/check on any of the plugins following this patch.



class DeployOSMorphingResourcesTask(base.TaskRunner):
Expand Down