Monday, 2025-03-10

opendevreviewCallum Dickinson proposed openstack/nova master: Add image meta to libvirt XML metadata  https://review.opendev.org/c/openstack/nova/+/94276605:05
*** ralonsoh_ is now known as ralonsoh07:04
sean-k-mooneymelwitt: would you mind readding your +2 adn approving https://review.opendev.org/c/openstack/nova/+/939411/10 the only differnce since you last reviewd was the addtion of a release note11:15
sean-k-mooneybauzas: this ^ was a regression breaking rebuild last cycle so we will have to backport it anyway but still nice ot have before rc111:16
opendevreviewMerged openstack/nova master: libvirt: fix maxphysaddr passthrough dom parsing  https://review.opendev.org/c/openstack/nova/+/94237112:13
opendevreviewYaguang Tang proposed openstack/nova stable/2024.2: Fix device type when booting from ISO image  https://review.opendev.org/c/openstack/nova/+/94392412:16
bauzassean-k-mooney: ack, stephenfin already sent it to the gate fwiw13:19
bauzassean-k-mooney: I started a new etherpad for rc tracking https://etherpad.opendev.org/p/nova-epoxy-rc-potential13:19
bauzassean-k-mooney: you can as usual add some changes if you think those are important before RC1 in the right section 'bugfixes requiring attention before RC1'13:20
sean-k-mooneybauzas: ack, this is/was a latent issue form the ironic leeasee serise so not super urgent. i.e. if it missed rc 1 it woul dnot be a new regression intoduced in epoxy, just nice to have sooner rtahter then later13:32
sean-k-mooneystephenfin: thanks for reviewing :)13:33
sean-k-mooneybauzas: as it stand im not really aware of anyting super urgent for RC1 outside of the mechanical patches we do every release13:33
opendevreviewMerged openstack/nova master: api: project/tenant and user IDs are not UUIDs  https://review.opendev.org/c/openstack/nova/+/94036913:46
opendevreviewMerged openstack/nova master: api: Address TODO in microversion v2.99  https://review.opendev.org/c/openstack/nova/+/94325313:46
opendevreviewYaguang Tang proposed openstack/nova stable/2024.2: Handle iso+gpt detections  https://review.opendev.org/c/openstack/nova/+/94394013:54
opendevreviewMerged openstack/nova master: Fix parameter order in add_instance_info_to_node  https://review.opendev.org/c/openstack/nova/+/93941114:09
gibidansmith: o/ regarding https://review.opendev.org/c/openstack/nova/+/943816/1/nova/compute/pci_placement_translator.py14:10
gibithat palce exists in the pci in placement code to implement missing allocation healing and not intended to remain there for ever. It is more like a transitional code to support upgrade and gradual enablement of the featureh14:11
dansmithorly, okay14:11
gibiin general pci allocation should only happen during scheduling in the scheduler / conductor. 14:11
gibithis allocation healing is like the old healing periodic when we introduced placement14:12
gibi5+ years ago14:12
dansmithright, well, this is not really allocation and I think the consensus (me, sean, sylvain) is that the reservation should be in the compute node14:12
gibiI think you ended up looking at this code as friday in your env the allocation happened in this code14:12
gibiyeah I know that your code is not allocation 14:12
dansmithbut yeah I traced the placement update stuff looking for a good place to do it and this looked perfect :)14:12
gibibut the code path seem to only exists for allocation14:12
gibiso far14:12
dansmithI had originally written my own inventory update in the RT14:13
dansmithso I can go back to that approach, which it sounds like would be better so this healing can be ripped out later yeah?14:13
gibiI might be mistake. I have to dig a bit deeper where we do the actual inventory reporting of PCI in placement14:13
gibihttps://github.com/openstack/nova/blob/5f3133efc04dde15d64679e1e7564d2e575549db/nova/compute/resource_tracker.py#L1325-L134214:14
dansmithokay, tbh, this looked like it was intended to be permanent.. it seems related to making sure placement inventory gets healed if our resources ever changed14:14
gibiI'm reacting based on this comment14:14
dansmithokay, so the rest of update_to_placement() seems to intent to stay there long-term right?14:15
dansmithyou're just thinking the pci part is not permanent?14:16
gibihttps://github.com/openstack/nova/blob/5f3133efc04dde15d64679e1e7564d2e575549db/nova/compute/pci_placement_translator.py#L554-L556 OK this says both inventory and allocation handling so maybe this is both at the moment and then it means it will remain the same14:16
dansmithright, it's doing both14:16
dansmithgibi: this is the "don't do it here because..." feedback I wanted before going further, but if you need to dig in deeper, feel free to punt until you have time to do proper thinking14:17
gibiin short, there is allocation healing in that code path which will go away. There is inventory generation there which remains there. So extending the inventory generation with this feature might be OK.14:20
gibiso maybe it is OK14:20
dansmithokay14:20
gibiindependenty I'm not sure how racy this is with the pci_claim14:20
dansmiththe pci claim on the compute node happens before this14:21
gibiif this happens independently from the pci_claim in the periodic then we have a race window14:21
dansmiththe benefit here is that if you mark a device as OTU after it has already been allocated to an instance, it will be "healed" to reserved state14:21
dansmiths/the/a benefit/14:21
gibiI see14:21
dansmithI'm not sure what race you mean,14:21
dansmiththere's a small window where we could fail before we do the reserve, but if that happens, the instance will not boot14:22
dansmithso it's safe and the window is very small14:22
dansmithI don't think there's a race14:22
dansmiththis happens synchronously at the end of the instance_claim() method14:22
dansmith(*and* in the periodic)14:22
gibiI mean the pci claim claims an OTU device and returns to the compute manager spawn a VM and a periodic update_available_resouces runs after a while and sees the allocated OTU device and reserve it14:22
dansmithno,14:23
dansmithit happens synchronously14:23
gibiif this happens in the same code path as the claim then cool14:23
dansmiththis doesn't depend on the periodic running before it's reserved14:23
gibiOK14:23
gibithat was basically my second question14:23
dansmithruns right here in instance_claim() https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L21514:23
dansmiththat _update() all calls the _update_to_placement() as its first thing, which your first  link above 14:24
sean-k-mooney dansmith  from my perspective anything after self.pci_tracker.claim_instance in that function is ok, that is the earise that doing the reserved=total make sense14:25
dansmithif that all seems reasonable now, I can document all the above details in the spec more thoroughly so that review is easier14:25
dansmithsean-k-mooney: cool14:26
sean-k-mooneyalthought there are ways we can fail later so it could be later14:26
gibidansmith: OK I see it now, no race14:26
dansmithsean-k-mooney: I think there's always going to be some compromise, but it seems like when we're doing the pci accounting is the right balance to me14:26
sean-k-mooneydansmith: so what your propsoing i think will work for spawn and unshleve and evacuate14:30
sean-k-mooneyi suspect  self._update is reusted for cold migration/resize14:30
sean-k-mooneyya it is here https://github.com/openstack/nova/blob/276685b3db6e8f2ad59c33bc254461c255700ff8/nova/compute/resource_tracker.py#L37814:30
sean-k-mooneyso i think that is fine too14:31
sean-k-mooneylive migration is out os scope for this this cycle?14:31
sean-k-mooneylive migration does use a move claim but the claiming of the device in the nova db happens a littel earlier14:33
sean-k-mooneyi think its fine to still do the reserved=total in the same place, im just asking for the scope fo testing/support14:34
dansmithI'll track down the live case, but yeah the cold move case is also good14:34
sean-k-mooneyso for cold migration i think we just need to document that the device is burned once we have got to verify_resize14:35
dansmithI think I'm fine with live being out of scope here, especially since we can mark a device as live_migratable=false now, but there's no reason this shouldn't work the same I think, even if we have to do it in a slightly different place on the dest14:35
dansmithsean-k-mooney: yeah once you've touched both devices on either end, both are burned :)14:35
sean-k-mooneyexactly, some opt might assuem that happens at confirm14:36
sean-k-mooneyso we just need to document that14:36
opendevreviewMerged openstack/nova master: Update conductor and filters allowing migration with SR-IOV devices  https://review.opendev.org/c/openstack/nova/+/94214614:36
dansmithsure14:37
gibidansmith: yeah by default flavor based PCI devices are not live migratable, only exception is via the recently added live_migratable flag in the dev_spec and alias but we don't expect that will be set for OTU devices14:40
dansmithoh, okay well, that's even easier I guess14:40
sean-k-mooneyya the default helps14:41
gibionly neutron based PCI devs are live migrated by default via unplug and plug14:41
sean-k-mooneyright but we dont support them in placment yet14:41
gibiyes14:41
sean-k-mooneyso we can also ignor them14:41
gibiyes14:41
sean-k-mooneywe could add a check14:41
sean-k-mooneythat you dont set otu and physical_network14:41
sean-k-mooneysince we know thats invalid today14:41
sean-k-mooneybut its not strictly required14:42
opendevreviewGaudenz Steinlin proposed openstack/nova master: Make SSH compression for remote copy configurable  https://review.opendev.org/c/openstack/nova/+/94337714:42
dansmithOkay well, I'll add some notes to the spec for this to cover the above details14:44
opendevreviewSylvain Bauza proposed openstack/nova master: doc: mark the maximum microversion for 2025.1 Epoxy  https://review.opendev.org/c/openstack/nova/+/94394815:06
opendevreviewDan Smith proposed openstack/nova-specs master: Add one-time-use-devices spec  https://review.opendev.org/c/openstack/nova-specs/+/94348615:10
gibidansmith: left couple of comments but I think the direction looks good15:11
dansmithgibi: thanks!15:11
gibithe only think I'm a bit hesitant about it is the long term effect of the decision that we go with placement to solve this while and therefore we are forced not to support OTU VFs.15:12
dansmithgibi: tbh, I'm not really sure when VFs would need to use this, as I suspect any virt-knowing device would be sensitive to exposure between uses (perhaps not)15:13
dansmiththat said, I think that from the operator point of view, it doesn't necessarily matter that it's happening in placement (other than the steps to unreserve after cleaning I guess).. but the devspec wouldn't need to change if we started accounting for the one-time-use in, say, PCI tracker15:14
gibiyeah that is a valid point. This is why I'm not against going with the placement route, just hesitant that our udnerstanding of SRIOV VFs are correct15:14
gibidansmith: yeah the unreserve is the problematic step if we go with the PCI tracker route15:14
dansmithright15:14
gibinova has no API for that15:15
dansmithan SRIOV NIC is safe for use without cleaning, because you can't update firmware I think15:15
dansmithnot sure about a VGPU and memory exposure I guess, but .. they have to know that the point of that is sharing, so I would expect it's okay15:15
gibiI also think that, but I never checked what happens with GPU VFs or NVMe VFs15:15
dansmithan SRIOV NVMe would be a problem I guess, but if you're de- and re-allocating them they should be safe (like new namespaces on a regular one)15:16
gibitrue, if the VF is mapped to a namespace then it is OK15:17
dansmithyeah15:17
gibiOK. I'm less hesitant now. We have good reasons to believe that this feature is not needed for VFs15:17
gibia sane VF implementor needs to handle the sharing issue15:17
dansmithI think we just make sure it's clear in the doc that this is only for whole devices because they're not virt-aware, and we can also say "for more comprehensive situations you need cyborg anyway" :)15:17
dansmithyeah15:17
gibiOK15:18
gibiand :) for cyborg as a fallback15:18
dansmithI really don't want to blur the line of nova-vs-cyborg much, so I think keeping this limited to specific uses is helpful to avoid people trying to push nova to do more15:19
opendevreviewSylvain Bauza proposed openstack/nova master: Update compute rpc alias for epoxy  https://review.opendev.org/c/openstack/nova/+/94395215:19
sean-k-mooneydansmith: gibi  you cant update teh frimware vai the vf from an nic i have worked with15:19
dansmithright15:20
sean-k-mooneydansmith: you can flash ab bitrstream via a specific VF on an fpga that change the behvior of other VFs15:20
sean-k-mooneybut that a very different usecasue15:20
sean-k-mooneyto me that is not what you are creating this for15:20
dansmithdefinitely not15:20
sean-k-mooneyif we add pci groups in the future15:20
sean-k-mooneywe could extend the concpet to the group15:21
sean-k-mooneyi.e. a one time group 15:21
dansmithtbh, the pci-in-placement approach seems to me to be a bit of a stretch of the model, since placement really assumes resources of a given class are fungible and does not track actual assignment15:21
sean-k-mooneydansmith: well in general tehy are fungible15:22
dansmithso anything specific to a given VF probably needs a new model and new API (which cyborg is well-positioned to handle, IMHO)15:22
dansmithsean-k-mooney: VFs are, but not whole PCI devices15:22
sean-k-mooneymaybe you could argue it either way15:23
sean-k-mooneyi get where your coming form15:23
dansmiththe fact that we have a custom resource type for each device, even if its identical to another, with a total=1 is, I think, the stretch of the model15:23
sean-k-mooneyon the cyborg side do we watn to reach out to them during the ptg on any of these topics?15:23
dansmithwe're calling two devices that are identical but have different PCI addresses "different" :)15:24
gibiyeah we made the assumption in PCI in Placement that the same RC inventory on an RP means fungible devices15:24
sean-k-mooneydansmith: we are not15:24
sean-k-mooneythey have the saem resouce class if they have the same vendor_id and product id15:24
gibiand we opted in to represent a PF as RC=1 while VFs as RC=N15:24
sean-k-mooneythe reason they are diffent RPs is because fo VFs15:25
gibitwo RPs with the same RC=1 inventory are non fungible due to the RP name encoding the PCI address15:25
dansmithsean-k-mooney: okay, sorry, terminology.. same resource class, but separate providers..15:25
dansmithright ^15:25
sean-k-mooneygibi: yep and that because of other factors like numa or phsynet associativity15:26
sean-k-mooneyas it stands to day we could have an invetlory of PFs directly off the compute RP15:26
gibiyeah exactly numa and physnet granurality is also aligned with that15:26
sean-k-mooneythe reason we dont was to allow numa and physnets to be modleed in the future15:27
dansmithI guess it's not unlike if we had two storage providers on a host that you can't just arbitrarily sum together.. however, the inventory within those are still fungible, but it's not within a PCI provider, hence the only one inventory15:27
sean-k-mooneywe only need 1 RP per device in the VF case today15:27
sean-k-mooneydansmith: we could in some cases coalese multipel PCI invetories into one provider15:28
sean-k-mooneyit just seamed harder to have it sometiems on the root provider and sometimes on its own15:29
sean-k-mooneydepending on what you set in the devspec15:29
dansmithsean-k-mooney: I'm just saying, I think we're bordering on breaking/abusing the model... it still fits nicely, but only just15:29
sean-k-mooneyand what we discoverd (vendor/product id)15:29
dansmithand tbc, I'm not complaining :)15:30
sean-k-mooneydansmith: to b efair we have a design for numa in palcment and neutorn port in placment15:30
sean-k-mooneyim also hesitant to proceed with the numa part for similar reasons15:30
sean-k-mooneyi think part of the orginal selling point for palcemetn (moveing all or almost all the schduling work to it) didnt really pan out15:31
sean-k-mooneyif we are beign pargmatic and say the nova schduler will always be needed to do find grained choices liek numa15:31
dansmithI have no mental space to consider that now, but a numa node with fungible inventory inside is a closer fit to the original goal I think15:31
sean-k-mooneythen there is an argrument to say palcment should never know about it15:31
sean-k-mooneyit being numa15:32
sean-k-mooneyack lets leave it there for now15:32
dansmithI wonder if we could introduce another data type to placement that represents actual physical allocations where needed15:32
sean-k-mooneymaybe15:32
dansmithsomething that ties an allocation uuid to that other thing that represents an actual thing with an address15:32
sean-k-mooneyim happy to disucss that at some point just also dont have the brain power to context switch to this in detail right now15:33
gibilets add more metadata :)15:33
dansmithmemory has no address or serial number, but a pci device does, a disk might15:33
sean-k-mooneymemory kind of does15:33
dansmithgibi: it would only be good if we can drop something else in favor, of course :)15:33
sean-k-mooneyits at least got a numa node 15:33
sean-k-mooneyit might not be a unique idetifier across all host15:34
sean-k-mooneybut there are ways to talk about fungible pools of it15:34
dansmithsean-k-mooney: if the numa node is a nested provider, then the allocation within has no (further) addressing15:34
dansmithright15:34
dansmithanyway, we're way off in the weeds, didn't mean to start digging a hole15:34
sean-k-mooneyhttps://collection.nam.ac.uk/images/960/204000-204999/204324.jpg15:35
dansmithheh yeah15:36
sean-k-mooneythats not actuly the meme i was loogkign for but it will do15:36
gibi:)15:36
bauzasfolks I just saw your discussion, will review again dansmith's spec tomorrow15:50
opendevreviewMasahito Muroi proposed openstack/nova master: Use dict object for request_specs_dict in the _list_view  https://review.opendev.org/c/openstack/nova/+/93965816:52
masahitosean-k-mooney: gibi: please review this api bug fix again https://review.opendev.org/c/openstack/nova/+/939658  thank you.16:54
gmannsean-k-mooney: do you remember the some missing package for pcre.h in bindep.txt for doc job running on noble. this one https://review.opendev.org/c/openstack/nova/+/93557718:25
sean-k-mooneyyes18:25
sean-k-mooneydidnt htat go away?18:25
sean-k-mooneyor is that still a thing18:25
gmannsean-k-mooney: no, I tired some but still error18:25
sean-k-mooneywhat job was failing18:26
gmannlibpcre3-dev should include it but that is already in bindep file18:26
sean-k-mooneydocs and apiref it seams18:26
sean-k-mooneyi thinki have noble somewhere18:26
gmannopenstack-tox-docs this one https://zuul.opendev.org/t/openstack/build/a1a1fbda6cf24246a3ed9912c5e952b318:26
gmannyeah18:26
gmannI am not able to produce it locally I think I installed some extra package there18:27
sean-k-mooneyif you install devstack on the know it fixes the porblem if i recall18:28
sean-k-mooneyim runnign the docs target localy now18:29
sean-k-mooneyill check where that header came form after18:29
sean-k-mooneygmann: oh you kno what it might be that if you have to build the wheel form source it fails but if you can get it form the wheel cache ti works18:31
sean-k-mooneythe ci job is trying to build the wheel for python-pcre18:31
gmannyeah18:34
sean-k-mooneyubuntu@sean-devstack-watcher-1:~/repos/nova-docs$ find /usr -name "pcre.h"18:36
sean-k-mooney/usr/include/pcre.h18:36
sean-k-mooneyubuntu@sean-devstack-watcher-1:~/repos/nova-docs$ apt-file search /usr/include/pcre.h18:36
sean-k-mooneylibpcre3-dev: /usr/include/pcre.h  18:37
sean-k-mooneygmann: so no noble https://review.opendev.org/c/openstack/nova/+/935577/3/bindep.txt is not correct18:37
sean-k-mooneyline 53 should be all that is required18:37
sean-k-mooneylibpcre3-dev [platform:dpkg test]18:37
gmannyeah, libpcre3-dev should be enough but it is not finding it when building wheel for python-pcre18:39
sean-k-mooneyi can try doing it in a clean vm and see what happens18:39
gmannor you can see PS2 test run and it fails there too https://review.opendev.org/c/openstack/nova/+/935577/218:40
gmannbut log expire there18:40
gmannexpired 18:40
gmannoh, its there https://zuul.opendev.org/t/openstack/build/5ddcd6d9f0a94008a722346992cf499b18:40
sean-k-mooney ya so this sound like somethign to do with the build chain18:42
sean-k-mooneyand by that i mean how the toll chain is set up in the vm18:42
sean-k-mooneylike its missing one of the devel packages like make or something is missing18:43
sean-k-mooneyi dont think this is it but one thing i do notice is i have a slightly old verion of virutal env locally then it used in the failed buil18:47
sean-k-mooneyvirtualenv = 20.26.3 locally18:47
sean-k-mooneygmann: so not a clean vm but https://paste.opendev.org/show/bv240D3BK3XVEKBywbMN/18:58
sean-k-mooneyif i disable using all binaireas and force a build it definly works 18:58
sean-k-mooneygmann: ok i create a clean ubuntu vm on our internal cloud. im gong to confirm pcre.h is not pre installed in the image and then only install the bindep package with the test profile19:04
sean-k-mooneyand ill see if mater doc target fail or not19:04
sean-k-mooney... cloning nova is painful aat 500KiB/s19:06
sean-k-mooneygmann: so this is what im going to install19:12
sean-k-mooney.venv) ubuntu@sean-upstream-docs:~/nova$ bindep -b test | nc termbin.com 999919:12
sean-k-mooneyhttps://termbin.com/hrrp19:12
sean-k-mooneythat is for master19:12
sean-k-mooneyi guess i should also include the docs target19:12
sean-k-mooneygmann: so locally that works for me on a clean vm19:23
sean-k-mooneygmann: what i did was as followes19:23
sean-k-mooney sudo apt install python3.12-venv19:23
sean-k-mooneypython3 -m venv .venv19:24
sean-k-mooneyactully ill past it19:24
sean-k-mooneyhttps://paste.opendev.org/show/bq494eyZBhcRJTR1ATEm/19:25
sean-k-mooneythis seams to be somethign either specific to the ci vms or to the playbooks we are using to do this in ci19:27
gmannyeah, it seems so. something wit editable install with pyproject.toml ? but not sure19:29
opendevreviewGhanshyam proposed openstack/nova master: Testing doc job on Ubuntu Noble  https://review.opendev.org/c/openstack/nova/+/93557719:29
opendevreviewMerged openstack/nova master: Update libvirt fixtures to support hostdevs  https://review.opendev.org/c/openstack/nova/+/94320720:12
opendevreviewMerged openstack/nova master: Update driver to map the targeted address for SR-IOV PCI devices  https://review.opendev.org/c/openstack/nova/+/94214720:20

Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!