commit 0e9cd6c4d66ca4afb95bb60edb412af9e96c546e
Author: Brooks Kaminski <email address hidden>
Date: Sat Jan 27 01:35:07 2018 -0600
XenAPI: XCP2.1+ Swallow VDI_NOT_IN_MAP Exception
Changes within XenAPI have enforced a more strict policy when checking
assert_can_migrate. In particular when checking the source_vdi:dest_sr
mapping it insists that the SR actually exist. This is not a problem for
local disks, however this assertation is called extremely early in the
live migration process (check_can_migrate_source) which is called from
conductor, which makes a problem for attached volumes.
This early in the process the host has just barely been chosen and no SR
information has been configured yet for these volumes or their initiators.
Additionally we cannot prepare this SR any earlier as BDM information is
not set up until the pre_live_migration method. With the options to either
skip this assertion completely or swallow the exception, I have chosen to
swallow the exception. My reasons for this are two-fold:
1. --block-migration can be called without regard for whether an iSCSI
volume is attached, and we still want to ensure that VIF, CPU and other
factors are checked, and not just skip all checks entirely.
2. Currently the Assert only exists within the --block-migration code
base but this needs to change. A future commit will remove this logic
to ensure that the commit runs without this flag. Once that is done we
want to be able to continue to use this Exception swallow logic rather
than continuing to skip the assert for all XCP2.1.0+ even without volumes.
This decision should help us handle less work in a future commit and does not
seem to align with the goals of that commit, where it does align properly here.
This commit still changes very little of the current codebase and puts us in
a good position to refactor the way this is handled at a later date, while
adding a TODO note to correct VM.assert_can_migrate only running during a
block migration.
Additionally there seems to be some confusion that the mapping data that is
generated during this initial trip through _call_live_migrate_command is needed
to continue along the code, however this data appears to be purely used to send
the mapping information through the assertation call, and is then discarded.
The only data returned from these methods is the original dest_data which
is carried into the live_migration method. The _call_live_migration method is
called again during the live_migration() method, and during this time it does
need that mapping to send along to XenAPI for the actual migration, but not
yet. Because this codebase is so confusing, I am providing a little bit of
context on the movement of these variables with some psuedocode:
LiveMigrationTask._find_destination()
Scheduler Things. Gets a Dest ref.
_check_compatible_with_source_hyp
_call_livem_checks_on_host(host) <-----
_check_can_live_migrate_destination()
returns Host Node Name and Host Compute. That's all.
(Checks for the Assert)
try:
migrate_data = check_can_live_migrate_source(dest_check_data)
return migrate_data
---VMOPS--
check_can_migrate_source(self, ctxt, instance_ref, dest_check_data)
if block_migration:
_call_live_migration_command(assert_can_migrate) _generate_vdi_map()
Does NOT return
ALSO Does NOT return
return dest_check_data
The changes made to address this issue are a fairly simple oslo_utils
version check. To pull this data I created two new host related methods
within VMops as well as a new import of oslo_config.versionutils.
I believe these methods ultimately belong in the xenapi.host class, but
for two very small methods I believed it better to avoid such a large import
to get minimal data.
Finally, an adjustment to the Fake XenAPI driver had to be made as it currently
does not include the host details beyond hostname environment in the
create_host check. The change amends the stub dictionary to include this data.
Reviewed: https:/ /review. openstack. org/538415 /git.openstack. org/cgit/ openstack/ nova/commit/ ?id=0e9cd6c4d66 ca4afb95bb60edb 412af9e96c546e
Committed: https:/
Submitter: Zuul
Branch: master
commit 0e9cd6c4d66ca4a fb95bb60edb412a f9e96c546e
Author: Brooks Kaminski <email address hidden>
Date: Sat Jan 27 01:35:07 2018 -0600
XenAPI: XCP2.1+ Swallow VDI_NOT_IN_MAP Exception
Changes within XenAPI have enforced a more strict policy when checking can_migrate. In particular when checking the source_vdi:dest_sr can_migrate_ source) which is called from
assert_
mapping it insists that the SR actually exist. This is not a problem for
local disks, however this assertation is called extremely early in the
live migration process (check_
conductor, which makes a problem for attached volumes.
This early in the process the host has just barely been chosen and no SR
information has been configured yet for these volumes or their initiators.
Additionally we cannot prepare this SR any earlier as BDM information is
not set up until the pre_live_migration method. With the options to either
skip this assertion completely or swallow the exception, I have chosen to
swallow the exception. My reasons for this are two-fold:
1. --block-migration can be called without regard for whether an iSCSI
volume is attached, and we still want to ensure that VIF, CPU and other
factors are checked, and not just skip all checks entirely.
2. Currently the Assert only exists within the --block-migration code
base but this needs to change. A future commit will remove this logic
to ensure that the commit runs without this flag. Once that is done we
want to be able to continue to use this Exception swallow logic rather
than continuing to skip the assert for all XCP2.1.0+ even without volumes.
This decision should help us handle less work in a future commit and does not can_migrate only running during a
seem to align with the goals of that commit, where it does align properly here.
This commit still changes very little of the current codebase and puts us in
a good position to refactor the way this is handled at a later date, while
adding a TODO note to correct VM.assert_
block migration.
Additionally there seems to be some confusion that the mapping data that is migrate_ command is needed migration method is
generated during this initial trip through _call_live_
to continue along the code, however this data appears to be purely used to send
the mapping information through the assertation call, and is then discarded.
The only data returned from these methods is the original dest_data which
is carried into the live_migration method. The _call_live_
called again during the live_migration() method, and during this time it does
need that mapping to send along to XenAPI for the actual migration, but not
yet. Because this codebase is so confusing, I am providing a little bit of
context on the movement of these variables with some psuedocode:
--- CONDUCTOR. TASKS.LIVE_ MIGRATE- -- onTask. Execute( ) _find_destinati on() <----- live_migration( self, host, instance, destination,
block_migrate, migration, migrate_data)
LiveMigrati
self.
Unrelated Work
compute.
LiveMigrati onTask. _find_destinati on() compatible_ with_source_ hyp livem_checks_ on_host( host) <----- check_can_ live_migrate_ destination( )
Scheduler Things. Gets a Dest ref.
_check_
_call_
_
returns Host Node Name and Host Compute. That's all.
--- COMPUTE. MANAGER- -- live_migration_ destination check_data = xenops. can_live_ migrate_ destination
_do / _check_
dest_
(Checks for the Assert) live_migrate_ source( dest_check_ data)
try:
migrate_data = check_can_
return migrate_data
---VMOPS-- can_migrate_ source( self, ctxt, instance_ref, dest_check_data) call_live_ migration_ command( assert_ can_migrate)
_generate_ vdi_map( )
check_
if block_migration:
_
Does NOT return
ALSO Does NOT return
return dest_check_data
The changes made to address this issue are a fairly simple oslo_utils versionutils.
version check. To pull this data I created two new host related methods
within VMops as well as a new import of oslo_config.
I believe these methods ultimately belong in the xenapi.host class, but
for two very small methods I believed it better to avoid such a large import
to get minimal data.
Finally, an adjustment to the Fake XenAPI driver had to be made as it currently
does not include the host details beyond hostname environment in the
create_host check. The change amends the stub dictionary to include this data.
Change-Id: Ie5295564a68f87 7fc061376c00f3f a706370a251
Closes-Bug: #1704071