Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-29
17:04:31 cfriesen I updated the vTPM spec, I think it should address all the issues that were raised in the pervious version.
17:05:00 mnaser mriedem: not even mixed version hypervisor
17:05:12 mnaser i think this breaks live migrations of any vm created before pre-rocky to post-rocky
17:05:13 mdbooth_390 mnaser: Sorta. Mixed on the same host.
17:05:20 mnaser yeah i guess
17:05:29 cfriesen mixed version nova, not hypervisor
17:06:02 mriedem so live migrating a queens vm to a stein?
17:06:36 cfriesen I think queens to rocky, potentially.
17:07:44 cfriesen for vifs of type ethernet and bridge, on libvirt 3.3.0+
17:07:59 cfriesen (assuming it's https://github.com/openstack/nova/commit/f02b3800051234ecc14f3117d5987b1a8ef75877 that is the culprit)
17:08:34 openstack Launchpad bug 1713783 in OpenStack Compute (nova) pike "duplicate for #1583504 After failed evacuation the recovered source compute tries to delete the instance" [High,Fix committed] - Assigned to Matt Riedemann (mriedem)
17:08:34 tomtom001 https://bugs.launchpad.net/nova/+bug/1583504 This is still an issue when: You shutoff CPU-A - then evacuate VM-B, then if it fails you decide to delete VM-B. When you start CPU-A, it cannot find VM-B and fails to start nova-compute
17:08:38 mdbooth_390 mriedem: You create an instance on Queens, then upgrade nova on that compute to Rocky, then try to migrate it somewhere.
17:09:05 mdbooth_390 mnaser: ^^^^ ?
17:13:58 mnaser mdbooth_390: correct
17:14:25 mnaser mriedem: cfriesen migrating a vm that was running before nova was upgraded to rocky would be affected
17:33:46 mnaser oh wait
17:33:49 mnaser that might not even be the commit that broke it
17:34:08 mnaser https://github.com/openstack/nova/blob/f02b3800051234ecc14f3117d5987b1a8ef75877/nova/virt/libvirt/driver.py#L6900-L6904
17:34:47 mnaser because at the time we didnt regen network
17:36:15 mnaser https://github.com/openstack/nova/commit/2b52cde565d542c03f004b48ee9c1a6a25f5b7cd
17:36:18 mnaser this one specifically broke it
17:39:36 openstack Launchpad bug 1800511 in OpenStack Compute (nova) "VMs started before Rocky upgrade cannot be live migrated" [Undecided,New]
17:39:36 mnaser https://bugs.launchpad.net/nova/+bug/1800511
17:39:39 mnaser i think this might be an easy fix actaully
17:40:17 mnaser looks like _update_vif_xml is only used in migrations
17:44:49 mriedem it is
17:44:53 mriedem live migrations
17:45:44 mnaser ok, so i will try to get a failing test case there
17:46:15 mnaser where no mtu => adds an mtu, seems straightforward
17:46:56 mriedem is the libvirt version the same on both hosts?
17:47:02 mriedem >=3.3?
17:47:12 mnaser mriedem: nope its 3.9
17:47:25 mriedem which is >3.3 where mtu is supported for those vif types
17:47:55 mnaser yeah but the problem is if you provide an mtu the internals of qemu change and the instance becomes non-livemigrateable
17:48:16 mnaser and actually if you do virsh dumpxml --migratable <instance> it doesnt even include mtu
17:48:23 mnaser so libvirt kinda conciously doesnt put it, but we do
17:51:14 mriedem ok so https://github.com/openstack/nova/commit/2b52cde565d542c03f004b48ee9c1a6a25f5b7cd exposes the issue because it calls the get_config code for the vif which returns the mtu now
17:51:19 mriedem and we try sending that to the dest
17:51:48 mnaser mriedem: correct
17:51:51 cdent jaypipes: if you loving on the gabbi integration tests, I made this one for nova (the required tempest and gabbi-tempest changes have merged): https://review.openstack.org/#/c/613386/
17:51:54 mriedem i think this would have been an issue regardless of https://github.com/openstack/nova/commit/2b52cde565d542c03f004b48ee9c1a6a25f5b7cd if the guest xml on the source had the mtru in the vif xml
17:51:59 mriedem *mtu
17:52:08 mriedem since we'd just send that over unchanged
17:52:31 mnaser mriedem: if mtu exists on both sides, it's fine
17:52:37 mnaser if mtu doesn't in one, it blows up
17:52:53 mriedem sure but i don't think https://github.com/openstack/nova/commit/2b52cde565d542c03f004b48ee9c1a6a25f5b7cd changes that
17:52:54 mnaser so if source has mtu, then dest will get it .. if source doesnt, dest won't.. and we're good
17:53:15 mnaser it calls self.vif_driver.get_config
17:53:18 mnaser which gets mtu configs
17:53:23 mriedem *on the source host*
17:53:31 mriedem we don't know what is configured for mtu on the dest
17:53:37 mriedem since that's not in the LiveMigrateData object
17:53:39 mriedem as far as i know
17:53:54 mriedem _update_vif_xml is called from the source
17:53:59 mriedem using migrate data from the dest
17:54:18 mnaser yes, to "modify" the xml which will be shipped to dest to start the instance to recieve data.. right?
17:54:29 mriedem yes
17:54:44 mriedem but if the xml on the source already had mtu in it, and the dest doesn't support that, then kaboom anyway right?
17:54:52 cfriesen mriedem: I think the issue is that the instance originally didn't have mtu in it
17:54:59 cfriesen then we upgraded nova, and now it would have mtu
17:55:10 mnaser the host_mtu was added really long time ago in qemu
17:55:20 mnaser so its not likely that qemu will crash for lack of that feature
17:55:38 mnaser but if the destination doesnt get a host_mtu.. it just .. doesnt set one and moves on
17:55:53 cfriesen nova on the dest doesn't need to have the mtu, but the xml that we pass the live migration needs to match the xml that was used to start the running instance.
17:56:10 mnaser cfriesen: doing a much better job explaining this than i am anyways :p
17:58:53 mriedem "but the xml that we pass the live migration needs to match the xml that was used to start the running instance."
17:58:57 mriedem ok i guess i didn't know that was a thing
18:00:44 mriedem i'm not sure why it cares about the xml from when the guest was last started if we're sending over the xml that we want
18:01:07 mriedem same is true for things like block devices
18:02:07 mriedem so the proposed fix is going to be checking within _update_vif_xml if the source guest xml had mtu set before we call get_config to get the new dest xml vif config?
18:02:21 mriedem and if not, omit it from the dest xml?
18:02:33 mnaser mriedem: i think that is where im going
18:02:47 mnaser i think i have a failing test too
18:04:18 mnaser another reason it was missed was because the tests were testing bridge => vhostuser
18:04:23 mnaser vhostuser doesnt set mtus
18:04:29 mnaser so the tests didnt show mtu appearing
18:07:39 mriedem i'm pretty sure the live migration tests for _update_vif_xml are just using fake xml aren't they?
18:08:31 mriedem well maybe not, but i'm not sure they would have cared much about the delta given part of the reason for that change was live migrating between vif types which is going to yield different xml
18:11:54 mnaser mriedem: kinda, also only bridge and ethernet net_type support mtu
18:12:12 mnaser and the tests was testing bridge=>vhostuser which meant mtu was never added anyways
18:19:09 openstackgerrit Jack Ding proposed openstack/nova-specs master: High Precision Event Timer (HPET) on x86 guests https://review.openstack.org/607989
18:21:29 openstackgerrit Mohammed Naser proposed openstack/nova master: Add tests for bug #1800511 https://review.openstack.org/614004
18:21:30 openstack bug 1800511 in OpenStack Compute (nova) rocky "VMs started before Rocky upgrade cannot be live migrated" [High,Triaged] https://launchpad.net/bugs/1800511
18:21:42 mnaser mriedem: cfriesen mdbooth_390 ^ round 1.. that's a failing test that's commented out, i'll work on the actual fix now
18:29:30 mnaser mriedem: sorry to bother ya, do you think _update_vif_xml() should make sure "mtu" is there (if it was before) and removed if it wasn't there before.. or should i pass an extra arg to get_vif_config() inside _update_vif_xml() like migration=True and handle it in the vif layer?
18:30:46 mriedem i think it would probably be simpler to isolate it within _update_vif_xml rather than plumb down through to the vif get_config stuff
18:31:01 mriedem i just wonder how whack-a-mole this is going to be going forward with any changes to that os-vif code
18:31:10 mriedem or if this mtu thing is an outlier
18:32:40 mnaser mriedem: it looks like the mtu is an outlier because $kvm things .. but i dunno
18:34:33 mriedem yeah i was just wondering based on that bz
18:34:35 mriedem which i skimmed
18:34:40 mriedem sounds like libvirt had to revert that
18:34:45 mriedem s/that/something/
18:34:51 mnaser yep, they did back in 3.3
18:34:56 mnaser so we really just repeated their bug a layer above
18:35:03 mriedem sweet
18:44:15 openstack bug 1800511 in OpenStack Compute (nova) rocky "VMs started before Rocky upgrade cannot be live migrated" [High,Triaged] https://launchpad.net/bugs/1800511
18:44:15 openstackgerrit Mohammed Naser proposed openstack/nova master: Add tests for bug #1800511 https://review.openstack.org/614004
18:47:57 cfriesen mriedem: missed your comment earlier. the XML that we send as part of the migration is used to start up qemu on the dest, this creates data structures in memory, and those data structures need to match the data structures for the guest on the source node.

Earlier   Later