| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-19 | |||
| 15:26:14 | alex_xu | yea, so...we should ask the user to do that | |
| 15:26:14 | sean-k-mooney | if the cores do not exist or there is not enough hugepages libvirt fails the migrtion due to the qemu error | |
| 15:26:17 | sean-k-mooney | and it auto reverts | |
| 15:26:25 | alex_xu | s/should/shouldn't/ | |
| 15:26:28 | sean-k-mooney | that is the same behavior as today | |
| 15:27:10 | alex_xu | I remember we have a patch stop that LM api call by checking the instance has numa | |
| 15:27:19 | sean-k-mooney | yep we merged that | |
| 15:27:25 | artom | Yeah, it's now disabled by default | |
| 15:28:03 | sean-k-mooney | so to live migrarte form S->T they need to re enabeld it | |
| 15:28:21 | sean-k-mooney | although i think artom is removing that option in T? | |
| 15:28:27 | sean-k-mooney | or is that puhsed to U | |
| 15:28:31 | alex_xu | so...we will tell the user, now you can LM your instance again when you upgrade from S to T, but you instance may gone based on lucky :) | |
| 15:28:41 | artom | sean-k-mooney, it's being deprecated but not removed, because of the upgrade impact | |
| 15:28:56 | artom | We don't want to turn it on by default unless we're sure the whole cloud supports it | |
| 15:29:33 | sean-k-mooney | alex_xu: the important thing is we do not regres or change any live migration behavior with artom code | |
| 15:30:01 | alex_xu | sean-k-mooney: I agree with that...but doesn't mean we should encourge the user to do that | |
| 15:30:14 | alex_xu | special for danger thing | |
| 15:30:22 | sean-k-mooney | we are not encuragin people to do it | |
| 15:30:38 | sean-k-mooney | if you have numa instance you should cold migrate them or do an inplace upgrade | |
| 15:30:50 | sean-k-mooney | that is what we will be telling customers | |
| 15:31:11 | alex_xu | yes, so we needn't to support src to dest numa instance LM :) | |
| 15:31:18 | alex_xu | for s to t | |
| 15:31:41 | sean-k-mooney | alex_xu: orginally we didnt | |
| 15:32:17 | sean-k-mooney | but both i belive dansmith and mriderman wanted the spec changed to the current form | |
| 15:32:33 | artom | (Oh man, M. Riderman) | |
| 15:32:39 | artom | I am lolling right now | |
| 15:34:21 | sean-k-mooney | actully it look like dansmith didnt review that spec but i know we spoke to him about it | |
| 15:35:35 | sean-k-mooney | anyway for sriov live migration since it never worked before we have a hard check in the conductor | |
| 15:35:47 | sean-k-mooney | and fail if both nodes are not knew enough | |
| 15:36:13 | sean-k-mooney | since live migration wiht hugepages is a thing we support for ovs-dpdk we did not want to break it by blocking it | |
| 15:36:52 | openstackgerrit | Merged openstack/nova master: Bump the openstackdocstheme extension to 1.20 https://review.opendev.org/671694 | |
| 15:37:48 | alex_xu | I guess the only reason, the user know they have same type machine, and the instances won't break with pinning to same pcpu | |
| 15:37:54 | sean-k-mooney | alex_xu: uncontidionally blocking old to new migration would be a regression. | |
| 15:38:06 | alex_xu | ah.. | |
| 15:38:22 | sean-k-mooney | well if the instance is not pinned but just has hugepages we dont want to break that | |
| 15:38:33 | alex_xu | I see the reason now | |
| 15:38:37 | artom | alex_xu, yeah, I think that's explicitly called out somewhere: NUMA LM is only "guaranteed" to work if you're migrating to an "empty" identical machine, identically configured, | |
| 15:39:27 | alex_xu | got it | |
| 15:40:20 | sean-k-mooney | in the hugepage case if there are not enoguht free spawning the qemu instace fail on the dest and we abort the migration before we even pause the vm on the source | |
| 15:41:02 | sean-k-mooney | the same is true if the cores dont exist on the dest | |
| 15:41:22 | sean-k-mooney | so the only issue that is not caght by that is two vms being pinned to the same host | |
| 15:41:27 | sean-k-mooney | * core | |
| 15:41:48 | alex_xu | can we check the src and dst service version in the conductor? | |
| 15:42:14 | openstackgerrit | Artom Lifshitz proposed openstack/nova master: libvirt: move checking CONF.my_ip to init_host() https://review.opendev.org/671471 | |
| 15:42:18 | sean-k-mooney | we can but why would we | |
| 15:42:22 | alex_xu | emm...I guess we need another param for the rpc | |
| 15:42:36 | artom | alex_xu, that's what I had at some point, Dan talked me out of it | |
| 15:42:53 | sean-k-mooney | we need the data that is pass back from can live migrate on dest | |
| 15:42:59 | artom | dansmith, efried ^^ (the CONF.my_ip to init_host() thing) | |
| 15:43:01 | sean-k-mooney | to be able to generate the updated xml | |
| 15:43:04 | efried | sean-k-mooney: Is https://review.opendev.org/#/c/638680/ still waiting on https://review.opendev.org/#/c/670189/ ? | |
| 15:43:45 | sean-k-mooney | kind of. im not changeing the public interface of the function it calls | |
| 15:44:03 | sean-k-mooney | but i suspect that if i chekcout that patch locally it will still expode | |
| 15:44:23 | sean-k-mooney | i can check that quickly. | |
| 15:46:37 | efried | artom: would you mind adding a teeny unit test that asserts _check_my_ip is called during init_host? | |
| 15:48:53 | artom | efried, ack, makes sense | |
| 15:49:25 | efried | artom: I recognize that a unit test for init_host as a whole is missing, and would be an appropriate add, but unrelated... | |
| 15:49:58 | artom | efried, hrmm, yeah, went looking, didn't find anything | |
| 15:50:04 | efried | yeah | |
| 15:50:08 | artom | So, maybe as a separate fup patch on top? | |
| 15:50:25 | artom | The full unit test, I mean | |
| 15:50:35 | efried | sure, that would be great, though I would also just not notice if it never happened | |
| 15:50:44 | efried | There are lots of gaps in unit test in libvirt/test_driver | |
| 15:50:49 | efried | lots | |
| 15:51:28 | artom | My thinking it, given how much I'll have to mock to just assert this one thing being called, might as well do the full thing | |
| 15:51:40 | artom | *is | |
| 15:51:41 | sean-k-mooney | efried: its not failing because my kernel does not support sev. so https://review.opendev.org/#/c/638680/27/nova/virt/libvirt/host.py@1066 prevent the call to get_domain_capablities | |
| 15:52:20 | efried | artom: There are other tests that call init_host but don't mock a zillion things. I didn't look too deeply into how they're doing that, but... | |
| 15:53:03 | efried | probably because of FakeLibvirtDriver | |
| 15:53:24 | artom | Ah, indeed | |
| 15:53:31 | efried | naw, even the ones that use real LibvirtDriver with FakeVirtAPI | |
| 15:53:39 | efried | so, it should be possible to do it without too much mocking. | |
| 15:54:38 | sean-k-mooney | if i comment out the if i get this trace back | |
| 15:54:40 | sean-k-mooney | http://paste.openstack.org/show/754594/ | |
| 15:55:11 | efried | though that leads me to wonder how you're getting away with not mocking netifaces.interfaces() and .ifaddresses() | |
| 15:55:33 | artom | get_machines_ips gets mocked out entirely | |
| 15:55:34 | efried | artom: Is your test actually running netifaces.interfaces() on whatever system you're invoking it on?? | |
| 15:55:47 | efried | yeah, I mean all the other tests that invoke init_host() :) | |
| 15:56:00 | artom | Oh, hah | |
| 15:56:05 | artom | I guess they do? | |
| 15:56:22 | efried | that's... not great | |
| 15:56:39 | efried | unpredictable results on linux, sure failure on other platforms, nah? | |
| 15:56:48 | artom | It's supposed to be cross-platform | |
| 15:56:50 | artom | But yeah | |
| 15:56:54 | efried | Not that I would really expect test_driver to work on another platform anyway, but... | |
| 15:57:04 | sean-k-mooney | artom: it should not be actully calling it however | |
| 15:57:27 | efried | so perhaps you need to fixture out get_machine_ips (or netifaces.interfaces) at a higher level | |
| 15:57:41 | sean-k-mooney | you shoudl be able to set up a mock in the setup function once | |
| 15:57:52 | artom | Yep | |
| 15:57:54 | artom | ;_: | |
| 15:58:05 | artom | And here I was thinking I'd get to do a pure removal patch :( | |
| 15:58:21 | efried | sorry artom. Your karma ran over your dogma. | |
| 15:58:51 | artom | Is that like my kar running over my dog, just with my ma involved? | |
| 16:00:27 | efried | just so | |
| 16:03:08 | efried | sean-k-mooney: I may need some help, or you should bring in a SME, reviewing that get_dowain_capabilities patch. kashyap? | |
| 16:05:17 | sean-k-mooney | sure. i could harden it more then i did but that at least fixes the orignial issue | |
| 16:05:56 | sean-k-mooney | im still not catching the libvirt exection if we ligitetly misconfigur the config | |
| 16:07:00 | sean-k-mooney | but we wont explode any more with the defaults | |
| 16:07:52 | sean-k-mooney | efried: do you know if aspires is still working on openstack? | |
| 16:08:08 | sean-k-mooney | i have not seen them involded in the sev work lately | |