| sean-k-mooney[m] | melwitt: sorry didnt realise that enjoy your time off | 03:52 |
|---|---|---|
| auniyal__ | Hi O/ | 05:26 |
| auniyal__ | please review these | 05:26 |
| auniyal__ | https://review.opendev.org/c/openstack/nova/+/853811 | 05:26 |
| auniyal__ | https://review.opendev.org/c/openstack/nova/+/853812 | 05:26 |
| crohmann | Hey lovely nova folks. I was just about to raise a bug about duplicate indices for tables of Nova and Placement, but then found an old, but unfixed bug: https://bugs.launchpad.net/nova/+bug/1641185 | 06:46 |
| crohmann | Since this is already assigned to ABHAY (since 2018) I believe this might be under the radar. Any chance this could be reassigned or place onto the list of "open isuses" ? | 06:47 |
| crohmann | This also appears to have a simple fix in removing the double definitions of colums as primary indexes as well as them having a unique constraint. | 06:52 |
| gibi | crohmann: hi! thanks for checking before reporting a new bug. Do you plan to proposa a fix? | 07:21 |
| gibi | if so, then feel free to reassing the bug | 07:22 |
| gibi | (or I can reassing it to you if you don't have the rights) | 07:22 |
| crohmann | gibi: I did assign me and also raised a bug for placement at https://storyboard.openstack.org/#!/story/2010251. | 07:39 |
| crohmann | Regarding a fix ... I suppose there are two sides: Fixing the schema for new installs, but also dropping them for existing ones, right? | 07:40 |
| crohmann | "them" = the duplicate index | 07:41 |
| gibi | crohmann: you are correct | 07:44 |
| gibi | you need to drop it from the schema and also propose a db schema migration to drop it from existing dbs during upgrade | 07:45 |
| opendevreview | Gorka Eguileor proposed openstack/nova master: Support os-brick specific lock_path https://review.opendev.org/c/openstack/nova/+/849328 | 09:04 |
| opendevreview | Gorka Eguileor proposed openstack/nova master: Support os-brick specific lock_path https://review.opendev.org/c/openstack/nova/+/849328 | 09:08 |
| opendevreview | Amit Uniyal proposed openstack/nova master: Adds check for VM snapshot fail while quiesce https://review.opendev.org/c/openstack/nova/+/852171 | 09:11 |
| sean-k-mooney | the unique key is the one that should be kept | 09:15 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support move operations with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | 09:19 |
| opendevreview | ribaudr proposed openstack/nova master: Default Nova persistent objects without soft delete. https://review.opendev.org/c/openstack/nova/+/854355 | 09:20 |
| sean-k-mooney | gibi: since the update userdata feature will need a new trait and a new os-traits reelase i think we shoudl swap the microverions for that and rebuild | 09:23 |
| Uggla | gibi, I did that change ^ then I want to modify act objects like aggregate that do not requires soft delete. Unfortunately it changes the API output. So I guess we need a new microversion at minimum. But do we need a "deprecation" cycle too ? | 09:23 |
| sean-k-mooney | Uggla: waht are you chagining | 09:24 |
| gibi | sean-k-mooney: we have "time" until friday to push an os-traits change. But if the rebuild series is ready then I have no objection to swap | 09:24 |
| sean-k-mooney | i have not reviewed it so i cant say | 09:25 |
| Uggla | sean-k-mooney, https://review.opendev.org/c/openstack/nova/+/854355 trying to make persistent objects without soft delete "by default" | 09:25 |
| gibi | Uggla: let me look at it. For sean-k-mooney, the new ShareMapping object needs to be non-soft-deletable and that needs some new baseclass for Nova ovos as the current one adds the deleted_at field | 09:25 |
| sean-k-mooney | right but we dont need to change all the others | 09:26 |
| gibi | sean-k-mooney: re rebuild series: me neither so for me both userdata and rebuild is in the grey zone but if somebody says that rebuild is ready to land then I'm OK to swap | 09:26 |
| sean-k-mooney | we could eventually | 09:26 |
| gibi | sean-k-mooney: re ovo: yes, we only need to change the base class for the new ShareMapping | 09:26 |
| sean-k-mooney | right so https://review.opendev.org/c/openstack/nova/+/854355/1/nova/objects/base.py#142 is wrong | 09:27 |
| sean-k-mooney | we shoudl not modify the NovaPersistentObject | 09:27 |
| sean-k-mooney | we shoudl leage that the same and add a seperate one that does not use soft delete | 09:28 |
| gibi | ^^ agree | 09:28 |
| sean-k-mooney | maybe call it NovaPersistentObjectHardDelete for now | 09:29 |
| gibi | wondering that the problem is only that the base class change changes the object signature used for versioning | 09:30 |
| sean-k-mooney | it will change the ovo shas | 09:30 |
| Uggla | sean-k-mooney, gibi it means we will keep objects with the removal of delete deleted_at "tricks" forever ? | 09:30 |
| sean-k-mooney | but if Uggla updated them all to point to NovaPersistentSoftDeleteObject it woudl be fine | 09:30 |
| sean-k-mooney | Uggla: if we remove soft delete it will be a sperrate spec | 09:31 |
| Uggla | sean-k-mooney, no the current changes does not change the shas | 09:31 |
| sean-k-mooney | its not something you should do in your current one | 09:31 |
| gibi | Uggla: then what is the exact API change you are worried about? | 09:33 |
| gibi | Uggla: I don't see the reason why the aggregate API output would differ | 09:34 |
| gibi | after your patch | 09:35 |
| gibi | ohh | 09:35 |
| Uggla | gibi, in another patch I wanted to clean the aggregate object | 09:35 |
| gibi | so the aggregate is not soft deleted | 09:35 |
| Uggla | yep | 09:35 |
| gibi | leave that alone for now :) | 09:35 |
| gibi | in your feature you add a new ovo which is not soft deletable, that is good | 09:36 |
| gibi | then in AA if you want, you can work on bumping the Aggregate OVO to 2.0 and add a microversion to hide deleted_at | 09:36 |
| Uggla | so I change it as a test to inherit from NovaPersistantObject (without soft delete) | 09:36 |
| Uggla | gibi but is it something that could be done in AA or later to have a "deprecation" time ? | 09:38 |
| gibi | there is multiple things. The API change needs a microversion bump, it does not need a deprecation period but we need to support old microversions( you can fake deleted_at in the API response for old microversions) | 09:39 |
| gibi | the Aggregate OVO change needs a major ovo version bump to 2.0 to remove a field | 09:39 |
| sean-k-mooney | same for all other objects | 09:39 |
| gibi | that means if we pass Aggregate OVOs over RPC then we need to support Aggregate 1.x and 2.0 parallel for an extra release | 09:40 |
| sean-k-mooney | we cant do the db contraction till CC | 09:40 |
| gibi | yes, that is the 3rd thing | 09:40 |
| sean-k-mooney | because of teh new life cycle | 09:40 |
| gibi | but the Aggregate table has no deleted_at so no need to contract there | 09:41 |
| sean-k-mooney | well technially we can do it in BB if we deperecate teh soft delete feature in AA | 09:41 |
| Uggla | yes so do you think it worth it ? | 09:41 |
| sean-k-mooney | yes long term but i can upgrade my -1 to a -2 if you like | 09:41 |
| sean-k-mooney | you should not be doing this as part of the share change | 09:41 |
| sean-k-mooney | its its own spereate thing | 09:42 |
| Uggla | sean-k-mooney, no this is outside of the share stuff. | 09:42 |
| sean-k-mooney | right it needs a spec | 09:42 |
| gibi | Uggla: it depends. I think the important part is not to add new ovos with soft delete. Cleaning up the old ones is a bit of busy work in my eyes | 09:42 |
| sean-k-mooney | it has a prerrty large upgrad impact so it cant reasonable be done as a bugfix or specless blueprint | 09:43 |
| sean-k-mooney | gibi: this kind of need to be done in tandom with removign shadow tables for it to be useful | 09:43 |
| gibi | yes I agree to draft a spec for it if you want to work on this | 09:43 |
| gibi | sean-k-mooney: good point | 09:43 |
| Uggla | gibi, agree I don't want to work on something if you think the benefits are low. | 09:44 |
| sean-k-mooney | personally removing the shaddow tabels woudl be more useful in my view then the soft delete mechanisum | 09:44 |
| sean-k-mooney | Uggla: its not that its low its that there is a lot more work to do then just that patch | 09:44 |
| gibi | Uggla: you can bring this up on the PTG to get a wider set of opinions about it | 09:45 |
| sean-k-mooney | Uggla: its quite a large change to do right and ensure we dotn break rooling upgrades | 09:45 |
| Uggla | sean-k-mooney, agree I know that changing that is a lot of work due to the API impact etc... But from your point of view is it something that "bring value" ? | 09:47 |
| sean-k-mooney | by the way if an object suppots soft delele is determin by if it has the softDeleteMixin in the db model | 09:47 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L139 | 09:47 |
| sean-k-mooney | not the ovo | 09:47 |
| sean-k-mooney | Uggla: its something i have wanted to do for a few releases | 09:47 |
| sean-k-mooney | basically i want to remove or custom db backup/audit facilaties sicne operator have written there own | 09:48 |
| sean-k-mooney | nova i think is the only project with shadow tabels so they wroge a solution that worked for all of them | 09:48 |
| sean-k-mooney | https://github.com/ovh/osarchiver/ | 09:50 |
| sean-k-mooney | ^ | 09:50 |
| sean-k-mooney | i woudl like to replace or in tree shadow tabels with that solution eventually | 09:51 |
| sean-k-mooney | i would propose formally deprecating Shadow tabels in AA and remove them in BB | 09:52 |
| sean-k-mooney | soft delete we might want to keep becuase you can undelete instnaces | 09:52 |
| sean-k-mooney | if the soft delete functionlatiy is enbaled itn the config | 09:52 |
| sean-k-mooney | Uggla: gibi by the way we do not support fog deleteing for the api db resouces | 09:56 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/api/models.py#L61-L95 | 09:56 |
| sean-k-mooney | we do for all the cell db resouce excpt server tags and console auth tokens | 09:56 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/db/main/models.py#L1023-L1073 | 09:57 |
| gibi | yep I noted that above that the aggregate db table has not deleted_at column so no need to contract | 09:57 |
| sean-k-mooney | so for the share mappign we dont actully need a new ovo | 09:57 |
| sean-k-mooney | we can just not add the soft delete mixing to the table in the db model | 09:58 |
| sean-k-mooney | Uggla: https://github.com/openstack/oslo.db/blob/master/oslo_db/sqlalchemy/models.py#L129-L137 is where soft delete is implemented | 09:59 |
| gibi | the ovo base class adds deleted_at column today | 10:00 |
| gibi | to the ovo | 10:00 |
| sean-k-mooney | to the object sent over the wire | 10:00 |
| sean-k-mooney | but that does not mean it needs to be in the api reponce or the db model | 10:00 |
| sean-k-mooney | we can just ignore them. if we want to have a new base class that does not have them that is also ok | 10:01 |
| gibi | my original comment on Uggla's patch was to not hack out the deleted_at column from the ShareMapping ovo but not add it in the first place | 10:01 |
| sean-k-mooney | ya im fine with that too | 10:01 |
| gibi | https://review.opendev.org/c/openstack/nova/+/839401/6/nova/objects/share_mapping.py#49 | 10:02 |
| sean-k-mooney | so they dont need to do that | 10:02 |
| sean-k-mooney | if they just dont defien the columns in the db model | 10:03 |
| gibi | honestly I don't want to to have columns in a new ovo that are not used | 10:03 |
| sean-k-mooney | ack | 10:04 |
| sean-k-mooney | we have that alredy for many object | 10:04 |
| gibi | yes, but don't increase that debt if not really necesary | 10:04 |
| sean-k-mooney | sure | 10:04 |
| sean-k-mooney | so i think we are agreed Uggla should add one new base class without those fields | 10:05 |
| sean-k-mooney | and not touch any of the rest | 10:05 |
| sean-k-mooney | and we can disucss at the ptg if we will have time to do object cleanup in AA or BB and the faith of shadow tables | 10:05 |
| gibi | agreed | 10:05 |
| gibi | :) | 10:05 |
| Uggla | sounds good to me. thanks. | 10:06 |
| gibi | sean-k-mooney: re pci: I have to retract my original statement about nova splitting pools per PF. It splitted in my original test due to numa node differences (the fixture creates numa automatically). So on the same numa node we create pools with multiple PFs (of PCI devs). But as I already confirmed that the scheduler logic can split a request over multiple pools I can (and will) cahnge the pooling | 10:08 |
| gibi | logic not to merge pools from different PFs or PCI devs | 10:08 |
| sean-k-mooney | ack | 10:09 |
| gibi | I realized that when I created a host with 3 PCI devs. so numa0 got two | 10:09 |
| sean-k-mooney | ah | 10:09 |
| sean-k-mooney | ya so i know it did it for numa | 10:09 |
| sean-k-mooney | but within a numa node i was not sure | 10:10 |
| sean-k-mooney | i think it also splits for differnnt tags | 10:10 |
| sean-k-mooney | i.e. physnets | 10:10 |
| sean-k-mooney | or tursted vs non trusted | 10:10 |
| gibi | yes I assumed so, what I did not relaized that the fixture automatically split devices equally between numa0 and numa1 | 10:10 |
| sean-k-mooney | but i guess it combines if they are the same | 10:10 |
| sean-k-mooney | ah yes it does | 10:10 |
| sean-k-mooney | you can just create the device manually but the fixture i generally nicer to use | 10:11 |
| gibi | yep | 10:11 |
| gibi | basic cold migrate and resize works with miniumal changes in the conductor so I think evac and unshelve will be easy too | 10:13 |
| gibi | and live migration is not supported for flavor based PCI so that is super easy :D | 10:13 |
| gibi | then I will look at resize revert, reschedule, and multi create in this order | 10:14 |
| * gibi really glad to have the func test env to verify all these scenarios easily | 10:15 | |
| sean-k-mooney | lol yep live migration should be trivial :P | 10:15 |
| sean-k-mooney | fortunetly you can also verify them on real hardware for a change too once its working in the func test env | 10:15 |
| gibi | yes I will do a final round in the lab too | 10:15 |
| sean-k-mooney | i still need to go extend the reservation fo those nodes | 10:16 |
| sean-k-mooney | ill do that now | 10:16 |
| gibi | thanks | 10:16 |
| gibi | I use those nodes to run the func and unit tests in bluk too as it is a lot faster there :D | 10:16 |
| gibi | I rsync up my local dev repo and run tox via ssh | 10:17 |
| sean-k-mooney | yep thats why i often do dev on my home server | 10:17 |
| opendevreview | ribaudr proposed openstack/nova master: Default Nova persistent objects without soft delete. https://review.opendev.org/c/openstack/nova/+/854355 | 10:17 |
| sean-k-mooney | its avaiabel until 02-Nov-2022 but that seem longer then we need any prefered end date | 10:18 |
| sean-k-mooney | we can return them early too | 10:18 |
| sean-k-mooney | will i extend it to october 1st? | 10:18 |
| sean-k-mooney | that gives us a month of grace period | 10:18 |
| gibi | oct 1 is totally ok | 10:19 |
| sean-k-mooney | ok done | 10:20 |
| gibi | thanks | 10:20 |
| gibi | OK, so pools are splitted on these keys https://github.com/openstack/nova/blob/master/nova/pci/stats.py#L66 and I need to actaully check parent_addr there as well | 10:21 |
| gibi | if the parent_addr is None or not equal betweent two devs then we need to split | 10:22 |
| sean-k-mooney | yep that makes sense | 10:22 |
| sean-k-mooney | although hum | 10:23 |
| sean-k-mooney | we shoudl be spliting on more then that | 10:23 |
| sean-k-mooney | like phsynet | 10:23 |
| gibi | yes for neutron based sriov we might need that too | 10:23 |
| sean-k-mooney | can you add that while your adding the partent adress | 10:24 |
| sean-k-mooney | im wondering about trusted too | 10:24 |
| sean-k-mooney | i dont know if we need to split based on that | 10:24 |
| gibi | wait | 10:24 |
| gibi | I have to correct myself | 10:25 |
| gibi | https://github.com/openstack/nova/blob/94065763d32287606895c07bd5882bab083a4e48/nova/pci/stats.py#L136-L138 | 10:25 |
| gibi | we split on both those dev fields and the devspec tags | 10:25 |
| gibi | to physnet is handled | 10:25 |
| sean-k-mooney | ah ok as is trusted | 10:26 |
| * gibi learned a lot about the PCI code in nova in the last couple of weeks | 10:26 | |
| sean-k-mooney | so this will auto split on traits and resouce class | 10:26 |
| gibi | hehe :) | 10:26 |
| sean-k-mooney | technially we allow operator provided tags too but they were never uable for anything | 10:26 |
| sean-k-mooney | we just ignore them | 10:26 |
| sean-k-mooney | at one point there was talk of allowign the pci alias to match on extra tags | 10:27 |
| gibi | I had to ignore traits and resource_class https://review.opendev.org/c/openstack/nova/+/853316/4/nova/pci/stats.py to keep the pool matching work | 10:27 |
| sean-k-mooney | ah ok hehe | 10:28 |
| gibi | it is mostly ther to keep _filter_pools_for_spec happy as the request contains the traits tag but the pool will not mapped to traits just to RPs | 10:29 |
| sean-k-mooney | ya thats proably fine | 10:29 |
| sean-k-mooney | will we have the tags in the pools | 10:31 |
| sean-k-mooney | *traits | 10:31 |
| sean-k-mooney | i assume the intent is just to relay on placment to do the trait/rc filtering | 10:31 |
| sean-k-mooney | and then we use the rp id to corralate the pools with the allcoation candaate | 10:32 |
| sean-k-mooney | so we dont need to check them in the filters | 10:32 |
| sean-k-mooney | so we can just not put them in the pools | 10:32 |
| gibi | we use the rp_uuid to correlate the request with the pool | 10:33 |
| gibi | the rest is doen in placemnet | 10:33 |
| sean-k-mooney | yep that is what i was expecting | 10:34 |
| gibi | the pooling logic uses all the dev_spec tags automatically so I needed to explicity ignore traits and resource_class there | 10:34 |
| gibi | to not to put them into the pool | 10:34 |
| gibi | as we don't need them | 10:34 |
| gibi | and it also won't match with the request in generic way | 10:35 |
| sean-k-mooney | so your going to do two change right. 1 split the pools now also by parent adress and 2 split the alias requests into multipel instance_pci_request object if the alias request more then one of something | 10:35 |
| gibi | yes | 10:35 |
| gibi | the later is already up | 10:35 |
| gibi | I doing the former now | 10:35 |
| gibi | https://review.opendev.org/c/openstack/nova/+/852771/6/nova/objects/request_spec.py#540 | 10:35 |
| gibi | this is the request splitting ^^ | 10:36 |
| sean-k-mooney | cool ill try and review more of the seriese today | 10:37 |
| gibi | thanks | 10:37 |
| sean-k-mooney | i have some coments on the ones i reviewd but im +2 on all the ones i have reviewed so far | 10:37 |
| gibi | I will go through your comment | 10:38 |
| sean-k-mooney | i had a littele bit of consern with https://review.opendev.org/c/openstack/nova/+/846470 | 10:38 |
| gibi | I just want to make the functionality complete first. If you see some dealbreaker in the series then use -1 so I will stop and go back to it | 10:38 |
| sean-k-mooney | but i think the pci tracker will be sufficent to protect us | 10:38 |
| sean-k-mooney | ack | 10:38 |
| sean-k-mooney | so i just want to highligh a subtle behavior that you may or may not be aware of | 10:39 |
| sean-k-mooney | if a pci device has a claim against it in the pci_devices table | 10:39 |
| sean-k-mooney | and you remove it form the pci whitelist/dev spec | 10:39 |
| sean-k-mooney | we do not remove it form teh pci tracker until the vm is delete or moved | 10:40 |
| sean-k-mooney | that is to prevent you form currupting your db | 10:40 |
| gibi | yes | 10:40 |
| sean-k-mooney | by typoing the config | 10:40 |
| sean-k-mooney | so we need to make sure we dont break that | 10:40 |
| gibi | I follow that logic in the placement side as much as I can | 10:40 |
| sean-k-mooney | ack | 10:40 |
| sean-k-mooney | that is what i was wondering | 10:40 |
| sean-k-mooney | i was hoping that we woudl not remove RPs if they had allcoations | 10:41 |
| gibi | the commit message has an edge case described when nova will fail to start though https://review.opendev.org/c/openstack/nova/+/852397/5//COMMIT_MSG | 10:41 |
| sean-k-mooney | this else branch https://review.opendev.org/c/openstack/nova/+/846470/15/nova/compute/pci_placement_translator.py#320 | 10:41 |
| gibi | but other than that the PCI RP will be kept until the PCIDevice is in the nova DB | 10:42 |
| sean-k-mooney | is for the case where its in the pci tracker with a claim agaisnt it but removed form the config right | 10:42 |
| gibi | at that point in the series we have no allocations against PCI RPs. so we just ignore the device without spec | 10:42 |
| sean-k-mooney | we ignore removign it | 10:43 |
| sean-k-mooney | so the rp stays there whiel the device is not deleted in the pci tracker | 10:43 |
| sean-k-mooney | i guess it will just stay there | 10:44 |
| sean-k-mooney | ok ill review what you have later anyway | 10:44 |
| sean-k-mooney | since you have a patch for the case i was really worried about | 10:44 |
| sean-k-mooney | but ya until we have allocations it really does not matter anyway | 10:45 |
| gibi | later in the series we have the patch for reconf with allocations | 10:45 |
| gibi | https://review.opendev.org/c/openstack/nova/+/852397/5/nova/compute/pci_placement_translator.py#419 | 10:45 |
| sean-k-mooney | perfect you even tell them how to fix it in the warning | 10:46 |
| gibi | and here is a test case for it https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#760 | 10:48 |
| gibi | below that there is the test case for reconfiguring by removing a PF but adding its VFs. That is a hard stop | 10:48 |
| gibi | that would lead to a dependent device config which we explicitly not support | 10:49 |
| sean-k-mooney | can we add a test for creating an instnace then removing the device form the config and restarting the agent | 10:56 |
| sean-k-mooney | that shoudl allow the agent to start but complain loudly if we want to keep the old behaivr | 10:57 |
| sean-k-mooney | or fail to start | 10:57 |
| sean-k-mooney | but in either case the RP should not be updated and the allcoation should be kept in placment | 10:57 |
| gibi | yes here is the test case that warns https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#760 | 11:01 |
| gibi | and here is the case where nova refuse to start | 11:01 |
| gibi | https://review.opendev.org/c/openstack/nova/+/852397/5/nova/tests/functional/libvirt/test_pci_in_placement.py#802 | 11:01 |
| gibi | as the reconf would create dependent device situation | 11:01 |
| gibi | a simple config removal only cause a warning but RP and allocation is kept | 11:02 |
| gibi | a config change that removes an allocated PF and configures its VF will be a hard stop | 11:02 |
| sean-k-mooney | ack that sound like the semantics we want to have | 11:03 |
| sean-k-mooney | the warning behavior is the same as we had today | 11:03 |
| sean-k-mooney | and you are also catching a failure mode we dont prevent today | 11:03 |
| sean-k-mooney | well it will be prevented differntrly | 11:04 |
| sean-k-mooney | the current case if you add the pf and remove the vfs | 11:04 |
| sean-k-mooney | i think will result in the pf goign to unaviaable | 11:04 |
| sean-k-mooney | since the child device is claimed | 11:04 |
| sean-k-mooney | so its a slight behavior delta but not nessisarly a bad one | 11:04 |
| gibi | it is a delta becase we explicitly not support dependent device config with PCI in placement and that will be documented :) | 11:05 |
| gibi | this is the delta https://review.opendev.org/c/openstack/nova/+/846435/19/doc/source/admin/pci-passthrough.rst#370 | 11:05 |
| *** open10k8s_ is now known as open10k8s | 11:06 | |
| *** simondodsley_ is now known as simondodsley | 11:06 | |
| *** PrinzElvis_ is now known as PrinzElvis | 11:06 | |
| *** knikolla_ is now known as knikolla | 11:06 | |
| *** rpittau_ is now known as rpittau | 11:06 | |
| *** TheJulia_ is now known as TheJulia | 11:06 | |
| gibi | I probably need to mention the extra case in the doc about reconf | 11:06 |
| *** ozzzo1 is now known as ozzzo | 11:06 | |
| *** sfinucan is now known as stephenfin | 11:06 | |
| gibi | ohh I did https://review.opendev.org/c/openstack/nova/+/852397/5/doc/source/admin/pci-passthrough.rst#407 | 11:07 |
| *** arne_wiebalck_ is now known as arne_wiebalck | 11:24 | |
| gmann | gibi: dansmith_ : re on RBAC, I am +2 on dansmith_ patch https://review.opendev.org/c/openstack/nova/+/848021/2 | 12:15 |
| gmann | but little confused with the error in my patch, https://review.opendev.org/c/openstack/nova/+/849209/4 | 12:15 |
| opendevreview | Merged openstack/nova master: Add source dev parsing for vdpa interfaces https://review.opendev.org/c/openstack/nova/+/841016 | 12:20 |
| opendevreview | Merged openstack/nova master: Fix suspend for non hostdev sriov ports https://review.opendev.org/c/openstack/nova/+/841017 | 12:20 |
| sean-k-mooney | yay | 12:21 |
| sean-k-mooney | stephenfin: when you have time could you review the last vdpa patch https://review.opendev.org/c/openstack/nova/+/853704/7 | 12:22 |
| Uggla | sean-k-mooney, gibi is https://review.opendev.org/c/openstack/nova/+/854355 ok for you. Or you absolutely don't want to change NovaPersistentObject ? I think it is clearer in the way, but I don't mind changing if you think it is better. | 13:06 |
| gibi | gmann: left a note about the test failure in https://review.opendev.org/c/openstack/nova/+/849209/4 | 13:06 |
| gmann | gibi: that is what I am not getting why it is failing as there is no reference of PROJECT_ADMIN. locally all test passing | 13:09 |
| *** kopecmartin_ is now known as kopecmartin | 13:10 | |
| gmann | PS3 passed successfully which was rebased with no change in that patch | 13:11 |
| opendevreview | Ghanshyam proposed openstack/nova master: Remove system scope from all APIs https://review.opendev.org/c/openstack/nova/+/848021 | 13:12 |
| opendevreview | Ghanshyam proposed openstack/nova master: Keep legacy admin behaviour in new RBAC https://review.opendev.org/c/openstack/nova/+/849209 | 13:12 |
| gmann | rebasing on master, let's see | 13:12 |
| gibi | Uggla: I made some notes in https://review.opendev.org/c/openstack/nova/+/854355 . I can accept the direction you are proposing. | 13:15 |
| Uggla | gibi, thx I'm gonna look at the comments. | 13:16 |
| gibi | gmann: that feels like some weird rebase issue then in zuul | 13:16 |
| gmann | gibi: yeah seems so. it should pass now, let's see | 13:17 |
| gibi | yeah we will see | 13:17 |
| * gibi hates resize to same host | 13:18 | |
| *** dasm|off is now known as dasm | 13:27 | |
| sean-k-mooney | Uggla: i have not done a full review but that is proably workable providere there is not api or db impact | 13:47 |
| opendevreview | Rico Lin proposed openstack/nova master: Add locked_memory extra spec and image property https://review.opendev.org/c/openstack/nova/+/778347 | 13:48 |
| opendevreview | Rico Lin proposed openstack/nova master: libvirt: Add vIOMMU device to guest https://review.opendev.org/c/openstack/nova/+/830646 | 13:48 |
| opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | 13:48 |
| Uggla | sean-k-mooney, yep I just want the minimal change. Mostly to avoid next contributors "confusion" creating new objects. | 13:50 |
| *** mnasiadka_ is now known as mnasiadka | 14:25 | |
| *** dansmith_ is now known as dansmith | 14:29 | |
| auniyal__ | Hi All | 14:58 |
| auniyal__ | please review these | 14:58 |
| auniyal__ | https://review.opendev.org/c/openstack/nova/+/852171 | 14:58 |
| auniyal__ | https://review.opendev.org/c/openstack/nova/+/853811 | 14:59 |
| auniyal__ | https://review.opendev.org/c/openstack/nova/+/853812 | 14:59 |
| opendevreview | Ghanshyam proposed openstack/nova master: Keep legacy admin behaviour in new RBAC https://review.opendev.org/c/openstack/nova/+/849209 | 16:33 |
| gibi | sean-k-mooney: am I correct that in nova there is no implemented preference between consuming a PF (for a direct-phyiscal port) that has VFs over consuming a PF that has no VFs? | 16:35 |
| gibi | there is a single func test that assumes that we consume the PF without the VFs instead of the PF with VFs. But I think it was a simple coincidence that nova actually consumed that | 16:38 |
| gibi | with the pools splitted the order of the pools changed and that test fails now. | 16:39 |
| gibi | I will try to trick nova to change the order back... | 16:39 |
| gmann | gibi: 849209 should pass now, actually unshelve_to_host policy merged in between and RBAC patches were not rebases to master. after rebasing to master the unshelve_to_host new policy shows up in gerrit display which needs to be corrected in RBAC changes. | 16:47 |
| gmann | gibi: I fixed that and it in gate now | 16:48 |
| gibi | gmann: ahh, true, that explains it. I always forget that zuul rebases a patch even in the check queue | 16:49 |
| gmann | yeah | 16:49 |
| gmann | gibi: dansmith: unit/functional test pass (means things are ok now) on RBAC patch, please review https://review.opendev.org/c/openstack/nova/+/849209 | 17:20 |
| gmann | thanks sean-k-mooney for review, will check your query/feedback tomorrow | 17:20 |
| gibi | dansmith: if you can look at ^^ that would be awesome as you probably have the context. Otherwise I can try to look at it tomorrow morning with fresh brain | 17:23 |
| dansmith | gibi: I'm on my way to an appointment now.. so maybe later | 17:23 |
| gibi | dansmith: ack | 17:23 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Heal missing simple PCI allocation in the resource tracker https://review.opendev.org/c/openstack/nova/+/851359 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Heal PCI allocation during resize https://review.opendev.org/c/openstack/nova/+/852396 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Handle PCI dev reconf with allocations https://review.opendev.org/c/openstack/nova/+/852397 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Generate request_id for Flavor based InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/853835 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Create RequestGroups from InstancePCIRequests https://review.opendev.org/c/openstack/nova/+/852771 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support resource_class and traits in PCI alias https://review.opendev.org/c/openstack/nova/+/853316 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Map PCI pools to RP UUIDs https://review.opendev.org/c/openstack/nova/+/854118 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Make allocation candidates available for scheduler filters https://review.opendev.org/c/openstack/nova/+/854119 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Filter PCI pools based on Placement allocation https://review.opendev.org/c/openstack/nova/+/854120 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Store allocated RP in InstancePCIRequest https://review.opendev.org/c/openstack/nova/+/854121 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Func test for PCI in placement scheduling https://review.opendev.org/c/openstack/nova/+/854122 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support move operations with PCI tracking in placement https://review.opendev.org/c/openstack/nova/+/854247 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Split PCI pools per PF https://review.opendev.org/c/openstack/nova/+/854440 | 17:40 |
| opendevreview | Balazs Gibizer proposed openstack/nova master: Support same host resize with PCI in placement https://review.opendev.org/c/openstack/nova/+/854441 | 17:40 |
| * gibi disappears | 17:45 | |
| sean-k-mooney | gmann: non of it needs to be adressed in those patches | 18:05 |
| sean-k-mooney | gmann: just pointed out odd default we had like requiring admin to force shelve offload | 18:06 |
| sean-k-mooney | those are just things we proably should fix next cycle. | 18:06 |
| sean-k-mooney | gibi: correct | 18:06 |
| sean-k-mooney | gibi: there is no prefernce but adding one woudl make sense | 18:07 |
| sean-k-mooney | gibi: and could be done in a weigher now | 18:07 |
| sean-k-mooney | if we have provider summeries | 18:07 |
| sean-k-mooney | in addtion to the allcoation candiates | 18:07 |
| sean-k-mooney | actuly that woont be allowed if tracked in placment | 18:08 |
| sean-k-mooney | so ignore that | 18:13 |
| sean-k-mooney | it woudl have been nice in the on placment world to prefer PFs without vfs but now we will only allow it to be listed either as a pf or the vfs | 18:13 |
| sean-k-mooney | o/ talk to ye tomorrow | 18:13 |
| opendevreview | Merged openstack/nova master: Remove system scope from all APIs https://review.opendev.org/c/openstack/nova/+/848021 | 21:10 |
| opendevreview | Merged openstack/nova master: Add VDPA support for suspend and livemigrate https://review.opendev.org/c/openstack/nova/+/853704 | 21:22 |
| *** dasm is now known as dasm|off | 21:32 | |
| opendevreview | Brett Milford proposed openstack/nova master: Handle "no RAM info was set" migration case https://review.opendev.org/c/openstack/nova/+/852002 | 23:37 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!