| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-29 | |||
| 16:48:35 | mnaser | oh | |
| 16:48:41 | mdbooth_390 | cfriesen: Look how it's generated | |
| 16:48:53 | mdbooth_390 | cfriesen: 'generated' | |
| 16:49:08 | mdbooth_390 | It's from the running guest | |
| 16:49:08 | mnaser | https://github.com/openstack/nova/blob/d87852ae6a1987b6faa3cb5851f9758b47ef4636/nova/virt/libvirt/driver.py#L7092-L7124 | |
| 16:49:09 | cfriesen | mnaser: that commit looks suspicious to me | |
| 16:49:10 | mnaser | yes | |
| 16:49:11 | mnaser | but | |
| 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 | |