Earlier  
Posted Nick Remark
#openstack-nova - 2025-03-14
08:02:33 gokhan frickler, nova version is 29.2.0 and ceph is 18.2.2. But in my other deployments which has same version, DISK_GB on every hosts is correct.
08:06:41 gokhan frickler, ohh sorry ceph versions sre different. İt is working with ceph 18.2.4 but with ceph 18.2.2 it is not working. may be there is a known issue
10:50:20 ishanwar Hi Everyone!
10:50:38 ishanwar I am planning to take a shot at this LHF https://bugs.launchpad.net/nova/+bug/2027156
10:51:15 ishanwar Can someone help me understand why we invented to use to the neutron quota API here ? Please dont mind me question I am new to nova dev
10:51:32 ishanwar intend
10:52:02 ishanwar @Sean-k-mooney @bauzas
10:59:47 sean-k-mooney ishanwar: am short version is today we are pulling the full port object with all the extentions and then counting them in python
11:00:08 sean-k-mooney to see if we exceed the quota the project has for total ports
11:00:22 sean-k-mooney but neutron has an api that just returns this as a number
11:00:45 sean-k-mooney so its much faster to use the dedciated api and less work for both nova and neutron if we do
11:01:14 sean-k-mooney so this is just a performance optimisation ot reduce the amount of data sent over the wire and reduce the database load on the neutron side
11:02:33 sean-k-mooney ishanwar: so the idea is, 1 try and use the neutron api exteion for quota details, if that works great we can skip the extra work of counting the ports ourselves, if not fall back to what we do today
11:06:09 ishanwar Hi Sean
11:06:21 ishanwar Thanks for your response
11:06:51 ishanwar The big mentions that the quota api seems to be controlled by Also, the extension seems to be controlled by the cfg.CONF.QUOTAS.quota_driver config option: https://github.com/openstack/neutron/blob/6fa9142ced0eed8de9a89042747438b93de7202d/neutron/extensions/quotasv2_detail.py#L37
11:07:19 ishanwar So all neutron servers may not have this, how can we check the availability of the api ?
11:11:12 sean-k-mooney there are two way, we can assuem its there and just catch the excption when not and fall back
11:11:20 sean-k-mooney or we can call the extention list api
11:11:59 sean-k-mooney https://docs.openstack.org/api-ref/network/v2/#show-extension-details
11:12:28 sean-k-mooney using the extension list api is probaly better. in either case we have a convention of caching that result in nova
11:13:31 sean-k-mooney like this https://github.com/openstack/nova/blob/master/nova/network/neutron.py#L1435-L1455
11:39:07 ishanwar thanks let me try to gather more details before create a gerrit review request
13:28:56 opendevreview mitya-eremeev-2 proposed openstack/nova master: Add more thorough handling of evacuated instances https://review.opendev.org/c/openstack/nova/+/944058
13:39:28 dansmith gibi: I don't understand your response to my comment
13:41:06 dansmith _find_pool() is not where things get sorted into pools, right? It looks through the already-sorted pools and finds the one that _dev_pool (which I think is just the device, right?) matches?
13:41:14 opendevreview mitya-eremeev-2 proposed openstack/nova master: Remove instance directory on live-migration https://review.opendev.org/c/openstack/nova/+/944058
13:47:26 gibi dansmith: yes, find_pool tries to find a matching pool, the callers of _find_pool creating a logic that if a pool is not found for a dev then a new pool is created for the dev
13:48:02 gibi but as matching logic was broken in _find_pool the upper layer created new pool for the devs
13:48:05 gibi during deallocation
13:48:21 dansmith ohhh, I see okay, let me follow that thread
13:49:00 dansmith since you're changing it and the allocation part I was mostly linking those in my head (and because _find_pool is almost like _filter_pools)
13:50:13 dansmith so if I comment out the _find_pool() part, I fail this part of the test:
13:50:22 gibi yeah the two changed code path are only linked from afar. the _find_pool runs during new device discovery and device deallocatin (putting back to pool). The _apply runs during scheduling of a VM, the consume codepath runs during the pci claim from the pci tracker in the compute
13:50:24 dansmith # assert that the single pool is not broken into two during the de-allocation
13:50:33 dansmith which makes sense based on what you said
13:50:34 gibi yeas
13:51:28 gibi and the _find_pool fix is the only thing that is necessary, the other code change is just safetynet
13:54:43 dansmith gibi: okay and the place where we do that (add a pool if we don't find it) is in add_device()..
13:54:55 dansmith it doesn't look like we run remove_device() when we consume it from the pool or anything,
13:55:19 dansmith so I'm not sure why we're re-running add_device() there, except maybe just as idempotentcy/healing or something?
13:56:46 gibi I think add_device is used both during new device discover at startup and during deallocation when we place the device back to the pool
13:57:09 dansmith I see it used in consume_requests() but the only other place is update_device() which isn't called from here at all
13:57:40 dansmith anyway, it doesn't matter that much because I see the problem now, it just seems confusing to me
13:58:13 dansmith however, at least it's consistent with all the other confusing terminology in here (devs, pools, dev_pools, stats, all of which are kinda the same :)
13:59:06 gibi _allocate_devs pops the device from the pool
13:59:17 dansmith we also call add_device() from _free_device() in compute manager while we're deallocating
13:59:18 gibi during consume_request
13:59:41 dansmith oh, it doesn't call remove_device() but it removes it some other way?
13:59:46 gibi yeah
13:59:48 gibi :/
14:00:14 sean-k-mooney the way we test in teh schduler if a host passes is we call consume_requests on a copy of the pools and just dont save it from Supprots_requests()
14:00:21 gibi literarily pci_dev = pool['devices'].pop()
14:00:27 dansmith ack
14:00:48 dansmith okay so the add_device is adding it back in after it's no longer used I guess instead of just being marked as in-use...
14:00:49 gibi sean-k-mooney: nope, we call _apply, consume is for the pci tracker
14:01:04 gibi dansmith: yes that is my understanding
14:01:32 sean-k-mooney i may have mixed up the names but the point i wanted to make is, we test if we can allcoate the device by trying to do it on a copy of the data)
14:01:52 gibi sort of yes
14:03:03 gibi even in the scheduler (host manager) we do an allocation on the stats via apply to allow mutlicreate to see a proper resource availability
14:03:47 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L779-L821
14:04:01 sean-k-mooney yep
14:05:48 gibi bah we have 3 entry point not just two:
14:06:17 gibi 1. filters do supports_requests https://github.com/openstack/nova/blob/master/nova/scheduler/filters/pci_passthrough_filter.py#L62
14:06:29 gibi 2. host manager do apply_request https://github.com/openstack/nova/blob/master/nova/scheduler/host_manager.py#L313
14:06:47 gibi 3. pci_tracker do cosume_request https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L311
14:07:27 gibi but you are right 1 is basically 2 on a copy of the pools
14:09:27 sean-k-mooney that not all of them
14:09:35 sean-k-mooney it kind of is but
14:09:42 sean-k-mooney https://github.com/openstack/nova/blob/master/nova/scheduler/filters/numa_topology_filter.py#L105-L112
14:10:08 sean-k-mooney numa_fit_instance_to_host also takes into acocunt the pci requests
14:10:38 gibi yeah that is 1 with extras, it calls support_request and look into the pools as well
14:11:40 sean-k-mooney ya so supprots_request is the entry point from the filters effectivly
14:11:44 gibi this is not for the faint-hearthed
14:12:40 dansmith yup
14:14:11 sean-k-mooney i could have sworn that we had factor the common logic out by the way
14:14:27 sean-k-mooney i.e. that supprot called consume or that support and consume both called apply
14:14:41 sean-k-mooney i dont recall why that does nto seam to be the case
14:15:18 gibi it needs some divergence as consume needs to return a specific device while apply only decrase pool count
14:15:30 sean-k-mooney oh right
14:15:37 sean-k-mooney consume is what we run on the compute
14:15:58 gibi yepp
14:16:01 sean-k-mooney its actully operating not just on the ppoos but the pci_device records and selecting the device
14:16:52 dansmith gibi: another thing that seems messy is that we take a thing, which is a dict'ing of an object, and pop out things that we ignore, and then see if "everything else is the same" right?
14:17:32 dansmith seems like we should opt-in to the things that distinguish them so that new things that get added in later don't cause fracturing of the pools if you don't also update the exclusion list
14:17:36 dansmith (or just do this differently)
14:18:26 sean-k-mooney that would be more maintianable wehn this was first writtin jay was pretty again adding new tag in the future
14:18:33 sean-k-mooney since then we have added several
14:18:47 sean-k-mooney btu never really revisited that orgianl approch
14:19:48 gibi dansmith: I think the concept is that a pool is a set of devs with a set of common field values. So it is the defintion of the interesting fields is the question here
14:20:10 sean-k-mooney gibi: yep
14:20:26 gibi and yes I share you feeling that ignoring flags is less maintainable than listing interesting flags
14:20:54 dansmith right
14:21:02 gibi I'm not motivated to change this logic in a bugfix though. I can queue up a refactor after it.
14:21:04 sean-k-mooney and this is what i mean when i say tis placement like a pool is effectivly an rp with and inventory fo common "fungable" dvices
14:22:06 gibi I would even wrap these dev and pool dicts in proper objects in memory (not ovos) for more readability (and mypy)
14:23:20 gibi I'm bit affraid of what can of worms this opens up
14:24:12 gibi and I have the ovo version bump preemptive unit test and refactor series to get back to as well
14:24:24 gibi I need more time for actually writing code
14:24:27 dansmith gibi: no I'm not suggesting you fix it in this bugfix or even later, I'm just lamenting the design

Earlier   Later