| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-24 | |||
| 13:49:44 | efried | kashyap: Being merged, that first one looks great! | |
| 13:50:31 | kashyap | efried: Hehe, one more thing: because we're consolidating all the AMD stuff in amd.py, I am going to move 'SVM' trit from x86.py to amd.py (in a separate change). | |
| 13:50:49 | kashyap | (Didn't wanted to sneak it in part of this.) | |
| 13:52:23 | kashyap | alex_xu: efried: Also, a trait-per-CPU-feature is _correct_, isn't it? | |
| 13:53:44 | efried | kashyap: Yes, one trait per feature is correct. | |
| 13:53:44 | efried | Re: moving traits... I'm not sure what harm that could do, if any. You may need to provide a shim. edleafe jaypipes? | |
| 13:54:14 | efried | kashyap: tbc, you're not changing what the string would end up being; just the module path we take to get there, right? | |
| 13:54:38 | kashyap | efried: Yes, exactly. Just moving the 'SVM' as-is from one file to another | |
| 13:54:45 | kashyap | Not modifying the string itself | |
| 13:55:19 | efried | kashyap: I mean the string as it comes out the other end. Like if it's currently HW_CPU_FOO_SVM and you made it HW_CPU_FOO_AMD_SVM, that's bad. | |
| 13:55:36 | efried | i.e. that's a *new* trait (and you can't remove the old one, and it's now confusing which one consumers should use) | |
| 13:55:49 | kashyap | efried: Ah-ha...Hmm. | |
| 13:56:39 | efried | But if it's currently in hw/cpu/foo.py and called AMD_SVM, so it actually winds up being HW_CPU_FOO_AMD_SVM, and you move it to hw/cpu/foo/amd.py and name it SVM, the string itself wouldn't change. And that's the thing that *might* be okay. | |
| 13:56:41 | kashyap | I'll post the quick change, and would prefer to be educated there | |
| 13:57:14 | kashyap | efried: Ah-ha, I think you're right there | |
| 13:57:21 | kashyap | So, I see it being os_traits.HW_CPU_X86_SVM | |
| 13:57:22 | efried | kashyap: Sounds like you ought to propose it in its own patch :) | |
| 13:57:47 | kashyap | If I move SVM to amd.py, it can end up being: HW_CPU_AMD_SVM | |
| 13:57:49 | efried | kashyap: Okay, so by project charter, we can't make os_traits.HW_CPU_X86_SVM go away, ever. | |
| 13:58:09 | efried | HW_CPU_AMD_SVM will be a new trait. | |
| 13:58:21 | efried | If there's no such thing as SVM on X86, that's just too bad. | |
| 13:58:28 | efried | you can leave a comment on it that it's bogus | |
| 13:58:30 | efried | but you can't remove it. | |
| 13:59:21 | kashyap | efried: Yeah, indeed. SVM is AMD's equivalent of Intel's VMX (Virtual Machine Extensions) | |
| 13:59:57 | efried | Okay. So if you're just adding AMD_SVM, you can do it as part of your change if that's appropriate. Is it? Sounds like it might be unrelated. | |
| 13:59:58 | kashyap | efried: Okay, that makes sense -- add a comment that it's bogus; and add a new trait for SVM in amd.py | |
| 14:00:19 | efried | ++ | |
| 14:00:41 | kashyap | efried: It's slightly unrelated: but frankly, all these traits_ you are using hardware-based virt (Intel VMX; or AMD SVM) | |
| 14:00:57 | kashyap | Separate change coming soon | |
| 14:01:16 | efried | If your patch is called "add CPU traits related to spectre/meltdown" and it's not, then separate | |
| 14:02:19 | efried | If your patch is called "add a bunch of CPU traits that are missing" you'll have to justify why we need each one anyway. If it's a bunch of different reasons, all in one patch could be fine. But if it's 1) spectre/meltdown and 2) this other random thing that I saw while I was here -- separate patches :) | |
| 14:02:54 | kashyap | Yeah, noted. :-) | |
| 14:06:13 | edleafe | efried: (reading back) It's the full name of the trait that can't change. How it is constructed in the file structure of os-traits is irrelevant | |
| 14:06:24 | efried | edleafe: cool, gtk | |
| 14:13:14 | mriedem | efried: want to hit the easy gerrit cherry pick button to put this in stable/stein? https://review.opendev.org/#/c/654457/ | |
| 14:13:19 | openstackgerrit | Rodolfo Alonso Hernandez proposed openstack/os-vif master: Prevent "qbr" Linux Bridge from replying to ARP messages https://review.opendev.org/655332 | |
| 14:13:43 | efried | mriedem: I'm 42 minutes ahead of you. | |
| 14:13:51 | mriedem | ah didn't notice b/c the tb changed | |
| 14:13:57 | efried | tb? | |
| 14:14:02 | mriedem | topic branch | |
| 14:14:19 | mriedem | the gerrit ui appends an annoying branch specifier in the backports | |
| 14:14:21 | efried | oh. That's weird | |
| 14:14:36 | efried | I'll pay attention to that from now on and fix it. | |
| 14:24:14 | openstackgerrit | Eric Fried proposed openstack/nova master: Hacking N363: Don't use spec[_set]='string' https://review.opendev.org/650370 | |
| 14:24:14 | openstackgerrit | Eric Fried proposed openstack/nova master: Fix mock specs set to strings https://review.opendev.org/655455 | |
| 14:24:32 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Prepare _heal_allocations_for_instance for nested allocations https://review.opendev.org/637954 | |
| 14:24:33 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: nova-manage: heal port allocations https://review.opendev.org/637955 | |
| 14:24:33 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: pull out functions from _heal_allocations_for_instance https://review.opendev.org/655457 | |
| 14:24:34 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: reorder conditions in _heal_allocations_for_instance https://review.opendev.org/655458 | |
| 14:24:34 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: pull out put_allocation call from _heal_* https://review.opendev.org/655459 | |
| 14:24:46 | efried | melwitt: FYI since we talked about this a couple weeks ago: I fixed the Mock(spec='string') things ---^ and put the patch for the hacking rule on top of it | |
| 14:30:25 | efried | kashyap: You have a lot of support for https://review.opendev.org/#/c/645814/ -- if you add some words about when/how you're going to use the information (which I still don't know from outside conversations - fail spawn? fail to start n-cpu?) I'll fast approve if that's okay with sean-k-mooney | |
| 14:31:20 | efried | aspiers: I'm removing SEV from the PTG agenda, since it's approved \o/ | |
| 14:31:21 | sean-k-mooney | ya im fine with that. | |
| 14:31:28 | efried | thx | |
| 14:31:42 | kashyap | efried: Yeah, it's the top-2 item on my TODO, will add it by tomm, that okay? | |
| 14:31:49 | kashyap | If not for this traits yak, I would have done it today | |
| 14:31:57 | efried | kashyap: wfm | |
| 14:32:07 | efried | kashyap: IMO more important than traits yak | |
| 14:32:18 | efried | but of course I would say that because it eases my PTG schedule pain :P | |
| 14:32:29 | sean-k-mooney | ideally it would be nice to log the cpu basleing that was calulated + the flags form the cpu model and config and indicate what flag was incompatable. then kill the agent but we can discus that in the implementation | |
| 14:32:45 | kashyap | efried: Yesterday, I was about to add a quick note that: "Ack, will address this once I finish addressing the Secure Boot comments" | |
| 14:32:59 | kashyap | (The SB spec took a lot more time, too) | |
| 14:33:13 | sean-k-mooney | effectivly if nova can detect via libvirt that its incomatible we shoudl also tell the operator why so that they dont need to figure it out themselves | |
| 14:33:21 | kashyap | sean-k-mooney: Yeah. Let's hash it out when I actually get to implement it | |
| 14:33:27 | jaypipes | efried: ack, got it. thx! | |
| 14:33:56 | efried | jaypipes: Since parlayed into a draft agenda, see ML. | |
| 14:34:06 | efried | (which may be where you were coming from in the first place) | |
| 14:37:22 | kashyap | efried: On your question on that spec about 'where to fail' -- I'm thinking currently 'spawn'. Will add words to the spec | |
| 14:37:32 | efried | kashyap: Perfect, thanks. | |
| 14:37:44 | efried | I don't actually have a stake in what the answer is - I just want to see one :) | |
| 14:39:03 | efried | So it be like, "Before this wonderful feature, the instance might fail to come alive, but nova wouldn't know anything was wrong. Now we can fail the spawn operation, putting the instance in ERROR state, with a helpful log message indicating which bits are a problem." | |
| 14:44:12 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.opendev.org/555081 | |
| 14:46:48 | kashyap | efried: Hehe, noted | |
| 15:04:24 | melwitt | efried: ack | |
| 15:24:43 | mriedem | lyarwood: re getting the ceph job voting, we might consider disabling the ssh validation (which is on by default in tempest) to avoid spurious failures http://logs.openstack.org/06/641806/12/check/devstack-plugin-ceph-tempest/7fc39ec/testr_results.html.gz - ssh failing is something with networking breaking down, not really anything that should be a result of a ceph backend for storage | |
| 15:24:47 | mriedem | it'd also make the job fast | |
| 15:24:48 | mriedem | *faster | |
| 15:25:32 | mriedem | lyarwood: i also posted https://review.opendev.org/#/c/655452/ earlier today | |
| 15:30:20 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Standardize CPU resource tracking https://review.opendev.org/555081 | |
| 15:31:30 | lyarwood | mriedem: ack sounds and looks good, where's the best place to track devstack-plugin-ceph-tempest result trends btw? | |
| 15:31:57 | mriedem | here is a patch to disable the ssh validation https://review.opendev.org/655472 | |
| 15:32:05 | mriedem | there is a graph that is out of date, sec | |
| 15:32:17 | mriedem | http://grafana.openstack.org/d/-iKINcImz/ceph-failure-rate?orgId=1 | |
| 15:32:39 | mriedem | https://opendev.org/openstack/project-config/src/branch/master/grafana/ceph.yaml needs fixing to fix that graph | |
| 15:32:51 | mriedem | otherwise i think melwitt might be tracking ceph job failure rates somewhere | |
| 15:33:53 | lyarwood | kk, I can take a look at fixing the graph if there's not another way | |
| 15:34:24 | melwitt | I've been tracking via eyeball because I keep forgetting how to get the graph that shows the comparison with the tempest-full job | |
| 15:34:40 | mriedem | that's what that graph above is for, but it's broken | |
| 15:35:00 | melwitt | haha. I can also try to fix it | |
| 15:35:05 | mriedem | first it was broken by the ceph job renames, but now i think it's broken b/c of some zuulv3 thing, i had talked with corvus about it before but don't remember what needs fixing now | |
| 15:36:38 | mriedem | looking for one more core for this functional recreate test patch https://review.opendev.org/#/c/641521/ | |
| 15:37:02 | mriedem | then i can claim victory on merging 4/47 patches of my cross-cell series while it was in runways for 2 weeks | |
| 15:42:02 | stephenfin | mriedem: I thought my cells-v1 series was big... I won't get to it tonight but I can look at that tomorrow morning | |
| 15:43:07 | stephenfin | (assuming no one gets to it before me) | |
| 15:43:59 | mriedem | thanks | |
| 15:44:00 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Move legacy-grenade-dsvm-neutron-multinode-live-migration in-tree https://review.opendev.org/640207 | |
| 15:44:11 | efried | kashyap: I just did a thorough look at https://review.opendev.org/#/c/655193/ | |
| 15:44:11 | efried | I would like to get some os-traits-y people (edleafe jaypipes) to scrutinize, esp. to make a ruling on whether/how we should do the split I suggested. | |
| 15:44:32 | edleafe | efried: buried in meetings etc until the afternoon | |