| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2021-03-26 | |||
| 18:24:22 | openstackgerrit | Stephen Finucane proposed openstack/nova master: conf: Deprecate AggregateImagePropertiesIsolation opts https://review.opendev.org/c/openstack/nova/+/783396 | |
| 18:24:35 | sean-k-mooney | the draw back is you might need to update your flavors/image with teh traits | |
| 18:24:53 | stephenfin | Yes, I'm not sure we could ever remove it | |
| 18:24:53 | sean-k-mooney | but at least of new deployment i would not recommend turnning them on any more na just use that | |
| 18:25:00 | artom | Remote school is... interesting. She's already finished everything for the week with mom | |
| 18:25:13 | sean-k-mooney | well we could but it would need a few cycles | |
| 18:25:21 | stephenfin | at least not without offering a mechanism to update stored image/flavor metadata for an existing instance | |
| 18:25:32 | stephenfin | but the code costs very little to maintain and could happily be ignored | |
| 18:25:36 | sean-k-mooney | artom: enjoy | |
| 18:25:39 | stephenfin | anyway, PTG topic | |
| 18:26:05 | sean-k-mooney | artom: its the same type of claime we do for normal spawn | |
| 18:26:11 | artom | So yeah, we do a claim, but _update_port_binding_for_instance() assumed that pci_mapping can only change if there's a migration | |
| 18:26:15 | sean-k-mooney | which include claiming the pci devices | |
| 18:26:30 | artom | pci_mapping = self._get_pci_mapping_for_migration(instance, migration) | |
| 18:26:33 | sean-k-mooney | artom: ya that should like what might be happening | |
| 18:26:34 | artom | Yeah, we claim them | |
| 18:26:44 | sean-k-mooney | ah cool you found the issue then | |
| 18:26:46 | artom | But way before that, we've already updated the port bindings | |
| 18:27:01 | artom | With the old pci_slot, since the instance isn't actually migrating | |
| 18:27:25 | artom | Or rather, we haven't actually done the update, since pci_slot doens't change | |
| 18:28:31 | sean-k-mooney | well we just need to update the port again | |
| 18:28:43 | sean-k-mooney | it would be nice not to do it twice | |
| 18:28:52 | sean-k-mooney | but that is the simiple backportable fix | |
| 18:29:07 | sean-k-mooney | as long as we update it before we generate the xml its fine | |
| 18:29:41 | sean-k-mooney | then eventually i would like to use the adress form the claim/db instead but that harder | |
| 18:31:07 | artom | IOW, how do we *set* the pci_slot in the first place | |
| 18:31:20 | artom | _update_port_binding_for_instance() will only update it from a migration | |
| 18:38:02 | artom | Aha _populate_neutron_binding_profile() | |
| 18:38:14 | sean-k-mooney | ya it happens in the port bidning right | |
| 18:38:58 | sean-k-mooney | artom: i think its this https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L1332-L1333 | |
| 18:40:29 | sean-k-mooney | we call that during live migration fore example https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/conductor/tasks/live_migrate.py#L414 | |
| 18:43:36 | sean-k-mooney | artom: if you called self.network_api.bind_ports_to_host here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6579 | |
| 18:43:50 | sean-k-mooney | right before spawn i think that woudl fix it | |
| 18:44:41 | sean-k-mooney | you would need to change | |
| 18:44:42 | sean-k-mooney | with self.rt.instance_claim(context, instance, node, allocations, | |
| 18:44:44 | sean-k-mooney | limits): | |
| 18:44:53 | sean-k-mooney | to with self.rt.instance_claim(context, instance, node, allocations, | |
| 18:44:55 | sean-k-mooney | limits) | |
| 18:44:57 | sean-k-mooney | as claim | |
| 18:45:16 | sean-k-mooney | and then you could use the claim to get the pci diceis i think | |
| 18:49:28 | sean-k-mooney | artom: the update would look like this https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L10634-L10657 | |
| 18:49:57 | sean-k-mooney | hum actullyno | |
| 18:51:10 | sean-k-mooney | well no this is how we actully do it for live miggration https://opendev.org/openstack/nova/src/branch/master/nova/compute/manager.py#L10597-L10632 | |
| 19:01:33 | artom | I guess we'd have to distinguish between unshelve from just SHELVED and also SHELVED_OFFLOADED | |
| 19:01:38 | sean-k-mooney | artom: in the normal spawn workflow after we take the instance calim we call _build_resources | |
| 19:01:54 | sean-k-mooney | which does call that yes | |
| 19:02:10 | sean-k-mooney | well it all alocate_netork | |
| 19:02:47 | sean-k-mooney | artom: https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/compute/manager.py#L2349 | |
| 19:02:58 | sean-k-mooney | that is what allocate the networks on spawn normally | |
| 19:03:07 | artom | Yeah, I've been looking at that... | |
| 19:03:08 | sean-k-mooney | but we dont do that on unshelve | |
| 19:03:23 | artom | Which makes sense... but only if the instance was only shelved | |
| 19:03:40 | sean-k-mooney | well we woudl be calling it form unshelve | |
| 19:03:54 | sean-k-mooney | here https://github.com/openstack/nova/blob/master/nova/compute/manager.py#L6579 | |
| 19:04:13 | sean-k-mooney | litrally after the instance_claim but just before spawn | |
| 19:06:49 | sean-k-mooney | this is where its update for other move ops right https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L3394-L3419 | |
| 19:07:04 | sean-k-mooney | but since thre is no migration that oviously does not work | |
| 19:09:04 | artom | Ah, we can't allocate_for_instance on unshelve because even OFFLOADED instances keep their ports | |
| 19:09:15 | artom | So I'd think all kinds of things would explode | |
| 19:09:35 | artom | We really do need just the _update, but we need to somehow pull the PCI info from the claim, and pass that to the neutron api later | |
| 19:09:37 | sean-k-mooney | maybe | |
| 19:09:38 | artom | *layer | |
| 19:09:42 | sean-k-mooney | its doing more then we want anyway | |
| 19:10:40 | sean-k-mooney | _update_ports_for_instance is closer | |
| 19:11:39 | sean-k-mooney | https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L1498-L1524 | |
| 19:12:01 | sean-k-mooney | this is what actully update the port with the bindingprofile | |
| 19:12:22 | sean-k-mooney | the issue is we dont have a way to map the neutorn por tto the pci_request_id | |
| 19:12:46 | sean-k-mooney | if we had that we woudl jsut get the dev doing | |
| 19:12:50 | sean-k-mooney | pci_devices = pci_manager.get_instance_pci_devs( | |
| 19:12:52 | sean-k-mooney | instance, pci_request_id) | |
| 19:13:23 | sean-k-mooney | then https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L1487-L1496 | |
| 19:13:29 | artom | sean-k-mooney, yeah, I'm hitting that exact issue | |
| 19:13:37 | artom | With the migration, we have the mapping from old to new | |
| 19:13:38 | sean-k-mooney | would get that for us | |
| 19:13:46 | sean-k-mooney | yep | |
| 19:13:59 | sean-k-mooney | so i looked at sotring the pci resuet id in the vif breilfy | |
| 19:14:11 | sean-k-mooney | but unfortunetly that is a object change | |
| 19:14:57 | sean-k-mooney | hum https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L2112 | |
| 19:14:57 | artom | I'm assuming pci_vendor_info and physical_network from the port binding isn't enough | |
| 19:15:02 | artom | Even assuming we have the latter | |
| 19:15:23 | sean-k-mooney | https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L2165 | |
| 19:15:48 | sean-k-mooney | the pci request id shoudl be the nueturno port uuid | |
| 19:15:52 | artom | Ohhh | |
| 19:16:29 | sean-k-mooney | so i thnk we can look it up like that? | |
| 19:18:13 | sean-k-mooney | so if we get teh port we could loop over therm calling https://github.com/openstack/nova/blob/68ad2c67d891935c66e8c0e5a368695c202a9f49/nova/network/neutron.py#L1498-L1524 | |
| 19:18:41 | sean-k-mooney | passing the port['id'] as pci_request_id | |
| 19:18:55 | sean-k-mooney | and then after that has updated them in memeory we woudl do a port update? | |
| 19:19:22 | sean-k-mooney | then i think we cloud call driver .spawn | |
| 19:20:39 | sean-k-mooney | artom: do you think that would work? | |
| 19:31:28 | sean-k-mooney | ... its not the neutron port uuid | |
| 19:31:52 | artom | sean-k-mooney, think so, yeah | |
| 19:31:58 | artom | Lemme try it | |
| 19:33:11 | sean-k-mooney | artom: the requester_id is the neutron port uuid | |
| 19:33:25 | sean-k-mooney | the pci_request_id is the request_id | |
| 19:33:40 | sean-k-mooney | unfortunetly the requester_id is not sotred in teh db | |
| 19:34:54 | sean-k-mooney | the requested_networks network object i think is the only thing that has the mapping | |
| 19:35:51 | sean-k-mooney | i think this is the same roadblock i hit the last time | |
| 19:39:40 | sean-k-mooney | ... | |
| 19:39:51 | sean-k-mooney | so the instance_extra tbale has it but not fully | |
| 19:39:54 | sean-k-mooney | [{"count": 1, "spec": [{"physical_network": "public"}], "alias_name": null, "is_new": false, "numa_policy": "preferred", "request_id": "03e6f4be-dc49-4825-a33d-337f2708be22", "requester_id": null} | |