Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-07-19
15:25:08 artom That's why it's currently half-broken
15:25:09 alex_xu hah
15:25:25 sean-k-mooney what might happen is two vms get pinned to the same cores
15:25:36 sean-k-mooney but that could happen before.
15:25:51 artom It might actually break if it's pinned to CPUs that don't exist on the dest
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?

Earlier   Later