Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2018-10-29
16:49:17 mnaser it refreshes all the nic info
16:49:18 mnaser from that xml
16:49:59 mnaser https://github.com/openstack/nova/blob/f02b3800051234ecc14f3117d5987b1a8ef75877/nova/virt/libvirt/vif.py#L602-L624
16:50:17 mnaser https://github.com/openstack/nova/blob/f02b3800051234ecc14f3117d5987b1a8ef75877/nova/virt/libvirt/vif.py#L566-L598
16:50:44 cfriesen sean-k-mooney: possible problem with one of your commits: ^
16:50:47 mnaser https://github.com/openstack/nova/blob/9ae0cb67fabe09d6114680602f13b24da9dc88e2/nova/virt/libvirt/migration.py#L90
16:50:52 mnaser and that's how it breaks
16:51:28 mnaser we upgrade the vif config so if you're trying to migrate, host_mtu will be set in the destination which results in live migration failing
16:51:34 mdbooth_390 mnaser: So the guest was created before mtu was added, then nova was upgraded on the source, then we added mtu before lm?
16:51:44 mnaser mdbooth_390: yes
16:52:23 mnaser guest was long living, nova upgraded to rocky, live migration attempted, mtu is added before lm starts, lm fails at destination because host_mtu affects some internal kvm stuff that are beyond me :)
16:52:29 mdbooth_390 mnaser: Still sounds like a libvirt bug to me that this changes addressing, no?
16:52:44 cfriesen mdbooth_390: no, we're feeding the dest xml additional parameters that weren't there before
16:52:46 mnaser HOWEVER this also confirms the theory that instances started AFTER rocky would not be affected by this issue, i see it here
16:53:12 mnaser mdbooth_390: the libvirt bug there did the same behaviour we just started doing in nova, it started feeding host_mtu all the time and broke live migrations, so they reverted it
16:53:18 cfriesen in a live migration you need to create the dest guest with xml that is compatible with the source
16:53:20 mnaser we're doing the same mistake.. just one layer above :)
16:55:36 mdbooth_390 mnaser: I feel like I was at best a catalyst in that discussion. A figurative rubber duck :)
16:55:39 cfriesen I think we'll need to special-case this in the virt driver to ensure we don't add/remove the MTU setting over a live migration
16:56:17 cfriesen and we get to make fun of sean-k-mooney and mriedem for missing this scenario
16:56:21 mnaser well i needed to know about the nova-touching-xml part and then it made more sense :)
16:56:22 mnaser okay so
16:56:28 mnaser i'll file a bug
16:56:40 mnaser and i can hack on this because it's blocking a huge upgrade right now..
16:57:23 mnaser if anyone has suggestions.. maybe try to see if there's no mtu set and it's an lm, not set it ?
16:57:44 openstackgerrit Matt Riedemann proposed openstack/nova master: No longer call _normalize_inventory_from_cn_obj from upt flow https://review.openstack.org/613991
16:58:23 cfriesen I don't think we store the nova service version at the time an instance moved to a given compute node, so I think you might have to look at that instance xml before updating the vif-related xml stuff and ensure the mtu part stays the same
16:58:40 mriedem what did i miss? mixed version hypervisors with host mtu stuff on one of the 100 different vif type configurations in neutron? oops.
16:59:18 mdbooth_390 mriedem: Yeah, come on. It's so obvious!
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 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: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: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 mnaser https://bugs.launchpad.net/nova/+bug/1800511
17:39:36 openstack Launchpad bug 1800511 in OpenStack Compute (nova) "VMs started before Rocky upgrade cannot be live migrated" [Undecided,New]
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?

Earlier   Later