| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-05-13 | |||
| 11:16:54 | stephenfin | The less magic, the better | |
| 11:17:41 | sean-k-mooney | ack its nice form an opts perspectvie if an only if it means they dont have to do anything on upgrade | |
| 11:17:53 | stephenfin | sean-k-mooney: woah, that vIOMMU thing has got way bigger. I was proposing enabling by default on supported platforms with zero configurability. We now have...three knobs? :-O | |
| 11:18:40 | stephenfin | I've asked ricolin to clarify the need for each knob since it's not at all obvious from reading the spec. If they're really necessary, we'll need this info to document the extra specs. | |
| 11:18:46 | gibi | upgrading to Zed (if it lands) with default config will mean only the old PCI scheduling will be used by nova. So no real upgrade impact. Then operators needs to enable the new tracking | |
| 11:18:49 | sean-k-mooney | stephenfin: yep. the model, bit with which i dont like exposing but is required and locked memroy wich is technialy unrealted | |
| 11:19:23 | sean-k-mooney | but they need locked memroy for there specific hardware and you can only get that with realtim or sev today | |
| 11:19:42 | sean-k-mooney | gibi: yep with no config changes old behavior | |
| 11:19:58 | gibi | so we need to incentivise ops to enable the new behavior | |
| 11:20:00 | stephenfin | At risk of pre-empting discussion on the spec, why is the model necessary? Are these guest OS behavior implications or something? Is there not a sane default we can pick? | |
| 11:20:04 | sean-k-mooney | so no upgrade impact by defualt but we will ahve to document how to move form one to the other | |
| 11:20:39 | sean-k-mooney | stephenfin: the sane default would be virtio but it need a very new libvirt/qemu | |
| 11:21:03 | sean-k-mooney | we cant use intel because well there usecase is for aarch64 servers | |
| 11:21:33 | sean-k-mooney | we could choose dynmically i guess | |
| 11:21:47 | stephenfin | And I'm guessing we can't check what the libvirt version is and use 'virtio' if libvirt > 8.3.0 else 'intel'/'smmuv3' for Intel/ARM respectively? | |
| 11:21:49 | sean-k-mooney | based on the machine-type but i dont like the live migration implciations of that | |
| 11:22:01 | stephenfin | So long as we record the model used in system metadata, we should be fine | |
| 11:22:05 | sean-k-mooney | well we could but ya | |
| 11:22:09 | stephenfin | (i.e. so we don't change during rebuild) | |
| 11:22:14 | stephenfin | *migration | |
| 11:22:19 | sean-k-mooney | record in system_metadta or elsewhere for migration | |
| 11:22:23 | stephenfin | Yup | |
| 11:22:46 | sean-k-mooney | i was hoping we woudl only expose the model extra spec initally | |
| 11:22:51 | stephenfin | In case it's not obvious, I hate exposing knobs unless they're really useful. If people want all the knobs, use libvirt/oVirt :) | |
| 11:23:15 | sean-k-mooney | yep i know it had 6 i think at one point | |
| 11:23:25 | sean-k-mooney | i have been slowing getting it down to the minimal set | |
| 11:25:45 | stephenfin | what about aw_bits? | |
| 11:25:54 | sean-k-mooney | stephenfin: by the way im not sure if we can just turn the viommu on always or if there are performance overhead even if you dont explicty try to use it for say realtime guests | |
| 11:26:06 | sean-k-mooney | stephenfin: that i really did not want to expose | |
| 11:26:53 | sean-k-mooney | stephenfin: mnaser left a comment about why they needed it i dont knwo if we can always set it to the max support or if it need to be tuneable | |
| 11:29:23 | sean-k-mooney | actully it was on irc let me see if i can find it | |
| 11:31:12 | sean-k-mooney | stephenfin: i actully tought you exposed that in your patch by the way | |
| 11:31:16 | sean-k-mooney | the aw_bit | |
| 11:31:56 | stephenfin | Nope https://review.opendev.org/c/openstack/nova/+/830646/1/nova/virt/libvirt/config.py#3676 | |
| 11:32:27 | sean-k-mooney | oh so ricolin added it | |
| 11:33:07 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/830646/5/nova/virt/libvirt/config.py#3700= | |
| 11:33:21 | sean-k-mooney | ah you just found it | |
| 11:34:08 | sean-k-mooney | stephenfin: so if this really is needed i woudl liek to tie its value to the vm. | |
| 11:34:18 | sean-k-mooney | stephenfin: so either in instance_system_metadta | |
| 11:34:25 | sean-k-mooney | or as a flavor/image property | |
| 11:34:55 | sean-k-mooney | if it was a per host option we would have to schdule on it some how | |
| 11:35:17 | sean-k-mooney | and possible extend the migration data objects if we did not store it in the instace_system_metadata | |
| 11:35:29 | stephenfin | Yeah, no arguments from me there. Host-level configuration for guest-specific knobs is always a problem | |
| 11:35:34 | sean-k-mooney | i was hoping we could jsut not set it and let it up ot libvirt | |
| 11:37:32 | stephenfin | me too | |
| 11:38:01 | sean-k-mooney | """Use virtio if libvirt >= 8.5.0, else intel or the aarch64 equivalent""" hehe i can say the aarch64 equivalent so i cant rememeber it either | |
| 11:39:57 | sean-k-mooney | stephenfin: since i have your attention do you have any feedback on https://review.opendev.org/c/openstack/oslo-specs/+/839819 im trying to poc it now | |
| 11:40:50 | stephenfin | Sure | |
| 11:41:47 | stephenfin | I think I know what the problem is there already 🙃 | |
| 11:42:02 | sean-k-mooney | your right i should have stuck with nano | |
| 11:42:19 | sean-k-mooney | actully the problem was it was sudenly in vim mode i think | |
| 11:42:37 | stephenfin | touché | |
| 11:42:43 | sean-k-mooney | i use the spacemecs deistribution which has the evil plugin | |
| 11:42:54 | sean-k-mooney | which by default make it have vim key bindings | |
| 11:43:14 | sean-k-mooney | so for some reason i coudl not use any of the ones im use to or select text | |
| 11:44:04 | sean-k-mooney | anyway its back to working in emacs/nano like mode and i can use it again but that was not fun | |
| 12:24:18 | stephenfin | gibi: sean-k-mooney: I think this is ready to merge now https://review.opendev.org/c/openstack/nova/+/839029 | |
| 12:24:36 | stephenfin | also, it passes 🎉 | |
| 12:25:04 | gibi | stephenfin: yepp, +2 | |
| 12:26:20 | sean-k-mooney | oh cool let me run it locally first since i have 3.10 and then ill +w | |
| 12:29:22 | sean-k-mooney | stephenfin: i could have sworne i made the fucntional evne generitive at some point | |
| 12:29:37 | stephenfin | you did, but it hasn't merged. I stole that idea sorrynotsorry :) | |
| 12:30:15 | sean-k-mooney | oh i tought that merged ages ago | |
| 12:30:26 | sean-k-mooney | i guess i should take a look at that again | |
| 12:30:46 | sean-k-mooney | cool your patch is running locally | |
| 12:31:31 | sean-k-mooney | there are a bunch of deprecations warnings | |
| 12:31:43 | stephenfin | 3.10 specific? | |
| 12:31:51 | sean-k-mooney | home/sean/repos/openstack/nova-3/nova/tests/fixtures/notifications.py:42: DeprecationWarning: notifyAll() is deprecated, use notify_all() instead | |
| 12:31:53 | sean-k-mooney | self._cond.notifyAll() | |
| 12:32:04 | stephenfin | I know distutils is puking everywhere since that's going away in 3.12 | |
| 12:32:22 | sean-k-mooney | stephenfin: so yes i think that is 3.10 related | |
| 12:32:32 | sean-k-mooney | it seams to just be that actully | |
| 12:32:41 | sean-k-mooney | but its multiple times on every test | |
| 12:34:00 | sean-k-mooney | ok not every test | |
| 12:34:02 | sean-k-mooney | https://zuul.opendev.org/t/openstack/build/8a45e04d9dbf4703bb1fb9f7d91a549b/log/job-output.txt#8815 | |
| 12:34:18 | sean-k-mooney | but we see it in the ci too anything that uses that fixture | |
| 12:34:57 | sean-k-mooney | that seams to be the only one | |
| 12:35:23 | sean-k-mooney | care to fix that in a followup maybe or respin | |
| 12:35:27 | stephenfin | sure | |
| 12:47:08 | opendevreview | Alexey Stupnikov proposed openstack/nova stable/wallaby: Test aborting queued live migration https://review.opendev.org/c/openstack/nova/+/841483 | |
| 12:54:44 | sean-k-mooney | stephenfin: actully do you want me to just fix the notificaiton fixture in a followup or are you already working on it | |
| 12:54:59 | stephenfin | go for it; I'm playing around with cinder stuff rn | |
| 12:55:13 | sean-k-mooney | cool will do | |
| 13:03:12 | gibi | stephenfin, sean-k-mooney: should we just rename the whitelist to device_list or we make the two config totally separate and use the device_list also to enable the new feature? | |
| 13:04:16 | gibi | if we make them separate then in the initial release the whitelist still needed for the neutron based sriov even if the device_list is used for the alias based PCI passthrough | |
| 13:05:15 | gibi | now I think that renaming is simpler and then I would add an extra config to enable the new placement based PCI tracking | |
| 13:05:39 | sean-k-mooney | ok | |
| 13:05:57 | sean-k-mooney | lets rename and add a hopefully tempoery extra option for placment tracking | |
| 13:06:04 | gibi | should the placement PCI tracking config be on libvirt virt driver level or on the compute level? | |
| 13:06:15 | gibi | compute level is future proof | |
| 13:06:26 | gibi | but a bit confusing as it will only be used by the libvirt driver now | |
| 13:06:38 | sean-k-mooney | good question | |
| 13:06:43 | sean-k-mooney | you could put it in the pci section | |
| 13:07:04 | gibi | that is a good middle ground | |
| 13:07:05 | gibi | thanks | |
| 13:07:08 | sean-k-mooney | [pci]/report_in_placement=True|False | |
| 13:08:06 | gibi | ack | |
| 13:08:19 | sean-k-mooney | stephenfin: any opipion ^ | |
| 13:09:32 | stephenfin | Agreed. Sounds like a '[pci]' option | |
| 13:09:34 | sean-k-mooney | gibi: i know there are some that want us to do the rename anyway for policital reasons so we proably should not make operators choose between fucntionality and politics | |