Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2022-02-21
12:28:05 sean-k-mooney chateaulav: ^ at the end fo the day the descion is yours
12:28:12 gibi sean-k-mooney: +1
12:35:52 chateaulav Lol. Yeah I think that will be better overall gibi, I agree with sean-k-mooney that I can look at doing that for next cycle. It seems as though there are also some things in the work for the riscv firmware community that may be needed. Overall initial support will be arm, s390x, ppc, and potentially mips if I can solve the pci issue. So not bad, if you ask me, for initial release of emulation
12:36:18 gibi chateaulav: cool
12:36:52 gibi sean-k-mooney: filed the bug for the pci.utils networking.neutron coupling issue https://bugs.launchpad.net/nova/+bug/1961587
12:59:47 sean-k-mooney ack just triaged it
13:43:45 gibi thanks
14:22:06 opendevreview Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/830010
14:25:12 opendevreview Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/830010
14:38:27 sean-k-mooney gibi: have i mentioned how much i hate the fact that gerrit forward ports comments now
14:38:37 sean-k-mooney its such a ux regressions
14:38:59 gibi it is only a problem iff we forget to close the comments when they are solved
14:39:18 sean-k-mooney liek these ones https://review.opendev.org/c/openstack/nova/+/821606/10/nova/virt/libvirt/volume/lightos.py
14:39:35 sean-k-mooney gibi: also this has been retoactivly enabeld for all reviews in the past
14:39:51 gibi yeah, that retroactive thing is a pain I agree
14:40:03 sean-k-mooney so if we lookup a revew becasue we are lookign at git blame we will now get all the unresoved ones where the feature did not exist
14:41:04 sean-k-mooney im going to ack most of the commend on that once i check them but to me this breaks the workflow since if the converation is happenign betwen reviewrs rather then just the revier and authour
14:44:17 gibi yeah
14:44:44 gibi not optimal but in the other hand it helps when people only fix a set of comments and forget the rest
14:44:49 gibi and push a new PS
14:45:16 sean-k-mooney yes and know. it can but over all i think this is a more harmful change then good
14:45:41 gibi It forces me to close comment when they fixed or answered
14:45:50 gibi I think in time we can learn to use it well
14:45:54 sean-k-mooney right but i dont think you should have to do that
14:46:10 sean-k-mooney i am hoping we will get the option to disable it personlly
14:46:48 sean-k-mooney its the frist time that gerrit has made a change that has made me consider if we shoudl continue to use gerrit longterm
14:47:14 sean-k-mooney to me the fact that comments were pined to reveriosn was one of the big benifits for using gerrit
14:47:17 sean-k-mooney this regresses that
14:48:00 sean-k-mooney i can see the beniftis too but to me in its current form the beiftis do not outway the downside
14:48:21 gibi hm, maybe a toggle in the UX to show / hide the old PS comments can be a compromise
14:48:32 opendevreview yuval proposed openstack/nova master: Lightbits LightOS driver https://review.opendev.org/c/openstack/nova/+/821606
14:48:38 sean-k-mooney if it only forward ported the comment that refered to lines that were modified in the next revision i could proably live with that
14:49:35 yuval_ gibi sean-k-mooney thanks for the fast reply!
14:49:41 sean-k-mooney like its infurating that i have to load back up ps 8 to see why they refer too
14:50:53 gibi yuval_: will check the last version before end of my day
14:52:16 yuval_ thanks, I just updated as sean-k-mooney suggested the req file and lower constraints
14:52:23 yuval_ to see zuul pass
14:52:47 yuval_ I am now re-adding the lightbits ci to work on nova repo
14:53:00 sean-k-mooney yuval_: https://review.opendev.org/c/openstack/nova/+/821606/11/nova/virt/libvirt/volume/lightos.py#31 can you respond to this too
14:53:40 yuval_ yes, no problem
14:53:45 dmitriis sean-k-mooney, gibi: w.r.t. the above ping, I can chime in to help with reviews
14:54:14 gibi dmitriis: thanks
14:55:08 gibi dmitriis, sean-k-mooney: w.r.t. the sysfs pci_utils network.neutron coupling, I started looking into the refactor and I have to say that the whole resource tracker is infected with teh sysfs coupling via the pci device tracker
14:55:52 sean-k-mooney gibi: yes that is not surpriseing to be honest
14:56:08 gibi so this sean-k-mooney but then we are back to that the compute manager depends on sysfs
14:56:16 sean-k-mooney everything pci related is only supproted on linux today
14:56:47 gibi which is an acceptable limitation but the external depenency (sysfs) is not abstracted out
14:56:58 gibi and heavily tangled in
14:57:10 sean-k-mooney yes it is
14:57:31 gibi so I think today it would be extremely hard to add support to PCI for a non Linux host
14:57:45 sean-k-mooney it could certenly be factored more but i guess the expecation is you woudl add windows suport to the pci module if that ever came to be a thing
14:58:01 gibi hm
14:58:10 gibi yeah that would be an option too
14:58:14 sean-k-mooney gibi: well you would have to just implenet the supprot in the pci.utils module i
14:58:36 gibi I might have been overzealeus about this
14:58:50 sean-k-mooney its what we do in os-vif for what its worth https://github.com/openstack/os-vif/tree/master/os_vif/internal/ip
14:59:16 sean-k-mooney we have an ip module with a windows and linux impl
14:59:22 gibi yeah maybe that is a better direction
14:59:44 gibi maybe I just wish for a better defined pci.utils module
14:59:51 gibi where the interface is clear
15:00:08 sean-k-mooney ack
15:00:18 sean-k-mooney we kind of have grown it organicly over time
15:00:27 gibi yes, it is very organic :)
15:00:56 gibi like the init_host in the compute manager parses the whitelist config early but by that it already reading sysfs a lot :)
15:01:15 sean-k-mooney at some point i think we will want to unify mdev+pci+vdpa into a more uniform device tracker imple
15:01:16 gibi even if the compute manager itself does not use the whitelsit config at all
15:01:34 gibi sean-k-mooney: yeah that is also an interesting direction
15:01:43 sean-k-mooney gibi: parsing the whitelist does not hit sysfs
15:01:47 gibi it does
15:01:50 sean-k-mooney it should not
15:01:53 gibi yeah :d
15:01:58 gibi :D
15:02:01 sean-k-mooney parsing the whitelist does not look at the devices
15:02:13 sean-k-mooney it just reads the json and constucts the filter object
15:02:31 sean-k-mooney it should not make a single call to sysfs
15:02:50 sean-k-mooney https://github.com/openstack/nova/blob/0e0196d979cf1b8e63b9656358116a36f1f09ede/nova/pci/whitelist.py#L56-L83
15:03:26 gibi https://github.com/openstack/nova/blob/0c31561792e0e13a9f8267e71fa484ab79957f04/nova/pci/devspec.py#L287
15:03:34 gibi this is part of the Whitelist.__init__ codepath
15:03:59 gibi via PciDeviceSpec
15:04:32 sean-k-mooney it really should not me
15:04:37 sean-k-mooney https://github.com/openstack/nova/blob/0e0196d979cf1b8e63b9656358116a36f1f09ede/nova/pci/whitelist.py#L80
15:04:44 sean-k-mooney is where you say the dep happens
15:05:12 sean-k-mooney that shoudl not be happening there
15:05:59 sean-k-mooney we shoudl only be asseting the valudes in the config are valid here
15:06:09 gibi yeah L80 eventually calls _init_dev_details in the PciDeviceSpec
15:06:18 sean-k-mooney i guess we just added the validation for remote managed here
15:06:29 gibi yes that is remote managed part now that calls out to sysfs
15:06:41 sean-k-mooney gibi: ok well the intent is we shoudl not hit sysfs in this code path
15:07:08 sean-k-mooney i had suggested moving the valdiation logic out of there and into the virt driver for remote-managed
15:07:14 sean-k-mooney perhaps we shoudl do that
15:08:17 sean-k-mooney everthing here https://github.com/openstack/nova/blob/0c31561792e0e13a9f8267e71fa484ab79957f04/nova/pci/devspec.py#L315-L351 shoudl really be in teh virt driver
15:08:54 gibi I think there is two places now that potentiall hit the remote managed validation path. 1) the init_host in the compute manager to early exit the compute service 2) the pci device tracker that cannot do anything with an invalid config as it is called from a periodic
15:09:25 gibi so we potentially need a 3rd place that is in the virt driver and part of the init sequence
15:09:33 sean-k-mooney init_host calls the driver
15:09:42 sean-k-mooney that si where this validation shoudl be done
15:10:27 sean-k-mooney in here https://github.com/openstack/nova/blob/0e0196d979cf1b8e63b9656358116a36f1f09ede/nova/virt/libvirt/driver.py#L707
15:11:09 gibi yeah the confusion is that the validation is part of the whitelist config parsing and that config is compute level not libvirt level :/
15:11:31 sean-k-mooney gibi: its not
15:11:33 gibi so it make sense to validate whitelist at the config level
15:11:39 gibi * compute level

Earlier   Later