| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-21 | |||
| 15:50:13 | dansmith | yeah | |
| 15:51:07 | gibi | I can file a bug and link it to the downstream PCI in Placement Jira as well so we will have time for it to fix | |
| 15:51:08 | dansmith | gibi: so ... why again can we not support just the trait? You can ask for all providers with that trait, no? | |
| 15:51:30 | gibi | you cannot have an allocation candidate without telling placement what resource you are allocating | |
| 15:51:51 | dansmith | because you're asking for inventory not a provider? | |
| 15:52:09 | dansmith | (and providers have the traits) | |
| 15:52:40 | gibi | I'm asking for things I can allocate, I cannot allocate a trait or a provider I can only allocate a piece of inventory | |
| 15:53:06 | dansmith | right, that's what I mean | |
| 15:53:09 | gibi | yepp | |
| 15:54:13 | gibi | in our oversimplified case where a single PF RP only have a single RC with a single inventory allocation the RP or allocating one piece of that inventory is the same. | |
| 15:54:38 | gibi | so it is easy to make that mental jump that we want to allocate RPs | |
| 15:56:42 | gibi | this also shows the effect of the design decisions that we connect traits to RPs not to an inventory of the RP | |
| 15:57:05 | gibi | we allocate inventories but we filter on traits that are not on the inventories | |
| 15:57:20 | gibi | it is a bit of a mixup | |
| 15:57:35 | gibi | so we tend to namespace traits to somehow refer to inventories :) | |
| 15:58:04 | gibi | like with the OTU trait, we put PCI in the name to signify it is only related to PCI inventories of the RP | |
| 15:58:18 | gibi | anyhow | |
| 15:58:23 | gibi | I will going to file a bug | |
| 15:58:32 | gibi | based on your paste | |
| 16:01:42 | Uggla | gibi, yes but finally the code has blocked the invalid settings. So not so bad. | |
| 16:02:00 | gibi | https://bugs.launchpad.net/nova/+bug/2111440 | |
| 16:02:20 | gibi | Uggla: yep, but we should consider pre-validating the alias at sevice startup | |
| 16:02:30 | gibi | and also be a bit more instructive in the lgs | |
| 16:02:31 | gibi | logs | |
| 16:02:46 | dansmith | gibi: yeah there have been a few things in this journey where traits applied to providers to say something about inventory has been a bit weird | |
| 16:03:44 | dansmith | gibi: Uggla I also had a typo in my pci alias config (unparseable json) and it didn't get noticed until the first time I tried to boot with a pci-enabled flavor, and we returned json parse complaints to the user in the error :( | |
| 16:04:00 | dansmith | which of course is "line 1, character X" which makes no sense since they're all one line | |
| 16:04:11 | dansmith | would be much better if we can parse and log on startup which one is the problem | |
| 16:05:36 | gibi | filed the upstream bug and a matching downstream jira | |
| 16:07:49 | opendevreview | sean mooney proposed openstack/nova master: move compile earlier https://review.opendev.org/c/openstack/nova/+/950516 | |
| 16:11:46 | sean-k-mooney | dansmith: that a typo in the docs | |
| 16:12:30 | sean-k-mooney | when we added the docs for pci maged and live migration flags | |
| 16:12:47 | dansmith | what's a typo in the docs? | |
| 16:13:07 | sean-k-mooney | we missed it was not valid json. i hit the same thing when testin gthe OTU devices feature and pinged Uggla but i dont knwo if we filed an upstream bug or not to fix it | |
| 16:13:54 | sean-k-mooney | gibi: validiatign the config on start up is good we also shoudl be cachign it. | |
| 16:15:24 | sean-k-mooney | gibi: https://review.opendev.org/c/openstack/nova/+/427145 | |
| 16:16:22 | gibi | sean-k-mooney: good point, added the link to the bug | |
| 16:17:56 | sean-k-mooney | stephen didn twant the addtional complexity but going form 4062 function calls to 3 after caching is a masive savign | |
| 16:18:54 | sean-k-mooney | espically since we can just slap functools.cache on it now | |
| 16:20:06 | sean-k-mooney | from the blueprint """During the creation of an instance, a list of request is build. One of this requested elements are the PCI devices, both from the API call and from the flavor. | |
| 16:20:08 | sean-k-mooney | In the PCI request gathering from the flavor, the variable "pci_alias", stored in the Nova config file, is always parsed. This step is only needed once, because the information is static.""" | |
| 16:23:34 | sean-k-mooney | so every time we need to trasnlate a pci alise to to a pci request object we parst all the alisas again https://github.com/openstack/nova/blob/master/nova/pci/request.py#L124-L209 | |
| 16:24:43 | dansmith | Uggla: (cc gibi) FYI nvme emulation in qemu does work for testing this and is cleanable (via format only, not sanitize) | |
| 16:25:27 | Uggla | dansmith, cool ! | |
| 16:25:29 | dansmith | that device reports that it supports lots of namespaces (not sure if it really does, haven't tested yet) but that flags a warning I have queued for the script locally about multi-namespace devices being cleaned with format only (so that's a good test) | |
| 16:25:36 | sean-k-mooney | dansmith: the only issue wiht that iw while qemu can emulate it libvirt currently does nto supprot that so you have to use qemu directly unless i missed something in the xml for this ? | |
| 16:25:57 | dansmith | sean-k-mooney: right, you can configure it in the xml, but only via qemu:commandline | |
| 16:26:08 | gibi | cool. So we can use that for local testing. | |
| 16:26:12 | dansmith | so not helpful for people testing on top of an existing nova impl, unfortunately | |
| 16:26:17 | gibi | not in CI though | |
| 16:26:18 | dansmith | gibi: yeah | |
| 16:26:20 | sean-k-mooney | you can yes | |
| 16:26:20 | dansmith | right | |
| 16:27:02 | dansmith | gibi: it's also helpful so you can get a bunch of devices instead of just one (if you only have one) | |
| 16:27:14 | sean-k-mooney | i looked into that when we were reviwiong the spec but kind of gave up when i realed the gap in libvirt. you could just add the qemu args in virt manager to test it locally or just invoke qemu yourslef | |
| 16:28:44 | dansmith | https://termbin.com/tm0b | |
| 17:03:07 | sean-k-mooney | @gibi: the crd change is in merge conflict https://github.com/openstack-k8s-operators/nova-operator/pull/948 | |
| 17:03:24 | sean-k-mooney | gibi: im sure its trival but it will need to be rebased before it can merge | |
| 17:03:45 | sean-k-mooney | oh you already have | |
| 17:04:32 | sean-k-mooney | gibi: in that case ill add lgtm and assumign it passes ci it can proceed, does that work for you? | |
| 17:07:47 | gibi | yepp | |
| 17:07:54 | gibi | works for me | |
| 17:08:06 | gibi | (strange place to discuss it but fine :) | |
| 17:13:12 | sean-k-mooney | gibi: hehe wrong tab | |
| 17:52:48 | opendevreview | Dan Smith proposed openstack/nova master: Make example OTU cleaner support NVMe sanitize https://review.opendev.org/c/openstack/nova/+/950592 | |
| 18:42:55 | sean-k-mooney | gmaan: i didnt get as far as i hoped today but the first 10 patches in stephens sereise are still good to merge IMO i.e. https://review.opendev.org/c/openstack/nova/+/936365/8 -> https://review.opendev.org/c/openstack/nova/+/937048/11 | |
| 18:43:15 | sean-k-mooney | ill take a look at the next coupel of patches tomorrow | |
| 18:44:00 | gmaan | sean-k-mooney: ack, I will see if I can check those this week but next week I am planning. | |
| 18:44:50 | Callum027 | and hopefully get it merged for Flamingo. | |
| 18:44:50 | Callum027 | Hi sean-k-mooney, I added a comment to the change but I'd thought I'd let you know here as well - sorry for the inactivity on my changes, been a bit busy with work. We've deployed the changes to our production OpenStack environments and everything seems to be working great, so I'm happy to attend the next Nova meeting so we can discuss the proposal | |
| 18:45:48 | sean-k-mooney | Callum027: ack, if you cant just send a mail to the list asking for it to be approved | |
| 18:45:57 | sean-k-mooney | i hope its not contoverial | |
| 18:46:15 | sean-k-mooney | there is at least some other interest form teh teleemtry folks to enabel this with cloud kitty | |
| 18:46:26 | sean-k-mooney | so that posivie | |
| 18:47:09 | Callum027 | Yeah, I'm sure they'd be interested since this improves the scalability of Ceilometer polling | |
| 18:47:58 | Callum027 | I can understand why people wouldn't be happy about it since this is basically putting lipstick on a pig, but anything more radical would require more fundamental design changes and I'm not sure I'm capable of championing that at this stage :) | |
| 18:49:14 | sean-k-mooney | well with my nova hat on it replaceing Ceilometer pooling the nova api with Ceilometer polling the libvirt api so if its a perfroamc eissue its not someone elses probelem. on the ohter had the domain xml is ment to be internal state | |
| 18:49:29 | sean-k-mooney | so if we just stop using them or libvirt changed ot yaml | |
| 18:49:47 | sean-k-mooney | that not a breakign change form a nova perspecitve as the xml is not public | |
| 18:49:55 | sean-k-mooney | but realisticlly that not going to happen | |
| 18:50:09 | sean-k-mooney | so as long as ceilometer is readonly its proably a net win | |
| 18:58:56 | Callum027 | I do think it's a relatively elegant solution to the scalability issues Ceiilometer faced, with the drawback that changes to the files require all instances to be shelved-and-unshelved to apply the changes (or updated in place using virsh commands) | |
| 18:59:34 | sean-k-mooney | no it just requries a hard reboot | |
| 19:00:04 | sean-k-mooney | the xml is regenerated on every hard reboot or start/power on call | |
| 19:00:10 | Callum027 | Oh, right, of course | |
| 19:00:31 | sean-k-mooney | shelve would work too but there are cheaper options | |
| 19:00:33 | Callum027 | I should update the release notes on those commits to mention that | |
| 19:00:50 | Callum027 | But the main thing is live migrating doesn't work since the XML gets copied over without modificaiton | |
| 19:01:02 | sean-k-mooney | it will also get added on a live migrate i think | |
| 19:01:16 | Callum027 | In our testing it didn't, but we're using an older version of Nova | |
| 19:01:17 | sean-k-mooney | or at least it would if you implemnted that in your code change | |
| 19:01:43 | sean-k-mooney | well no i mean nova can do that but you need to write the code | |
| 19:01:59 | Callum027 | Maybe part of "standardising" this would be that we write a more elegant way of updating the metadata of instances in place | |
| 19:02:08 | sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/virt/libvirt/migration.py#L55-L97 | |
| 19:02:14 | Callum027 | For our production environment we wrote a one-off Ansible playbook to do it | |
| 19:02:28 | sean-k-mooney | i mean that fair too | |
| 19:02:37 | sean-k-mooney | although we dont like operators touching the xml | |
| 19:02:55 | Callum027 | Yeah, I definitely would have preferred to use the proper core for it | |
| 19:03:32 | sean-k-mooney | so perhaps as a follow up we could add updating of the metadata in the domain on live migrate | |
| 19:03:37 | sean-k-mooney | that not doen today in general | |