bdm for swap created with no regard for previous

[Jesse J. Cook]

Observed nova pass the correct device number during vbd creation to xenserver, but the nova mapping created in the database was for the wrong device.

A bdm for swap, based on the flavor, is created on https://github.com/openstack/nova/blob/master/nova/compute/api.py#L725, but the swap is actually created on https://github.com/openstack/nova/blob/master/nova/virt/xenapi/vmops.py#L746, again based on flavor, but with no regard for the previously created bdm for swap.

Perhaps we should implement default_device_names_for_instance() in xen driver (as referenced on https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L1631).

Ramifications: https://bugs.launchpad.net/nova/+bug/1482403/comments/1
Steps to Reproduce: https://bugs.launchpad.net/nova/+bug/1482403/comments/3

[Jesse J. Cook (jesse-j-cook)]

Ramifications: we lose a device (we cannot use because nova thinks it's used for swap as record in the bdm). Affects xenserver 6.0 (pvhvm instances can only have 4 devices).

[Eli Qiao (taget-9)]

hi Jesse
codes changes on github, I think it's better to describe the steps of the process.

[Corey Wright (coreywright)]

Steps to recreate:
1. Create instance based on a flavor that includes swap on a Xen-based openstack installation.
2. See that in the database the instance has a BDM for swap at /dev/xvdb, but nothing for /dev/xvdc.
3. See that on the instance it has swap at /dev/xvdc, but nothing at /dev/xvdb.
4. Try to attach a volume at /dev/xvdb, but fail because the database has swap at /dev/xvdb.
5. Try to attach a volume at /dev/xvdc, but fail because the instance has swap at /dev/xvdc.

The above problem was introduced with http://git.openstack.org/cgit/openstack/nova/commit/?id=7f8128f87f5a2fa93c857295fb7e4163986eda25.

The below explanation is relative to a Nova Git commit as of today (and therefor won't changed).

A BDM for swap, based on the flavor, is created in nova/compute/api.py (pseudo call-stack: [1][2][3][4]), which is incorrectly (based on Xen and historical behavior) assigned to /dev/xvdb (pseudo call-stack: [5][6][7][8][9][10][11]), of which specifically get_next_device_name() insures that /dev/xvdc, the proper device name, will never be used [12].

[1] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/api.py#L775
[2] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/api.py#L829
[3] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/api.py#L750
[4] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/api.py#L761
[5] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/manager.py#L1663
[6] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/manager.py#L1705
[7] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/manager.py#L1642
[8] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/manager.py#L1650
[9] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/utils.py#L107
[10] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/utils.py#L120
[11] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/utils.py#L127
[12] https://github.com/openstack/nova/blob/bd3b5a62/nova/compute/utils.py#L173

The swap is actually created in nova/virt/xenapi/vmops.py (pseudo call-stack: [13][14][15]), again based on flavor, but with no regard for the previously created bdm for swap.

[13] https://github.com/openstack/nova/blob/bd3b5a62/nova/virt/xenapi/vmops.py#L678
[14] https://github.com/openstack/nova/blob/bd3b5a62/nova/virt/xenapi/vmops.py#L741
[15] https://github.com/openstack/nova/blob/bd3b5a62/nova/virt/xenapi/vmops.py#L89

[Jesse J. Cook (jesse-j-cook)]

Thanks Corey!

Eli Qiao, is that sufficient information? I linked Corey's comment in the bug description.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/221968

[OpenStack Infra (hudson-openstack)]

Change abandoned by Michael Still (<email address hidden>) on branch: master
Review: https://review.openstack.org/221968
Reason: This code hasn't been updated in a long time, and is in merge conflict. I am going to abandon this review, but feel free to restore it if you're still working on this.