| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-09 | |||
| 15:12:30 | kashyap | mriedem: If you closely read what I write, I make a lot of effort to write with "no assumptions", and with a full-assed beginning, middle and an end :-) | |
| 15:12:53 | kashyap | sean-k-mooney: Well, I remember the change. You're forgetting something: | |
| 15:13:04 | kashyap | sean-k-mooney: Until Rocky we white-listed; after that we lifted it. | |
| 15:13:28 | sean-k-mooney | kashyap: yes and i pushed back on lifting it until we have validation of the extra flags | |
| 15:13:36 | mriedem | kashyap: the writing isn't the problem, i understand the words, they just don't have context for me | |
| 15:13:49 | mriedem | anyway if others are cool with approving this fine | |
| 15:13:51 | mriedem | i didn't -1 it | |
| 15:13:53 | sean-k-mooney | i was over ruled but i personally still thing we should not have lifted the whitelist untill we have a way to validate it | |
| 15:13:53 | kashyap | mriedem: Noted, I should have also linked the relevant Nova code, from which it came from. | |
| 15:13:59 | kashyap | mriedem: Yeah, noticed it. Appreciate the restraint :-) | |
| 15:14:15 | kashyap | mriedem: That spec (and the libvirt work) is derived directly from the problem Nova was facing, BTW | |
| 15:14:26 | kashyap | (Anyway, you see that now. I shold've made it clearer) | |
| 15:15:19 | kashyap | sean-k-mooney: I think there we disagree. Lifting the restrictions then made sense, and now it does. | |
| 15:16:17 | sean-k-mooney | we only lifted the restition because of all the specultive execution vulnerablitys and the fact we tought there would need to be a lot of extra cpus flags in the coming months | |
| 15:16:32 | kashyap | sean-k-mooney: Recall the solid reason *why* we allowed free-form: at that time we had no damn idea what else CPU flags Intel et al will introduce | |
| 15:16:54 | dansmith | sean-k-mooney: are you talking about the cpu flags thing that we added for spectre | |
| 15:16:55 | dansmith | ? | |
| 15:16:58 | kashyap | Yep | |
| 15:17:08 | dansmith | sean-k-mooney: the whitelist was *only* for stable, never intended to have that in master going forward | |
| 15:17:28 | kashyap | Exactly. I think sean-k-mooney got things mixed up. | |
| 15:17:39 | sean-k-mooney | no | |
| 15:17:49 | sean-k-mooney | that was the desicion we came to in the end | |
| 15:18:07 | dansmith | um, what? | |
| 15:18:10 | sean-k-mooney | but i did not think we should add the feature at all without validation logic | |
| 15:18:40 | sean-k-mooney | i was ok with not having the validation logic if we had a whitelist | |
| 15:18:58 | sean-k-mooney | and i was ok with removing the whitelist due to the security concerns | |
| 15:19:09 | dansmith | tbh, I don't recall you even being involved :) | |
| 15:19:12 | sean-k-mooney | but i still think we should have the validation | |
| 15:19:33 | sean-k-mooney | i was at intel a the time and was not allowed to talk publically about it much | |
| 15:20:25 | sean-k-mooney | so im not sure how much of my comment were recored upstream and how much i said to indivigual people | |
| 15:20:41 | mriedem | efried: nit and question in https://review.openstack.org/#/c/648800/6 | |
| 15:21:21 | dansmith | well, anyway, the plan was to make it free-form going forward, and the compromise for stable was to add the whitelist instead of a different-for-stable config flag | |
| 15:22:02 | sean-k-mooney | ya and kashyap's spec https://review.openstack.org/#/c/645814/4/specs/train/approved/cpu-selection-with-hypervisor-consideration.rst add the validation i was originally hoping for so that makes me happy :) | |
| 15:22:13 | dansmith | the only reason for the whitelist was to avoid having a different config flag for stable than master | |
| 15:22:18 | kashyap | Yes, what dansmith said. | |
| 15:23:18 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Tools & docs for backlog & abandoned spec process https://review.openstack.org/648800 | |
| 15:23:33 | efried | mriedem: answered ^ | |
| 15:23:44 | efried | mriedem: stephenfin helped out with the tox thing, so it must be right. | |
| 15:24:08 | efried | gibi: Thank you for your participation! | |
| 15:31:29 | sean-k-mooney | kashyap: ok but my request for validating the feature is there in https://review.openstack.org/#/c/534384/5/nova/conf/libvirt.py@538 anyway im almost through your spec which seams to provide it anyway. | |
| 15:31:48 | mriedem | efried: ok final question https://review.openstack.org/#/c/648800/7 | |
| 15:33:15 | kashyap | sean-k-mooney: Nod; I recall seeing the comment | |
| 15:41:04 | efried | mriedem: responded | |
| 15:41:32 | efried | mriedem: Believe me, I would have preferred to put the glob in this change instead of individually in the four on top :( | |
| 15:42:02 | efried | I think gerrit will at least be smart enough not to generate a merge conflict on the three remaining when the first one merges... | |
| 15:46:58 | efried | 2) You can build locally by running `tox -e docs` to resolve formatting issues | |
| 15:46:58 | efried | 1) Please wrap the body of the commit message at 72c | |
| 15:46:58 | efried | dakshina-ilangov: A couple of things about https://review.openstack.org/#/c/651130/ just procedurally: | |
| 15:56:39 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Add 'flavor-extra-spec-image-property-validation-extended' spec https://review.openstack.org/638734 | |
| 16:08:32 | openstackgerrit | François Palin proposed openstack/nova master: nova diagnostics command is not working with all interfaces https://review.openstack.org/648123 | |
| 16:15:48 | stephenfin | kaboom | |
| 16:15:53 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells v1 jobs https://review.openstack.org/651289 | |
| 16:15:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove '/os-cells' REST APIs https://review.openstack.org/651291 | |
| 16:15:54 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'nova-cells' service https://review.openstack.org/651290 | |
| 16:15:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 in '/os-servers' API https://review.openstack.org/651293 | |
| 16:15:55 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 in '/os-hypervisors' API https://review.openstack.org/651292 | |
| 16:15:56 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for console authentication https://review.openstack.org/651295 | |
| 16:15:56 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'nova-manage cell' commands https://review.openstack.org/651294 | |
| 16:15:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'bdm_(update_or_create|destroy)_at_top' https://review.openstack.org/651297 | |
| 16:15:57 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove old-style cell v1 instance listing https://review.openstack.org/651296 | |
| 16:15:58 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_info_cache_update_at_top' https://review.openstack.org/651299 | |
| 16:15:58 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_fault_create_at_top' https://review.openstack.org/651298 | |
| 16:15:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_update_at_top', 'instance_destroy_at_top' https://review.openstack.org/651301 | |
| 16:15:59 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'get_keypair_at_top' https://review.openstack.org/651300 | |
| 16:16:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling 'update_cells' on 'BandwidthUsage.create' https://review.openstack.org/651303 | |
| 16:16:00 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove 'instance_update_from_api' https://review.openstack.org/651302 | |
| 16:16:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling 'cell_name' field on Instance.save() https://review.openstack.org/651305 | |
| 16:16:01 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for instance naming https://review.openstack.org/651304 | |
| 16:16:02 | openstackgerrit | Stephen Finucane proposed openstack/nova master: Remove cells code https://review.openstack.org/651306 | |
| 16:16:16 | sean-k-mooney | cells v1 removal | |
| 16:16:27 | stephenfin | I _think_ all but the last two or three should be passing unit test, functional tests and pep8 | |
| 16:16:34 | stephenfin | sean-k-mooney: yuuup | |
| 16:16:47 | sean-k-mooney | well the gate is going to be busy for a while | |
| 16:17:17 | stephenfin | First time in days that my laptop hasn't sounded like a rocket | |
| 16:17:32 | mriedem | stephenfin: are you going to throw that blueprint in the meeting agenda for thursday since it's a specless blueprint? | |
| 16:17:43 | stephenfin | mriedem: Good call. Sure | |
| 16:17:55 | gmann | mriedem: I would like to keep only first 3 fixes in this spec, hoping that will not be big for review/doc ? - https://review.openstack.org/#/c/603969/9 | |
| 16:18:11 | mriedem | because i may or may not have several new edge use cases and features for cells v1 and will block it's removal | |
| 16:18:48 | gmann | mriedem: success code change is something need more audit and can be done later ? | |
| 16:19:17 | mriedem | gmann: those first 3 are probably ok and straight-forward enough | |
| 16:19:41 | gmann | ok. let me update the spec. | |
| 16:25:25 | openstackgerrit | Sylvain Bauza proposed openstack/nova-specs master: Proposes NUMA affinity for vGPUs https://review.openstack.org/650963 | |
| 16:25:46 | bauzas | stephenfin: thanks for reviewing https://review.openstack.org/#/c/650963/, just wrote PS2 | |
| 16:33:46 | tssurya | mordred, mriedem, dansmith: after the back and forths on changing the type of the "locked" response key since all of you are for the seperate key style and no change on type I will just update the spec then to that approach. | |
| 16:34:02 | dansmith | tssurya: thanks | |
| 16:34:06 | mordred | ++ | |
| 16:34:06 | tssurya | separate* | |
| 16:34:18 | dansmith | I was shocked to have mordred come in and agree with me | |
| 16:34:40 | dansmith | he might not have realized what he was doing though | |
| 16:39:32 | dansmith | stephenfin: melwitt bauzas mriedem: I didn't see any discussion about how stephenfin only created five placeholders for the main db this time, but I do think that's probably more than enough and would be good for the future | |
| 16:39:59 | stephenfin | dansmith: I _think_ I called it out in the commit message or a comment, so I figured everyone was ok with that | |
| 16:39:59 | bauzas | yeah, I just feel 5 is enough given the history | |
| 16:40:14 | stephenfin | (did it because you mentioned that we'd never used them anyway) | |
| 16:40:14 | dansmith | stephenfin: I didn't see it anywhere | |
| 16:40:19 | mordred | dansmith: wait. I change my mind. whatever the opposite things is please | |
| 16:40:30 | dansmith | mordred: :) | |
| 16:41:06 | stephenfin | dansmith: Yeah, it wasn't as obvious as it should have been but it's there alright https://review.openstack.org/#/c/650964/1/nova/tests/unit/db/test_migrations.py@183 | |
| 16:41:24 | dansmith | ack, okay | |
| 16:44:11 | mriedem | he put it in a comment | |
| 16:44:17 | mriedem | oh yeah i'm late | |