Comment 12 for bug 1814245

Reviewed: https://review.opendev.org/683008
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=c8396dcae3eee4bdf7d2d5b4d0cc0f9b58d16117
Submitter: Zuul
Branch: stable/pike

commit c8396dcae3eee4bdf7d2d5b4d0cc0f9b58d16117
Author: Matthew Booth <email address hidden>
Date: Fri Mar 9 14:41:49 2018 +0000

    Avoid redundant initialize_connection on source post live migration

    During live migration we update bdm.connection_info for attached volumes
    in pre_live_migration to reflect the new connection on the destination
    node. This means that after migration completes the BDM no longer has a
    reference to the original connection_info to do the detach on the source
    host. To address this, change I3dfb75eb added a second call to
    initialize_connection on the source host to re-fetch the source host
    connection_info before calling disconnect.

    Unfortunately the cinder driver interface does not strictly require that
    multiple calls to initialize_connection will return consistent results.
    Although they normally do in practice, there is at least one cinder
    driver (delliscsi) which doesn't. This results in a failure to
    disconnect on the source host post migration.

    This change avoids the issue entirely by fetching the BDMs prior to
    modification on the destination node. As well as working round this
    specific issue, it also avoids a redundant cinder call in all cases.

    Note that this massively simplifies post_live_migration in the libvirt
    driver. The complexity removed was concerned with reconstructing the
    original connection_info. This required considering the cinder v2 and v3
    use cases, and reconstructing the multipath_id which was written to
    connection_info by the libvirt fibrechannel volume connector on
    connection. These things are not necessary when we just use the original
    data unmodified.

    Other drivers affected are Xenapi and HyperV. Xenapi doesn't touch
    volumes in post_live_migration, so is unaffected. HyperV did not
    previously account for differences in connection_info between source and
    destination, so was likely previously broken. This change should fix it.

    Conflicts:
          nova/compute/manager.py
          nova/objects/migrate_data.py
          nova/tests/unit/compute/test_compute.py
          nova/tests/unit/compute/test_compute_mgr.py
          nova/tests/unit/virt/libvirt/test_driver.py
          nova/virt/libvirt/driver.py

    NOTE(mriedem): The conflicts are primarily due to not having change
    I0bfb11296430dfffe9b091ae7c3a793617bd9d0d in Pike. In addition, the
    libvirt driver conflicts are also due to not having change
    I61a0bee9e71e9a67f6a7c04a7bfd6e77fe818a77 nor change
    Ica323b87fa85a454fca9d46ada3677f18fe50022 in Pike.

    NOTE(melwitt): The difference from the Queens change in
    nova/compute/manager.py to be able to treat the instance variable as
    a dict is because _do_live_migration is expected to be able to handle
    an old-style instance in Pike (this is exposed in the unit test).
    Other sources of conflict in nova/tests/unit/compute/test_compute.py
    are because change I9068a5a5b47cef565802a6d58f37777464644100 is not in
    Pike. The difference from the Queens change in
    nova/tests/unit/compute/test_compute_mgr.py to add a mock for the new
    get_by_instance_uuid database call is needed because in Queens the
    test was using a MagicMock as the compute manager whereas in Pike the
    test is using a real compute manager. The mock needs to be added to
    avoid a test error accessing the database in a NoDBTestCase. Another
    source of conflicts in nova/tests/unit/compute/test_compute_mgr.py is
    because changes I0f3ab6604d8b79bdb75cf67571e359cfecc039d8 and
    I9068a5a5b47cef565802a6d58f37777464644100 are not in Pike.

    Closes-Bug: #1754716
    Closes-Bug: #1814245
    Change-Id: I0390c9ff51f49b063f736ca6ef868a4fa782ede5
    (cherry picked from commit b626c0dc7b113365002e743e6de2aeb40121fc81)
    (cherry picked from commit 75e0f5a9b18293546db0ddf0fb073854e6704115)
    (cherry picked from commit 013f421bca4067bd430a9fac1e3b290cf1388ee4)