Earlier  
Posted Nick Remark
#openstack-nova - 2018-04-03
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?
13:57:02 efried POST is allowed to be used for just one, sure.
13:57:03 cdent that's why we created it and how I wrote it
13:57:09 edleafe jaypipes: the handler for POST loops on consumer_uuid. It's pretty clear that it is designed to handle multiple consumers
13:57:11 cdent it _can_ do one, but that's not it's point
13:57:37 jaypipes so tell me folks, what about my proposal *won't work*?

Earlier   Later