Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-02-05
15:37:46 ralonsoh sean-k-mooney, sure!
15:42:27 stephenfin sean-k-mooney: I changed the behaviour to rely on that 'port_details' field in a recent patch because I didn't know it was an optional extension
15:43:07 stephenfin sean-k-mooney: we need that info purely so we can get the 'device_id' field, which is the instance UUID, for the deprecated floating IP proxy APIs
15:43:12 stephenfin deprecated by not removed
15:46:36 sean-k-mooney we should not need that however.
15:47:14 sean-k-mooney we can list the ports assocaiated with an insnatce and then we should eb able to list the floating ips assinged ot each port
15:47:35 mriosfer sean : :The requested amount of video memory 128 is higher than the maximum allowed by flavor 0 :( something i changed wrong https://gyazo.com/302f96f1f0e2363da2b9dd10ad741e3e?token=b6e41e022fa6260b90802950e02137bf
15:48:04 stephenfin sean-k-mooney: That sounds like a lot more rework though :)
15:48:20 stephenfin Possible, yes. Worth it?
15:53:10 mriosfer sean: found the parameter is : hw_video:ram_max_mb
15:54:21 openstackgerrit Stephen Finucane proposed openstack/nova master: Rework how we check for extensions https://review.opendev.org/705792
15:54:36 sean-k-mooney mriosfer: that is the flavor one
15:54:44 sean-k-mooney yes
15:56:22 Sundar sean-k-mooney: I am here if you have any comments or questions about your evaluation of Cyborg patches.
15:56:57 sean-k-mooney Sundar: when i tested them yesterday it failed in the cyborg api to send the arq binding notification to nova
15:59:26 Sundar sean-k-mooney: Could you point me to the cyborg logs?
16:02:00 sean-k-mooney Sundar: http://paste.openstack.org/show/789142/
16:05:34 Sundar sean-k-mooney: Looks like you are pulling in an old version of Cyborg patches. NovaAPIConnectFailure exception has been replaced with InvalidAPIResponse exception: https://review.opendev.org/#/c/698846/6/cyborg/common/nova_client.py
16:06:48 mriosfer sean: dxdiag should show the param of 128MB for vRAM?
16:07:16 mriosfer https://gyazo.com/1ad894cd16911a7d6f3a9083fbd65fad
16:09:43 sean-k-mooney i think so yes
16:10:02 sean-k-mooney but i have not tested that
16:32:04 mriosfer humm im not sure if machine is getting the 128MB vram
16:32:20 mriosfer sockets and threats now looks better
16:32:55 efried dansmith: Left a review on https://review.opendev.org/#/c/631243/
16:32:55 efried TL;DR: the structural comments from PS43 still need to be addressed.
16:32:55 efried But dansmith (and gibi) it would be nice if you could scan through my analysis and see if you agree, or if I'm making a big deal out of nothing.
16:33:33 efried Basically I'm saying the steps of processing the device profiles should follow the steps of processing bandwidth requests.
16:34:04 dansmith efried: I don't have context on the bandwidth stuff to make that comparison, but will read
16:34:30 efried dansmith: I seeded the code with comments in the appropriate places, hopefully it's easy enough to follow.
16:34:46 dansmith ack
16:37:24 dansmith efried: I think we've told him specifically to follow the network_info and block_device_info patterns everywhere, which I think his code does
16:37:35 dansmith efried: i.e. make these look like our other attachable things, which are ports and volumes
16:38:03 efried I don't think what I'm suggesting deviates from that, does it?
16:38:24 dansmith seems like it, but I'm still reading
16:40:55 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Report COMPUTE_RESCUE_BFV and check during rescue https://review.opendev.org/701429
16:40:55 openstackgerrit Lee Yarwood proposed openstack/nova master: api: Introduce microverion 2.82 allowing boot from volume rescue https://review.opendev.org/701430
16:40:56 openstackgerrit Lee Yarwood proposed openstack/nova master: compute: Extract _get_bdm_image_metadata into nova.utils https://review.opendev.org/705212
16:40:56 openstackgerrit Lee Yarwood proposed openstack/nova master: WIP libvirt: Support boot from volume instance rescue https://review.opendev.org/701431
16:41:00 dansmith I'm also pretty sure we specifically told him to not use the legacy reqspec.from_components stuff
16:41:06 dansmith let me see if I can find that
16:44:26 efried What he's got will work fine afaict and does seem simpler at first glance.
16:44:26 efried My concern is that it's logically very similar to how we're processing port bandwidth requests (pull stuff from flavor and $api, create granular request groups, put them in a special place in the request spec),
16:44:26 efried so it would be nice if the reader could follow that logic similarly for both kinds of resource.
16:44:37 dansmith https://review.opendev.org/#/c/631243/30/nova/objects/request_spec.py
16:45:32 dansmith granted what he was doing was a lot more than what I _think_ you want him using from_components() for
16:46:15 openstackgerrit Stephen Finucane proposed openstack/nova master: objects: Add MigrationTypeField https://review.opendev.org/706013
16:46:19 efried dansmith: okay, yeah, I agree we shouldn't be doing the api callout from there. In what I'm suggesting, the device_profile_request_groups come into from_components ready-made, just like the port_resource_requests in the preceding chunk.
16:46:42 dansmith efried: shouldn't port_resource_requests just be resource_requests though?
16:46:53 efried yeah, that would be another great way to do it.
16:47:11 dansmith I'd much prefer that than just adding a new parameter of the same type of thing for each high level thing we add
16:47:42 efried Agree: fold port & device resource requests (and any others in the future) together prior to from_components. ++
16:48:57 efried so yeah -- how early can we fold those together? The earlier the better.
16:49:21 efried Maybe as early as the construction of base_options. Need to see how the supports_port_bandwith_requests business plays in.
16:49:25 dansmith that's not my preference, I'm just saying if we're still going to call this legacy method to set a single attribute, then they should be combined
16:49:47 dansmith what I'd *rather* is he leave what he has here and I follow up and move that port_resource_requests outside of from_components where it belongs in the first place, IMHO
16:50:19 efried I just don't love that from_components is initializing resource_requests, and then we're extending it after, outside of that method.
16:50:31 dansmith ack, that's legit
16:50:55 dansmith so can I follow up after his set and make PRR set after from_components() like he is doing here?
16:50:55 efried But you're right, we could easily accept what's here and refactor later.
16:51:11 dansmith if so I shall commit to it in writing
16:51:32 efried so, remove that as a param from from_components() entirely?
16:51:52 dansmith yeah
16:51:57 efried I don't have the big picture on from_components; you hinted we should be able to get rid of it entirely?
16:52:20 dansmith it was supposed to be bridge code to get us to objects and removed in mitaka or something
16:52:34 efried oh, didn't mriedem propose a WIP that started doing that?
16:52:46 dansmith he complained about it a lot, so probably
16:53:26 efried https://review.opendev.org/#/c/697686/ ?
16:53:38 efried no, not quite
16:53:49 dansmith well, that's what you're thinking of probably
16:54:11 efried yeah
16:54:15 dansmith regardless, as you can see, from_components is just "set a bunch of things and no other logic"
16:54:49 efried yeah. but it's common to both build and cold migrate
16:55:24 dansmith to what end?
16:55:33 efried just DRY
16:55:39 dansmith it's not really though
16:55:46 efried like, I'm not sure I see a better split logically. Whether we want to get rid of the filter_properties in some way, that's fine
16:55:49 dansmith because the actual constructor can take field values
16:56:20 efried There's a *little* bit of logic in there.
16:56:24 efried All these _from_* methods
16:57:03 dansmith okay, I guess so
16:57:20 dansmith but that would also kinda mean that the setting of a field that doesn't need any handling shouldn't be in here
16:58:55 efried So your refactor would be like the caller doing
16:58:55 efried reqspec = RequestSpec(foo=bar, ...)
16:58:55 efried for all foo/bar that are static, and then
16:58:55 efried reqspec.fold_in_other_shit(filter_properties, maybe_others, ...)
16:59:42 efried where the latter is from_components with all the static setters removed?
17:00:31 dansmith well, that's a potential bigger refactor, because from_components is a classmethod
17:00:50 dansmith and, I'm not sure that would really work with the way the _from things work.. they may expect to "go first"
17:01:24 dansmith so I would just take the req_spec that comes from from_components, and follow up with the direct things
17:01:36 dansmith req_spec = from_components(..); req_spec.requested_resources = port_requested_resources + accel_requested_resources; etc
17:06:08 efried I guess I don't see the benefit of assigning some fields in the classmethod and some afterwards. Similar reasoning as mentioned previously: then you always have to go make sure you're assigning the right pieces the right way on the right side of the method boundary.
17:06:29 efried but I don't feel strongly enough to -1 that.
17:06:44 mriedem beware the request spec quagmire
17:06:54 efried I think we're already quagged.
17:07:58 mriedem if you go one up that stack https://review.opendev.org/#/c/697697/ and read my comments, unwinding a bunch of that stuff is going to be dependent on doing a major conductor rpc api version
17:08:12 mriedem because there is backward compat code to handle the older version
17:08:20 mriedem which relies on a lot of this bridge code
17:08:36 dansmith efried: I commented and I'm late for another meeting
17:11:00 stephenfin melwitt: Does my reply at https://review.opendev.org/#/c/705654/ make sense?

Earlier   Later