| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-07 | |||
| 18:45:04 | sean-k-mooney | which is invoked as part of _init_compute_node | |
| 18:45:58 | dmitriis | sean-k-mooney: ack. And when it comes to exposing the property, I could populate _has_remote_managed after this line https://github.com/openstack/nova/blob/b6fe7521afa8d42febc68f5f79782f7bcc3b568f/nova/pci/whitelist.py#L51 and expose a property | |
| 18:46:11 | dmitriis | and also expose a property on the spec objects themselves | |
| 18:46:42 | dmitriis | I'd have to do a search over spec objects then and see whether any one of those has a property | |
| 18:46:58 | dmitriis | if that's ok, I'll just do it | |
| 18:47:21 | sean-k-mooney | i think that shoudl be ok | |
| 18:48:01 | sean-k-mooney | it sound cleaner to me overall | |
| 18:48:29 | sean-k-mooney | where/why are you using this by the way | |
| 18:49:02 | dmitriis | sean-k-mooney: this is to make sure remote_managed tags aren't usable on a host where libvirt is too old (doesn't have VPD handling code) | |
| 18:49:25 | sean-k-mooney | right so you dont need this on the spec object or whitelist object | |
| 18:49:34 | sean-k-mooney | that is a check you can do in the libvirt driver at start up | |
| 18:49:54 | sean-k-mooney | you can just loop over the spec objects and check for the tag | |
| 18:50:04 | sean-k-mooney | and check the verisons | |
| 18:50:40 | sean-k-mooney | the device spec will already have the value in the PciDeviceSpec.tags field | |
| 18:50:49 | dmitriis | https://review.opendev.org/c/openstack/nova/+/827839/2/nova/virt/libvirt/driver.py#816 | |
| 18:51:13 | dmitriis | ^ so it's a host-based capability | |
| 18:51:20 | dmitriis | host-dependent * | |
| 18:51:36 | sean-k-mooney | yes | |
| 18:52:15 | sean-k-mooney | that does not need to you modify these data stuctures | |
| 18:52:36 | dmitriis | sean-k-mooney: I don't have access to the host object in those | |
| 18:52:46 | dmitriis | but maybe there's some other way to check the Libvirt version | |
| 18:52:53 | sean-k-mooney | you should not be checkign for them in these | |
| 18:53:03 | sean-k-mooney | dmitriis: you shoudl not be doing https://review.opendev.org/c/openstack/nova/+/827839/2/nova/pci/devspec.py#319 | |
| 18:53:23 | sean-k-mooney | that is not the correct place to raise that excption | |
| 18:53:36 | sean-k-mooney | either the device spec of whitelist shoudl raise that | |
| 18:53:46 | dmitriis | sean-k-mooney: ah, sorry, yes we're moving it elsewhere | |
| 18:54:40 | dmitriis | sean-k-mooney: so maybe in the dev tracker I have access to that info, checking | |
| 18:54:58 | sean-k-mooney | i dont think you do | |
| 18:55:09 | sean-k-mooney | we abstrackt the virt driver info away | |
| 18:55:20 | sean-k-mooney | the pci module is ment ot be virt dirver independent | |
| 18:55:32 | sean-k-mooney | you shoudl be doing this check in the libvirt driver.py | |
| 18:56:19 | sean-k-mooney | you coudl perhaps do it in update_devices_from_hypervisor_resources | |
| 18:56:21 | sean-k-mooney | https://github.com/openstack/nova/blob/b6fe7521afa8d42febc68f5f79782f7bcc3b568f/nova/pci/manager.py#L111 | |
| 18:56:36 | sean-k-mooney | but you would have to modify the device_json which im not sure is the right thing | |
| 18:57:40 | sean-k-mooney | am i need to call it a day | |
| 18:57:56 | dmitriis | sean-k-mooney: ack, I'll think about how to do it | |
| 18:57:56 | sean-k-mooney | dmitriis: can you look at moving the check to the libvirt driver | |
| 18:58:09 | dmitriis | sean-k-mooney: sure | |
| 19:32:39 | opendevreview | Dan Smith proposed openstack/nova master: Join quota exception family trees https://review.opendev.org/c/openstack/nova/+/828185 | |
| 19:32:39 | opendevreview | Dan Smith proposed openstack/nova master: Move keypair quota error message into exception https://review.opendev.org/c/openstack/nova/+/828186 | |
| 19:32:40 | dansmith | melwitt: ^ | |
| 19:32:51 | dansmith | I have one other thing I'm going to stack on there, but it'll take me a few more minutes | |
| 19:33:51 | melwitt | dansmith: ack | |
| 19:34:30 | opendevreview | Dan Smith proposed openstack/nova master: Move keypair quota error message into exception https://review.opendev.org/c/openstack/nova/+/828186 | |
| 20:02:47 | chateaulav | <nova:flavor name="r1.small"> | |
| 20:02:47 | chateaulav | <nova:memory>4096</nova:memory> | |
| 20:02:47 | chateaulav | <nova:disk>16</nova:disk> | |
| 20:02:47 | chateaulav | <nova:swap>0</nova:swap> | |
| 20:02:47 | chateaulav | <nova:ephemeral>0</nova:ephemeral> | |
| 20:02:48 | chateaulav | <nova:vcpus>2</nova:vcpus> | |
| 20:02:48 | chateaulav | </nova:flavor> | |
| 20:03:13 | chateaulav | sean-k-mooney: so setting any topology items doesnt work | |
| 20:03:26 | chateaulav | the above and below are what is set in regards to cpu | |
| 20:03:31 | chateaulav | https://www.irccloud.com/pastebin/f9YIhb6z/ | |
| 20:09:44 | opendevreview | Dan Smith proposed openstack/nova master: Join quota exception family trees https://review.opendev.org/c/openstack/nova/+/828185 | |
| 20:09:44 | opendevreview | Dan Smith proposed openstack/nova master: Move keypair quota error message into exception https://review.opendev.org/c/openstack/nova/+/828186 | |
| 20:12:32 | opendevreview | melanie witt proposed openstack/nova master: Raise InstanceNotFound on fkey constraint fail saving info cache https://review.opendev.org/c/openstack/nova/+/826942 | |
| #openstack-nova - 2022-02-08 | |||
| 00:58:47 | opendevreview | Keigo Noha proposed openstack/nova stable/wallaby: libvirt: Always delegate OVS plug to os-vif https://review.opendev.org/c/openstack/nova/+/828148 | |
| 08:07:36 | opendevreview | Ilya Popov proposed openstack/nova master: Fix to implement 'pack' or 'spread' VM's NUMA cells https://review.opendev.org/c/openstack/nova/+/805649 | |
| 10:18:23 | kashyap | sean-k-mooney: Do you know if Swift is requirement for vTPM to work? The docs say no: https://docs.openstack.org/nova/latest/admin/emulated-tpm.html | |
| 10:18:39 | kashyap | But the spec mentions "Swift" | |
| 10:19:12 | sean-k-mooney | no its not | |
| 10:19:26 | sean-k-mooney | the storage to swift was never implemented | |
| 10:20:02 | kashyap | Ah-ha, good to know. | |
| 10:20:03 | kashyap | Thank you | |
| 10:20:39 | sean-k-mooney | if i recall correctly that was propsed for shelve which i belive we did not implement | |
| 10:21:30 | sean-k-mooney | kashyap: barbican is needed however for the key management | |
| 10:21:45 | kashyap | Yeah, figured as much from the doc | |
| 11:33:28 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: [yoga] Include pf mac and vf num in port updates https://review.opendev.org/c/openstack/nova/+/824833 | |
| 11:33:29 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Introduce remote_managed tag for PCI devs https://review.opendev.org/c/openstack/nova/+/824834 | |
| 11:33:29 | opendevreview | Dmitrii Shcherbakov proposed openstack/nova master: Bump os-traits to 2.7.0 https://review.opendev.org/c/openstack/nova/+/826675 | |
| 11:38:58 | dmitriis | sean-k-mooney: added handling for the PF PCI address + vf product/device id + remote managed tag case by using some additional info from sysfs: https://review.opendev.org/c/openstack/nova/+/824834/10/nova/pci/devspec.py#322 Also added unit tests to cover various ways to specify this in a whitelist | |
| 11:39:57 | dmitriis | handling of ANY ("*") for vendor/product id along with PF PCI address specified is interesting | |
| 11:45:11 | sean-k-mooney | well if its a PF and you dont specify the vendor and product id of the vf i think we can treat that as an error | |
| 11:45:24 | sean-k-mooney | since pfs are not supported | |
| 11:45:47 | sean-k-mooney | just using the adress of the pf woudl allow the pf not the vfs normally i belive | |
| 12:22:52 | dmitriis | sean-k-mooney: looks like WhitelistPciAddress.match treats the presence of pci_phys_addr as an indicator that the caller is trying to match a VF https://review.opendev.org/c/openstack/nova/+/824834/10/nova/pci/devspec.py#252 And then PciDeviceSpec.match https://review.opendev.org/c/openstack/nova/+/824834/10/nova/pci/devspec.py#377 does a | |
| 12:22:52 | dmitriis | comparison of its vendor_id/product_id against both ANY and the device dict passed in. | |
| 12:30:30 | dmitriis | IIUC, something like this `passthrough_whitelist = {"vendor_id":"*","product_id":"*", "address":"0000:0a:00.0"}` would match both a PF with address 0000:0a:00.0 and any vendor/device ID, and VFs of any PF with a PCI address 0000:0a:00.0 | |
| 12:33:02 | dmitriis | so, in summary: I could block this explicitly if remote_managed: "true" is present because it allows both PFs and VFs to be matched but we want to prohibit PFs. | |
| 12:35:47 | sean-k-mooney | dmitriis: sorry was on a meeting reading back | |
| 12:35:52 | dmitriis | np | |
| 12:36:51 | sean-k-mooney | you mean block "vendor_id":"*","product_id":"*", explcitly if remote managed | |
| 12:37:24 | sean-k-mooney | i think you need to do this outside of the whitelist/devspec parseing | |
| 12:38:17 | sean-k-mooney | when we are constucting the device pools in the pci tracker we could block it as we woudl know if its a pf | |
| 12:46:41 | sean-k-mooney | dmitriis: im just reviewing you latest patches by the way lets continue this conversation after that | |
| 12:47:27 | dmitriis | sean-k-mooney: ack, trying to figure out what to move to PciDevTracker. There's some remove-managed-related logic already in the PciDeviceSpec in the current patches | |
| 12:49:45 | sean-k-mooney | one important thign to rememebr is the nova/pci fould has to be driver independent so we need to ensure that we do not do any checks there that would be driver dependent. those need to live in the virt driver. in this case i think you can do checks pci manager possible in update_devices_from_hypervisor_resources | |
| 12:50:34 | sean-k-mooney | similar to https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L134-L136 | |
| 12:51:11 | sean-k-mooney | that is currently delegating back to the whitelist class https://github.com/openstack/nova/blob/master/nova/pci/whitelist.py#L85-L93 | |
| 12:51:49 | sean-k-mooney | but its really just validting the whitelist data is correct and that the device being tested is allowed | |
| 12:52:50 | sean-k-mooney | dmitriis: i could maybe live with a check here https://github.com/openstack/nova/blob/master/nova/pci/whitelist.py#L92 | |
| 12:53:17 | sean-k-mooney | assert that dev is not a pf if spec has remote managed | |
| 12:55:28 | sean-k-mooney | dmitriis: perhaps it would make sense to change device_assignable to return the spec that allowed it so that you can use it to do other validation | |
| 12:55:37 | dmitriis | sean-k-mooney: ok, I think I see where you are going. While the PF and VF-related checks for the remote-managed tag are not themselves driver-dependent, whether we need to do them or not is driver-depenent. | |
| 12:56:14 | dmitriis | s/depenent/dependent/ | |
| 12:57:02 | sean-k-mooney | well yes and no the data required to do the check is driver depenent since we need to get the list of device form the driver | |
| 12:57:14 | sean-k-mooney | i guess technially you could get that from sysfs. | |
| 12:57:32 | sean-k-mooney | im concerned that might break powervm | |
| 12:57:53 | sean-k-mooney | or hyperv but im not sure it would | |