Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2019-04-09
15:02:05 mriedem i should probably re-watch your summit talk that is related to this
15:02:22 dansmith kashyap: okay, I
15:02:29 mordred mriedem: so I think I'd vote, in this case, for flat, mostly because of the existence of the locked key already
15:02:33 dansmith have only skimmed, but.. probably need to be more clear about that
15:02:34 kashyap mriedem: It is not directly a clean one-to-one mapping of: "if you click on this button, elephants will dance on your screen"
15:03:03 kashyap mriedem: The operator doesn't have to do anything much here
15:03:41 mriedem is there a very obvious case of "this is an issue we have today which is a limitation due to using the older apis which would be solved by using the new"
15:03:59 mriedem like an actual example
15:04:05 kashyap Yes, let me link to the code
15:04:26 mriedem mordred: ok, if you can take a sec to comment on the spec that would go a long way i think
15:04:37 mordred sure!
15:05:12 kashyap mriedem: libvirt upstream added the two APIs (partly) based on the need that came out of Nova, noted here: https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L3947,#L3958
15:05:30 kashyap (That comment should be removed, now that the libvirt RFE I filed is fixed.)
15:05:50 openstackgerrit sean mooney proposed openstack/nova-specs master: Libvirt: add vPMU spec for train https://review.openstack.org/651269
15:06:51 mordred mriedem: done
15:07:01 kashyap mriedem: A practical example is: the existing libvirt APIs can't answer this: Is this combination of 'IvyBridge' CPU model with 'pcid' and 'pdpe1gb' supported by the KVM+QEMU+libvirt on the host?
15:07:33 sean-k-mooney kashyap: isnt that a defct in libvirt
15:08:07 kashyap sean-k-mooney: Yes, it is. That's why I asked libvirt upstream to provide the two new RFEs: https://bugzilla.redhat.com/show_bug.cgi?id=1559832
15:08:07 openstack bugzilla.redhat.com bug 1559832 in libvirt "[RFE] Fine-grained API to validate if a given CPU model and flags are supported by QEMU / KVM" [Unspecified,Closed: nextrelease] - Assigned to jdenemar
15:08:11 sean-k-mooney it could tell if pdpe1gb was supporte by looking the the host cpu flags
15:08:21 kashyap (To solve the problem for Nova.)
15:08:27 mriedem kashyap: so today operators can just populate cpu_model_extra_flags with ['pcid', 'pdpe1gb'] and we'll blindly throw those into the guest cpu config, which may or may not cause the guest to fail to launch?
15:08:48 sean-k-mooney kashyap: yes but this is somthing that qemu should also be able to do today but it does not provde a cli to ask to question
15:08:50 kashyap sean-k-mooney: I think we're hand-waving; if you see the upstream patch `diff` for libvirt that introduced the APIs, you'll see what is involved
15:08:59 kashyap And also please see the spec I posted. Which compares the two APIs
15:09:08 kashyap mriedem: Yep, exactly
15:09:39 kashyap sean-k-mooney: Here libvirt is taking advantage of the enormous work done in QEMU. (Look up: `query-cpu-model-expansion`)
15:09:48 sean-k-mooney mriedem: yes that why we originally had a whitelist of jsut pcid
15:09:59 kashyap sean-k-mooney: See this blog post from the QEMU's CPU modelling infra maintainer: https://habkost.net/posts/2017/03/qemu-cpu-model-probing-story.html
15:10:08 mriedem ok, so that would be good information for the spec, i.e. today operators have to be careful about how they configure nova-compute for the libvirt driver with regard to cpu_model and cpu_model_extra_flags because the wrong combination can lead to failures to launch a guest - with this change the libvirt driver will automatically figure that out so the operator can set it and forget it
15:10:39 kashyap mriedem: Yep, whatever it takes to make it clear. It's also the "curse of having too much context". I hesitate to add needless detail
15:10:46 mriedem mordred: thanks
15:10:50 sean-k-mooney i dont think we should forget it
15:11:15 sean-k-mooney if you enable somting in the config that is invalid i would prefer if the nova compute agent failed to start
15:11:17 kashyap (Yes, the whitelist was for stable branches)
15:11:32 sean-k-mooney kashyap: not just for stable branches
15:11:35 mriedem kashyap: i realize you're close to the libvirt / qemu details on this stuff, but consider your audience is not so as a nova spec reviewer i understand nova things and need to know how this translates to nova problems and how they are fixed by this spec
15:12:01 kashyap mriedem: I go to painful lengths to be aware of that.
15:12:06 sean-k-mooney kashyap: i reasied the problem of doing the validation on the reviews for you extra flags patches
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 kashyap mriedem: Noted, I should have also linked the relevant Nova code, from which it came from.
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: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 dakshina-ilangov: A couple of things about https://review.openstack.org/#/c/651130/ just procedurally:
15:46:58 efried 1) Please wrap the body of the commit message at 72c
15:46:58 efried 2) You can build locally by running `tox -e docs` to resolve formatting issues
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 'nova-cells' service https://review.openstack.org/651290
16:15:54 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove '/os-cells' REST APIs https://review.openstack.org/651291
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:55 openstackgerrit Stephen Finucane proposed openstack/nova master: Stop handling cells v1 in '/os-servers' API https://review.openstack.org/651293
16:15:56 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove 'nova-manage cell' commands https://review.openstack.org/651294
16:15:56 openstackgerrit Stephen Finucane proposed openstack/nova master: Stop handling cells v1 for console authentication https://review.openstack.org/651295
16:15:57 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove old-style cell v1 instance listing https://review.openstack.org/651296
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:58 openstackgerrit Stephen Finucane proposed openstack/nova master: Remove 'instance_fault_create_at_top' https://review.openstack.org/651298

Earlier   Later