| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-04-03 | |||
| 12:59:29 | alex_xu_ | bhagyashris: johnthetubaguy melwitt, good to have agreement on this first http://lists.openstack.org/pipermail/openstack-dev/2018-March/128914.html | |
| 12:59:46 | johnthetubaguy | kashyap: I got the impression host model had PCID missing put the CPU might still support it? | |
| 12:59:49 | kashyap | stephenfin: My rule to capitalize the sentence following a colon: if there is just _one_ sentence after colon, use lower case; otherwise upper case. | |
| 13:00:19 | johnthetubaguy | kashyap: maybe we should add a TODO to raise once its not a backport? | |
| 13:00:21 | kashyap | stephenfin: Hope that make sense? | |
| 13:00:24 | kashyap | johnthetubaguy: Hi | |
| 13:01:00 | stephenfin | kashyap: Fine by me. That should have been preceded by 'nit'. Does seem it's purely stylistic too (which I didn't know) https://www.grammarly.com/blog/capitalization-after-colons/ | |
| 13:01:11 | kashyap | johnthetubaguy: No-no: With `host-model`, *if* you have the updated microcode, guest/host kernel, QEMU and libvirt, then libvirt will do the Right Thing. | |
| 13:01:34 | kashyap | stephenfin: No worries, I enjoy the English nit-picks. :-) | |
| 13:02:35 | johnthetubaguy | kashyap: ah, good. I got the wrong end of the stick about that bit then. | |
| 13:02:36 | kashyap | johnthetubaguy: After the backports are done -- we will remove the restriction on 'host-model' | |
| 13:02:56 | kashyap | johnthetubaguy: No worries; can you please add a follow-up comment on the review, so that we're on the same line, same word? :-) | |
| 13:02:59 | johnthetubaguy | kashyap: so I think you need to set the extra_flags to None when you log the warning? | |
| 13:03:08 | kashyap | johnthetubaguy: That's the other bit | |
| 13:03:12 | kashyap | johnthetubaguy: I think you're right there | |
| 13:03:31 | johnthetubaguy | kashyap: no worries, I think we are in sync then, I added a follow up comment | |
| 13:03:57 | johnthetubaguy | kashyap: sorry this is dragging on, tricky to get the right balance and make it backportable, it turns out | |
| 13:04:28 | kashyap | johnthetubaguy: I first actually raised an exception, and then Matt and Dan came along saying: maybe a warning is less "heavy-handed" | |
| 13:04:31 | stephenfin | johnthetubaguy: Ack on https://review.openstack.org/#/c/552242/ | |
| 13:04:43 | stephenfin | sean-k-mooney[m]: Looking | |
| 13:04:44 | johnthetubaguy | kashyap: yeah, for the backport its a good idea | |
| 13:04:47 | kashyap | johnthetubaguy: Yeah, would like to get this rid of my plate; getting a bit too much of it. But yes, it's a tricky change with right balance | |
| 13:05:03 | kashyap | s/with right balance/that needs the right balance/ | |
| 13:05:20 | johnthetubaguy | stephenfin: thanks, wanted to make sure I didn't just misread it an wait for mikal to wake up after I sign off for the day. | |
| 13:06:50 | kashyap | johnthetubaguy: On your other question about 'host-passthrough': Yes, it is invalid. As libvirt wouldn't mangle anything, and if the CPU has it, libvirt will pass it right through to the guest. | |
| 13:06:51 | jaypipes | efried: I'm happy to do a hangout about the consumer generation today if you need to. | |
| 13:07:08 | efried | jaypipes: Is there email I need to catch up on first? | |
| 13:07:09 | kashyap | johnthetubaguy: To summarize, I'll quickly add: the 'extra_flags = None', before the warning. | |
| 13:07:37 | jaypipes | efried: no, just saying I left things yesterday afternoon in a bit of a "too tired to talk about this right now" state. | |
| 13:08:09 | johnthetubaguy | kashyap: yeah, +1 | |
| 13:08:21 | efried | jaypipes: Okay. The open issue is whether we should change the responses to PUT /alloc/{c} and POST /alloc from 204 No Content to 200 with payload. | |
| 13:08:26 | kashyap | johnthetubaguy: Thanks for the eagle eyes. | |
| 13:10:41 | jaypipes | efried: ok, I will answer that on the spec if that's what you prefer. | |
| 13:11:02 | efried | jaypipes: wfm, unless it needs back-and-forth. | |
| 13:11:12 | efried | cdent: Wouldn't mind an opinion from you as well. | |
| 13:11:35 | efried | cdent: oh, looks like you rendered one. Reading... | |
| 13:12:01 | kashyap | johnthetubaguy: Hmm, isn't it a bit "dirty" that we're quietly setting it to 'extra_flags' to None? Or is the onus is on the Operator to read the docs to see exactly what is valid? | |
| 13:12:33 | johnthetubaguy | kashyap: its not great, but I think we just change this to a raise in the follow on patch that isn't backported | |
| 13:13:25 | johnthetubaguy | kashyap: so it comes from our usual backport rules that mean no new config options or features, etc | |
| 13:17:20 | kashyap | johnthetubaguy: Raise an "error" you mean? | |
| 13:17:26 | kashyap | johnthetubaguy: (In a follow-up.) | |
| 13:17:43 | kashyap | Okay, noted. | |
| 13:30:13 | kashyap | stephenfin: Hmm, what is the alternative then, just don't indent it?: https://review.openstack.org/#/c/534384/22/nova/conf/libvirt.py@539 | |
| 13:30:31 | stephenfin | kashyap: Correct | |
| 13:31:07 | stephenfin | You indent subsequent lines of the same bullet point, but the bullet points themselves should not be indented | |
| 13:32:59 | kashyap | stephenfin: Okido; like it is for the disk cache modes. | |
| 13:33:03 | kashyap | Thanks | |
| 13:36:08 | kashyap | stephenfin: Just a quick note: I'm using the back ticks for the lower-cased ``pcid``, and for the rest, with upper-cased 'PCID' (leaving it as is) | |
| 13:36:15 | kashyap | That fine with you? | |
| 13:36:25 | stephenfin | kashyap: a-ok by me, yup :) | |
| 13:36:59 | kashyap | :-) One upper-case ``PCID`` (because that is talking about valid config value) | |
| 13:37:07 | kashyap | Thanks! | |
| 13:40:54 | kashyap | stephenfin: johnthetubaguy: Before I upload yet-another version, mind having a quick gander: http://paste.openstack.org/show/718287/ | |
| 13:41:12 | kashyap | Or is it hard to read, without colours? (Can understand if so.) | |
| 13:41:38 | openstackgerrit | Mark Goddard proposed openstack/nova master: Include only required fields in ironic node cache https://review.openstack.org/539508 | |
| 13:42:30 | kashyap | Also add the "Related options" section | |
| 13:42:56 | johnthetubaguy | kashyap: probably easier in context, but that looks about right | |
| 13:43:52 | kashyap | Yeah; will post a patch in a min | |
| 13:44:31 | openstackgerrit | Mark Goddard proposed openstack/nova master: Request only instance_uuid in ironic node list https://review.openstack.org/539509 | |
| 13:46:00 | jaypipes | cdent, efried, edleafe: k, commented on the consumer generation spec. | |
| 13:46:07 | efried | jaypipes: ack, thx | |
| 13:46:16 | jaypipes | cdent, efried, edleafe: in short, I don't believe much change is needed from the spec at all. | |
| 13:47:15 | edleafe | jaypipes: thx | |
| 13:47:52 | edleafe | jaypipes: why do you think we shouldn't require sending the consumer gen on a PUT? | |
| 13:47:55 | openstackgerrit | sahid proposed openstack/nova master: network: add command to configure trusted mode for VFs https://review.openstack.org/458513 | |
| 13:47:55 | openstackgerrit | sahid proposed openstack/nova master: network: update pci request spec to handle trusted tags https://review.openstack.org/458820 | |
| 13:47:56 | openstackgerrit | sahid proposed openstack/nova master: libvirt: configure trust mode for vfs https://review.openstack.org/458514 | |
| 13:48:03 | edleafe | jaypipes: How else could it detect a conflict? | |
| 13:48:44 | efried | edleafe: He's suggesting PUT should *always* 409 if the consumer exists. | |
| 13:48:46 | jaypipes | edleafe: see my response. I believe that for PUT /allocations/{consumer_uuid}, if *any* allocations exist for a consumer with {consumer_uuid}, we should simply return 409 Conflict | |
| 13:49:06 | efried | I.e. PUT is only for first-time create; POST is for modify. | |
| 13:49:09 | cdent | that's a huge change jaypipes | |
| 13:49:29 | cdent | POST exists explicitly for multiple consumers | |
| 13:49:39 | efried | jaypipes: Also, we would still need to support create-via-POST. | |
| 13:49:40 | edleafe | yeah, not for a single consumer | |
| 13:49:42 | cdent | for single consumer PUT should remain the locus | |
| 13:50:09 | efried | ...which still needs an answer to "what should the generation be for a heretofore-nonexistent consumer | |
| 13:50:11 | efried | ?" | |
| 13:50:15 | edleafe | The canonical case we are handling is nova and neutron both allocating resources for an instance at roughly the same time | |
| 13:50:41 | edleafe | they both get the current allocations, if any, and make their changes, and PUT them back | |
| 13:50:52 | jaypipes | edleafe: no, they aren't doing it "at the same time". | |
| 13:51:11 | edleafe | One will get back a 409, and it will have to re-GET the allocations, add its allocs, and then PUT that with the new gen | |
| 13:51:31 | jaypipes | edleafe: yes. that is precisely the point. | |
| 13:52:01 | edleafe | jaypipes: that exact example is in the Problem Description of the spec | |
| 13:52:31 | jaypipes | edleafe: but in real life, what will happen is neutron will allocate first for the consumer some networky thing. then the claim_resources() will run in the scheduler for the other compute resources, get a 409 Conflict about consumer already existing, and then it will merge the allocations and do a POST /allocations with the updated/merged allocations list. | |
| 13:52:47 | edleafe | jaypipes: so without a consumer gen in the PUT, how would the second actor ever modify allocations? | |
| 13:53:16 | jaypipes | edleafe: the second actor does GET /allocations/{consumer_uuid}, then POST /allocations | |
| 13:53:24 | jaypipes | edleafe: as I wrote in the spec response. | |
| 13:53:24 | edleafe | <shudder> | |
| 13:53:58 | edleafe | jaypipes: that would be a PATCH, which I proposed but was argued against | |
| 13:54:04 | cdent | jaypipes: why POST instead of PUT? It is in fact (restully speaking) a PUT | |
| 13:54:18 | edleafe | the POST, as cdent mentioned, is for multiple consumers | |
| 13:54:36 | edleafe | The POST actually does a series of PUTs, one for each consumer | |
| 13:55:06 | jaypipes | cdent: because that ship has already sailed? I argued for PATCH instead of POST /allocations to handle multiple consumers, but we went with POST /allocations to handle the "change multiple allocations at once" scenario for migrations/resizes | |
| 13:55:58 | edleafe | jaypipes: sure, but we aren't changing *multiple* | |
| 13:56:08 | cdent | jaypipes: what edleafe just said | |
| 13:56:18 | jaypipes | edleafe: nothing about POST /allocations implies or denotes changing multiple things. | |
| 13:56:29 | cdent | there's only one consumer uuid. and yes it does | |
| 13:56:35 | cdent | post allocations is for multiple consumers | |
| 13:56:49 | jaypipes | cdent: where? | |