| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-05-21 | |||
| 15:46:31 | cfriesen | kashyap`: makes it hard to live migrate | |
| 15:47:01 | kashyap` | cfriesen: Not quite -- 'host-model' explicitly makes sure that you _can_ live-migrate across disparate hosts. That's one of the reasons why Nova defaults to it | |
| 15:47:03 | sean-k-mooney | cfriesen: yes that is true but its kind of a know issue | |
| 15:47:27 | cfriesen | sean-k-mooney: yeah, but the whole point of the CPU model feature was to fix that known issue. :) | |
| 15:48:03 | sean-k-mooney | kashyap`: what he is raising is if you have 10 skylake servers and 90 ivybridge servers then if you land on the skylake ones you will only be able to migrate to the skylake ones | |
| 15:48:21 | cfriesen | as it stands, I think you'd need to set up host aggregates for the different hardware generations, or something like that. | |
| 15:48:30 | mriedem | fwiw i've had a non-RH core nom in mind for stable | |
| 15:48:35 | sean-k-mooney | so you would set cpu_mode=custom cpu_mode=ivybridge to mitigate against that | |
| 15:48:46 | mdbooth | mriedem: Do say! | |
| 15:48:51 | mriedem | ill never tell! | |
| 15:49:06 | mriedem | i can start that conversation again with the nova stable maint team | |
| 16:01:05 | sean-k-mooney | cfriesen: that is one apprch yes all though if i ever get arround to it i really want to play with the idea of a boaring host weigher that use traits as a proxy for how intersting a host is and selects the one with the least traits | |
| 16:01:15 | sean-k-mooney | e.g. the most boaring hosts | |
| 16:01:44 | cfriesen | boaring? like wild pigs? :) | |
| 16:01:46 | sean-k-mooney | which woudl mean if you did not ask for a trait specificaly the weigher would prefer selecting the oldest host | |
| 16:01:56 | cfriesen | sean-k-mooney: yeah, that's not a bad idea | |
| 16:02:09 | cfriesen | me too | |
| 16:02:31 | sean-k-mooney | yes its a weigher that digs holes :) | |
| 16:03:19 | cfriesen | sean-k-mooney: you'd still have the problem though that if you happened to land on a "newer" node due to resource pressure you'd be stuck on that hardware for live migration | |
| 16:04:05 | cfriesen | it just seems unnecessarily restrictive to say that all guests on a node must have the same virtual CPU | |
| 16:04:05 | sean-k-mooney | ya that is true but i acutlly want it for other reasons like avoiding my host that have gpus when you did not ask for them that sort of thing | |
| 16:04:48 | sean-k-mooney | well that is what nova has always done to this point but ya im aware of the new spec too | |
| 16:05:27 | sean-k-mooney | personally i just use host-passthough and say screw it ill use host aggrates to seperate things if i need too | |
| 16:06:11 | sean-k-mooney | actully i used ot use AZs at intel for that and ould have one AZ per generation but that was just so i could select the generation simplely when lanuching a vm | |
| 16:06:13 | mtreinish | mriedem: do I need to review something? | |
| 16:06:37 | mdbooth | mtreinish: https://review.opendev.org/#/c/659317/ please? | |
| 16:15:29 | sean-k-mooney | efried: thanks for reviewing the video models stuff ill adress your commets after dinner | |
| 16:20:12 | mriedem | kashyap: https://bugs.launchpad.net/nova/+bug/1829896 for that guest not stopping thing, i checked the libvirtd logs and the power off requests all return with 0 | |
| 16:20:13 | openstack | Launchpad bug 1829896 in OpenStack Compute (nova) "libvirt: "Instance failed to shutdown in 60 seconds." in the gate" [Undecided,New] | |
| 16:20:28 | mriedem | not urgent but is concerning | |
| 16:22:03 | sean-k-mooney | i wonder if the qemu monitor process had hung? | |
| 16:22:21 | sean-k-mooney | i would expect a non 0 return in that case but i dont know if that is the case | |
| 16:41:07 | ganso | mriedem: hi Matt! Just a heads up in case you haven't seen it and have some free time to look at: https://review.opendev.org/#/c/659338 Thanks in advance! =) | |
| 16:41:38 | kashyap | sean-k-mooney: Yeah, I'd expect this to return non-zero here, in of a hung QEMU mithe case of hung QEMU: 2019-05-16 19:47:19.996+0000: 10502: debug : virThreadJobClear:121 : Thread 10502 (virNetServerHandleJob) finished job remoteDispatchDomainShutdown with ret=0 | |
| 16:46:52 | kashyap | mriedem: It seems to fail with the "MigrationsAdmin" test (but your bug descriptino says: 'snapshot'), whose purpose is "Add os-migration tests for Nova v2 API" (added in 2014). | |
| 16:48:20 | mriedem | kashyap: if you look at the logstash results it's on pretty much everything | |
| 16:48:56 | mriedem | migrate_disk_and_power_off in a cold migration / resize will also power off the guest | |
| 16:49:11 | mriedem | which is where i noticed it while debugging a cold migration test today | |
| 16:52:25 | kashyap | Hmm. So it is the actual guest | |
| 16:53:16 | kashyap | Damned if I could put a finger on why. Let me ask the libvirt folks, if they can think of anything | |
| 16:53:45 | kashyap | Wish I could reproduce the issue. Damn sure it won't if I try locally | |
| 16:54:47 | kashyap | < aw> kashyap: is it the VM or the guest OS? I've had fedora fail to shutdown in a timely way for all sorts of reasons, some that I still haven't figured out | |
| 16:54:47 | kashyap | One dev asks: | |
| 16:55:14 | mriedem | don't know, but these test vms in the gate runs are just cirros images | |
| 16:55:19 | kashyap | (I've told it's CirrOS; and it's the VM, aka. QEMU process) | |
| 17:01:49 | openstackgerrit | Elod Illes proposed openstack/nova stable/ocata: Fix incompatible version handling in BuildRequest https://review.opendev.org/660456 | |
| 17:02:00 | kashyap | Can anyone please remind, where is the guest boot log from the serial console here: http://logs.openstack.org/56/656656/12/check/nova-multi-cell/6a403ba/compute1/logs/ | |
| 17:09:07 | sean-k-mooney | i think i just spent an hour debuging a unit test failure because teh unit tests in osc are wrong an i coppied them to create my own... | |
| 17:15:21 | sean-k-mooney | nope i am wrong it was 2 hours but now they pass | |
| 17:30:31 | sean-k-mooney | aspiers: mriedem i just updated https://review.opendev.org/#/c/643578/ if there are any other changes you would like let me know with a comment :) | |
| 17:30:44 | aspiers | sean-k-mooney: ok will take a look thanks! | |
| 17:53:28 | aspiers | efried / anyone got any thoughts on whether it still makes sense to use nova.virt.libvirt.utils.file_open for mocking file opens? | |
| 17:53:42 | openstackgerrit | Merged openstack/nova master: Add detection of SEV support from QEMU/AMD-SP/libvirt on AMD hosts https://review.opendev.org/633855 | |
| 17:53:50 | openstackgerrit | Merged openstack/nova master: Extract provider tree functional tests into new file. https://review.opendev.org/660157 | |
| 17:54:02 | aspiers | \o/ first real SEV patch lands in nova! | |
| 17:55:15 | aspiers | file_open() was added in 2011 by Soren with the original framework for testing the libvirt driver | |
| 17:55:57 | aspiers | https://opendev.org/openstack/nova/commit/bb622e6d7c921894fd0e7697a0003630989d4f35 | |
| 17:56:25 | aspiers | he wrote "I hope eventually to make it similar to fakelibvirt in style (e.g. keep track of files created and deleted and attempts to open a file that it doesn't know about, you'll get proper exceptions with proper errnos set and whatnot)." | |
| 17:57:06 | aspiers | meanwhile, a ton of other tests just use mock.mock_open() | |
| 17:57:21 | aspiers | sean-k-mooney: any thoughts? | |
| 18:00:16 | efried | aspiers: The reason it exists is so that fake_libvirt_utils can mock it | |
| 18:00:24 | efried | and fake_libvirt_utils is dying in a fire. | |
| 18:00:29 | efried | https://review.opendev.org/#/c/642558/8 | |
| 18:00:48 | efried | IMO there's no reason for it to exist; we should just use mock_open | |
| 18:00:49 | aspiers | efried: my point was that a whole bunch of stuff needs to (and does) mock open, not just that | |
| 18:00:57 | aspiers | and it seems we have two different ways of doing it right now | |
| 18:01:01 | efried | yup | |
| 18:01:18 | efried | If you want to do the cleanup to kill file_open, I would support that. | |
| 18:01:42 | efried | meanwhile, definitely don't do anything that relies of fake_libvirt_utils.file_open | |
| 18:01:53 | aspiers | file_open seems much less popular right now than mock_open | |
| 18:01:55 | efried | and save yourself having more cleanup by avoiding mocking file_open - just mock open. | |
| 18:02:00 | efried | yeah. | |
| 18:02:15 | efried | Nice intention, but clearly nobody in the last eight years has cared enough to make it go. | |
| 18:02:23 | aspiers | having said that, Soren's original intention does make sense ... right | |
| 18:02:50 | efried | Just be wary of side effects. I had a situation recently where mocking open at the test method level was too broad and effed up, like, a dynamic import or something. | |
| 18:03:23 | aspiers | but file_open is actually also defined in nova/virt/libvirt/utils.py | |
| 18:03:40 | efried | if f == 'thing_i_care_about': | |
| 18:03:40 | efried | def fake_open(f, ...): | |
| 18:03:40 | efried | orig_open = builtins.open | |
| 18:03:40 | efried | so I had to do the | |
| 18:03:41 | efried | return orig_open(f, ...) | |
| 18:03:41 | efried | else: | |
| 18:03:41 | efried | do_the_mock_thing | |
| 18:03:42 | efried | thing. | |
| 18:04:18 | aspiers | isn't that what mock_open is supposed to help avoid? | |
| 18:04:22 | efried | yes, it's defined in fake_libvirt_utils because it was defined in libvirt.utils and apparently whoever set that up didn't know how to mock (or maybe mock wasn't properly powerful back then, dunno). | |
| 18:05:03 | efried | aforementioned series is getting rid of fake_libvirt_utils and just mocking the utils directly. | |
| 18:05:14 | aspiers | ah | |
| 18:05:24 | aspiers | makes sense, since a bunch of stuff is doing @mock.patch('nova.virt.libvirt.utils.file_open', ... | |
| 18:05:40 | aspiers | I'll stare at that review until I understand it | |
| 18:06:38 | efried | aspiers: re "isn't that what mock_open is supposed to help avoid?" -- not that I know of. Is there a way to make the mock automatically trigger conditionally based on the inputs? | |
| 18:07:07 | aspiers | I *think* there is, IIRC | |
| 18:07:17 | aspiers | I may have even done it recently, but would have to check | |
| 18:07:48 | efried | I thought mock_open was just good for abstracting the context manager awkwardness and providing an easy way to specify "output". | |
| 18:08:08 | efried | I don't see any conditionally-use-the-real-open mentioned in https://docs.python.org/3.3/library/unittest.mock.html#mock-open | |
| 18:10:03 | aspiers | Yeah I'm probably thinking of something else | |
| 18:11:02 | aspiers | mock is very powerful, but I find the API pretty damn confusing | |
| 18:11:15 | aspiers | I don't think it's the best designed API in the world | |
| 18:11:33 | aspiers | rspec is quite a bit easier to use | |
| 18:13:48 | aspiers | efried: maybe I was just thinking of side_effect, which would presumably at least avoid the need for orig_open | |