Thursday, 2020-10-15

« Wednesday, 2020-10-14 Index Friday, 2020-10-16 »
*** k_mouza has joined #openstack-nova00:00
*** k_mouza has quit IRC00:05
openstackgerritMerged openstack/nova stable/train: api: Set min, maxItems for server_group.policies field  https://review.opendev.org/75270700:05
openstackgerritMerged openstack/nova stable/rocky: post live migration: don't call Neutron needlessly  https://review.opendev.org/75067300:06
openstackgerritMerged openstack/nova stable/queens: [placement] Add status and links fields to version document at /  https://review.opendev.org/75123900:06
*** macz_ has joined #openstack-nova00:15
*** JamesBenson has joined #openstack-nova00:17
*** macz_ has quit IRC00:20
*** JamesBenson has quit IRC00:22
*** artom has quit IRC00:28
*** JamesBenson has joined #openstack-nova00:35
*** hamalq has quit IRC00:43
*** gyee has quit IRC01:04
*** macz_ has joined #openstack-nova01:09
*** Liang__ has joined #openstack-nova01:14
*** macz_ has quit IRC01:14
openstackgerritMerged openstack/nova stable/stein: Set different VirtualDevice.key  https://review.opendev.org/74941501:29
*** spatel has joined #openstack-nova01:44
*** jangutter_ has joined #openstack-nova01:55
*** jangutter_ has quit IRC01:55
*** jangutter has quit IRC01:55
*** jangutter has joined #openstack-nova01:56
*** rcernin has quit IRC02:26
*** Yumeng has joined #openstack-nova02:45
*** rcernin has joined #openstack-nova02:49
*** brinzhang has joined #openstack-nova03:00
*** zzzeek has quit IRC03:10
*** ircuser-1 has joined #openstack-nova03:11
*** zzzeek has joined #openstack-nova03:13
*** lemko has joined #openstack-nova04:01
*** dave-mccowan has quit IRC04:03
*** manoj_kumar_kata has joined #openstack-nova04:04
*** lemko4 has quit IRC04:04
*** jangutter_ has joined #openstack-nova04:08
*** jangutter_ has quit IRC04:08
*** jangutter has quit IRC04:08
*** jangutter has joined #openstack-nova04:09
*** manoj_kumar_kata has quit IRC04:10
*** ratailor has joined #openstack-nova04:10
openstackgerritHemanth Nakkina proposed openstack/nova master: Update pci stat pools based on PCI device changes  https://review.opendev.org/74917504:15
*** jangutter_ has joined #openstack-nova04:29
*** jangutter_ has quit IRC04:30
*** jangutter has quit IRC04:31
*** jangutter_ has joined #openstack-nova04:31
*** evrardjp has quit IRC04:33
*** evrardjp has joined #openstack-nova04:33
*** spatel has quit IRC04:38
*** manoj_kumar_kata has joined #openstack-nova04:39
*** efried1 has joined #openstack-nova04:50
*** eharney has quit IRC04:51
*** efried has quit IRC04:52
*** efried1 is now known as efried04:52
*** eharney has joined #openstack-nova05:04
*** tetsuro has joined #openstack-nova05:21
*** tetsuro has quit IRC05:25
*** zzzeek has quit IRC05:30
*** zzzeek has joined #openstack-nova05:32
*** manoj_kumar_kata has quit IRC05:35
*** manoj_kumar_kata has joined #openstack-nova05:41
*** tetsuro has joined #openstack-nova05:44
*** tetsuro has quit IRC05:46
*** manoj_kumar_kata has quit IRC06:03
*** manoj_kumar_kata has joined #openstack-nova06:04
*** Liang__ has quit IRC06:05
*** Liang__ has joined #openstack-nova06:08
*** vishalmanchanda has joined #openstack-nova06:09
*** Yumeng has quit IRC06:20
*** tetsuro has joined #openstack-nova06:24
*** manoj_kumar_kata has quit IRC06:32
*** manoj_kumar_kata has joined #openstack-nova06:32
*** slaweq has joined #openstack-nova06:35
*** manoj_kumar_kata has quit IRC06:37
*** manoj_kumar_kata has joined #openstack-nova06:37
*** tetsuro has quit IRC06:46
*** tetsuro has joined #openstack-nova06:50
*** ralonsoh has joined #openstack-nova06:53
*** tetsuro has quit IRC06:55
*** tetsuro has joined #openstack-nova06:57
*** andrewbonney has joined #openstack-nova07:01
*** tetsuro has quit IRC07:03
*** tetsuro has joined #openstack-nova07:03
openstackgerritMamduh proposed openstack/os-vif master: [WIP] get_ifname_by_pci_address compare phys_port_id  https://review.opendev.org/75835207:04
*** manoj_kumar_kata has quit IRC07:08
*** freenzyfriday has joined #openstack-nova07:13
*** tesseract has joined #openstack-nova07:20
*** rpittau|afk is now known as rpittau07:22
*** tetsuro has quit IRC07:32
*** rcernin has quit IRC07:35
*** rcernin_ has joined #openstack-nova07:35
*** tetsuro has joined #openstack-nova07:37
openstackgerritHemanth Nakkina proposed openstack/nova master: Update pci stat pools based on PCI device changes  https://review.opendev.org/74917507:41
*** dtantsur|afk is now known as dtantsur07:41
*** rcernin_ has quit IRC07:42
*** tosky has joined #openstack-nova07:42
*** tetsuro has quit IRC07:46
*** tetsuro has joined #openstack-nova07:47
*** tetsuro has quit IRC07:50
*** Liang__ has quit IRC08:12
*** CeeMac has joined #openstack-nova08:12
*** Liang__ has joined #openstack-nova08:13
*** ociuhandu has joined #openstack-nova08:16
*** tetsuro has joined #openstack-nova08:17
*** freenzyfriday has quit IRC08:22
*** derekh has joined #openstack-nova08:39
*** k_mouza has joined #openstack-nova08:49
*** k_mouza_ has joined #openstack-nova08:52
*** k_mouza has quit IRC08:55
*** tkajinam is now known as tkajinam|away09:17
*** tkajinam|away is now known as tkajinam09:17
*** jangutter has joined #openstack-nova09:23
bauzasgood late morning Nova09:24
*** zzzeek has quit IRC09:25
*** tetsuro has quit IRC09:26
*** jangutter_ has quit IRC09:26
*** zzzeek has joined #openstack-nova09:29
*** rcernin_ has joined #openstack-nova09:32
*** tetsuro has joined #openstack-nova09:36
*** rcernin_ has quit IRC09:36
gibio/09:37
*** tetsuro has quit IRC09:40
lyarwoodMorning morning09:55
*** rcernin_ has joined #openstack-nova09:56
stephenfingibi: Heads up that I've added specless BP approval for https://blueprints.launchpad.net/nova/+spec/compact-havana-db-migrations to the agenda for today's meeting. I won't be around though so I can move to next week if it proves to be any way contentious10:11
* stephenfin lol'd when he read the summary for https://blueprints.launchpad.net/nova/+spec/compact-havana-db-migrations from the *Icehouse* release10:11
stephenfin"...and make it easier to move to Alembic in the future." /o\10:11
stephenfinthat went super well, evidently10:12
toskystephenfin: is that related to the usage of sqlalchemy-migrate?10:15
stephenfintosky: Dumping the use of it, yes10:15
toskyso sqlalchemy-migrate is still a dependency, but it's kind of abandoned (x namespace), I've tried to fix the zuul jobs but of course most of the people marked as maintainers are not around10:16
stephenfintosky: It's dead. Alembic is apparently a far better designed tool that we should have switched to ages ago but, you know, ENOTIME10:24
stephenfinbut our number of DB migrations has plummeted in recent releases so no better time, I'm thinking. We shall see10:25
*** rcernin_ has quit IRC10:25
openstackgerritMerged openstack/nova stable/victoria: Follow up for cherry-pick check for merge patch  https://review.opendev.org/75761310:31
*** rcernin_ has joined #openstack-nova10:42
gibistephenfin: ack. I don't expect too much pushback on that10:47
openstackgerritLee Yarwood proposed openstack/nova master: Add regression test for bug #1899835  https://review.opendev.org/75823910:50
openstackbug 1899835 in OpenStack Compute (nova) "n-cpu attempts to disconnect volumes after early pre_live_migration failures on the destination during a live migration" [Undecided,New] https://launchpad.net/bugs/1899835 - Assigned to Lee Yarwood (lyarwood)10:50
lyarwoodstephenfin: ^ another pre_live_migration functional test if you have time today10:50
*** tbachman has quit IRC11:02
*** rcernin_ has quit IRC11:06
*** Liang__ has quit IRC11:07
*** lemko has quit IRC11:07
toskystephenfin: I know it's dead, but as long as projects still depend on it (because ENOTIME), it's not really dead :)11:09
openstackgerritStephen Finucane proposed openstack/nova master: Compact pre-Juno database migrations  https://review.opendev.org/75839411:17
openstackgerritStephen Finucane proposed openstack/nova master: TODO: Compact pre-Kilo database migration  https://review.opendev.org/75839511:17
openstackgerritStephen Finucane proposed openstack/nova master: Compact pre-Liberty database migrations  https://review.opendev.org/75839611:17
openstackgerritStephen Finucane proposed openstack/nova master: TODO: Compact pre-Mitaka database migrations  https://review.opendev.org/75839711:17
openstackgerritStephen Finucane proposed openstack/nova master: WIP: Compact pre-Newton database migrations  https://review.opendev.org/75839811:17
stephenfingibi: Think you could take a spin through https://review.opendev.org/#/q/status:open+project:openstack/nova+branch:master+topic:remove-xenapi maybe early next week too? I assume we can merge now that V is out the door11:20
stephenfinlyarwood: +2 to that and its predecessor11:22
stephenfin<3 functional tests11:22
stephenfinlyarwood: Not nova, but could you buzz through https://review.opendev.org/#/c/755887/ and https://review.opendev.org/#/c/755888/11:22
gibistephenfin: thanks for the reminder, will do11:23
*** lemko has joined #openstack-nova11:23
stephenfinDoes anyone know where that docs that mriedem did on novaclient-OSC gaps is gone to? Think I'm going to spend some time this cycle closing that out too11:24
gibiit is an etherpad... looking..11:25
gibihttps://etherpad.opendev.org/p/compute-api-microversion-gap-in-osc11:25
*** elod is now known as elod_afk11:26
stephenfingibi++ thanks!11:28
gibistephenfin: I also thought about the relationship of o.vo and mypy. o.vo handles the fields by generating properties for it runtime so that static checker is clueless. But I think w might be able to generate a .pyi interface file for o.vos that has the fields with static types to help mypy11:31
*** zzzeek has quit IRC11:33
*** zzzeek has joined #openstack-nova11:34
*** ratailor has quit IRC11:37
*** belmoreira has joined #openstack-nova11:38
*** k_mouza_ has quit IRC11:40
*** k_mouza has joined #openstack-nova11:46
sean-k-mooneythat makes sense althoug we can proably also extend mypy to pars ovo but the pyi file is proably simpler11:47
*** bbowen has quit IRC11:48
*** bbowen has joined #openstack-nova11:48
gibisean-k-mooney: yeah a plugin to mypy is also a possibility but as far as I see the plugin interface of mypy is unstable11:49
sean-k-mooneyi have not really looked at it11:50
gibihttps://mypy.readthedocs.io/en/stable/extending_mypy.html#extending-mypy-using-plugins "The plugin system is experimental and prone to change. I"11:51
*** rcernin_ has joined #openstack-nova11:59
sean-k-mooneywe could likely write a small script that would loop over the nova/objects dir and produce the pyi files and run that in the tox env12:04
sean-k-mooneywe could optionally commit them or just have them generated on the fly12:05
*** raildo has joined #openstack-nova12:06
*** lbragstad has quit IRC12:07
gibisean-k-mooney: yeah, that is the direction of my thinknig12:14
lyarwoodstephenfin: FWIW I wanted to sort the evacuate command out in osc12:16
lyarwoodTIL sean-k-mooney had a change for this12:16
lyarwoodadded to the PTG pad12:19
sean-k-mooneylyarwood: yep it more or less done but we want to rename it back to evacuate12:28
sean-k-mooneyits been on my todo list for a whiel if you feel like taking it over12:28
sean-k-mooneyif not i can try and do it before the ptg12:28
lyarwoodsean-k-mooney: so was that outcome, that we wanted to stay with evacuate for now?12:29
lyarwoodsean-k-mooney: if so I can rework it now12:29
sean-k-mooneylyarwood: thats what dansmith  wanted12:30
sean-k-mooneyalso since i want to add recreate as a new action it proably better not to rename12:30
brinzhanggibi, sean-k-mooney: I encounter a question, I want to backport https://review.opendev.org/#/q/status:merged+project:openstack/nova+branch:master+topic:bp/numa-aware-live-migration to Rocky release12:30
brinzhanggibi, sean-k-moooney: Because there is a big gap between its minor versions, I only merged the changes below the api, including the RPC Version changes, but changed rpc_version=5.3 to rpc_version=5.1 to adapt to the version number of Rocky release.12:30
sean-k-mooneylyarwood: the main concen was people thinking it was  a new thing calling it resurect12:31
sean-k-mooneybrinzhang: that a feature not a bug12:31
sean-k-mooneybrinzhang: so you cant backprot it12:31
lyarwoodsean-k-mooney: ack yup np, I'll rework it now in that case12:31
brinzhanggibi: but, when I request live migration the rpcverion=5.1 can not valid12:31
sean-k-mooneybrinzhang: rpc backports cant realy be done12:31
brinzhangsean-k-mooney: I know, but if I upgrade the rocky release to ussuri, that will be some impact than backport, so I just want to backport it12:33
brinzhangbut encountered the rpcverion invalid issue12:33
sean-k-mooney brinzhang yes12:33
sean-k-mooneyyou cant backport rpc change because it will break upgrades and rolling deployments12:33
sean-k-mooneywe have tried downstream and it really did not go well12:34
*** artom has joined #openstack-nova12:34
brinzhangdoes it need to change the oslo.messaging to adopt 5.1?12:34
brinzhangI got +1 on the basis of the rpc version of the Rocky version, which is not a complete backport12:35
sean-k-mooney+1 from who?12:35
sean-k-mooneywe should not attempt to backport this upstream12:36
brinzhang+1 from rocky rpc version 5.0-->5.1, and change the target_compute version from 35 to 3612:36
brinzhangI looked into the service db table, the node compute can be 36, but when we restart the compute service,the rpc version also in 5.012:37
brinzhangsean-k-mooney: Backport is a last resort12:38
brinzhangbase on https://github.com/openstack/nova/blob/stable/rocky/nova/compute/rpcapi.py#L351, +1 for the rpcversion12:39
*** elod_afk is now known as elod12:39
brinzhangrefer this patch: https://review.opendev.org/#/c/634606/8612:40
*** macz_ has joined #openstack-nova12:40
*** nweinber has joined #openstack-nova12:41
gibibrinzhang: I've never tried backporting an RPC change.12:44
sean-k-mooneygibi: stephenfin  has12:45
gibiI did assisted backporting API changes downstream, there we simply put the new API into the latest microversion of that branch. It is ugly but it works12:45
*** macz_ has quit IRC12:45
sean-k-mooneythat is ya less then good if you have to ship it to others12:46
brinzhanggibi: I want to know what key points I have overlooked. This method of incrementing the version number based on the rocky version is the same as adding a feature to the master, but why doesn't it take effect?12:46
sean-k-mooneyour rule is if it has object,db,api or rpc change its not backported12:46
brinzhang the rules are clear :D12:47
sean-k-mooneyam have you pinned your rpc version in the config12:48
gibisean-k-mooney: yeah, it was a compromise where the necessary risk and future cost was accepted (and I don't have to touch it so I'm fine with it)12:48
brinzhangbut in theoretically, my thought can pass, but it is not12:48
sean-k-mooneygibi: we have considered doing the same for api changes but decied not to for feature discovery12:49
gibibrinzhang: might be your problem related to the RPC version alias on the given branch (simply guessing here)12:49
sean-k-mooneye.g. we did not want ot create interoperabity issue between clouds12:50
sean-k-mooneygibi: yep that is want i was wondering for the config12:50
sean-k-mooneymore or less12:50
sean-k-mooneye.g. is it pinned to a version and or an alias12:50
brinzhanggibi: alias? what does this mean?12:51
brinzhang RPC version alias?12:52
gibibrinzhang: https://docs.openstack.org/nova/latest/user/upgrade.html#concepts12:52
sean-k-mooneyhttps://github.com/openstack/nova/blob/stable/rocky/nova/compute/rpcapi.py#L354-L36412:53
*** lbragstad has joined #openstack-nova12:53
brinzhangsean-k-mooney: we dont change the VERSION_ALIASES12:54
brinzhangit is as the same as your paste12:54
sean-k-mooneyyes but what do you have your rpc version set to in the config12:54
sean-k-mooneyblank, auto, an alias or a value12:55
brinzhangthe default, because we dont config the rpcversion in conf file12:55
sean-k-mooneyso i think that will default to the newest version supported by the current node12:56
sean-k-mooneymeaing if you have not applie the change to all serivce some will be 5.0 and other will be 5.112:56
sean-k-mooneyauto i think figures out thet max supported version for all nodes12:57
sean-k-mooneyfor the numa migration at least both comptue nodes and the conductor would need 5.112:57
brinzhangsean-k-mooney: yeah, I can try your suggestion, I changed two nodes's code in my 3 nodes cluster12:58
brinzhangbut stoped the nova-* services in another node12:58
sean-k-mooneyi think we store the version in the db somewhere12:59
sean-k-mooneyso stopping it might not be enough dansmith is who i normally ask about this becasue i generally try to avoid it12:59
brinzhangI just find the compute version in service db table12:59
sean-k-mooneythis is still the video i go to to understand this stuff https://www.youtube.com/watch?v=wVXiw09XP04 i wonder how much of that still applies13:00
brinzhanghttps://github.com/openstack/nova/blob/stable/rocky/nova/conf/rpc.py this conf file seems not contain your said auto, default value13:01
brinzhangyeah, I will see the vedio tomorrow, need to jump the firewall13:02
sean-k-mooneyauto is not the default13:02
brinzhangif I want to use auto, how to config?13:02
sean-k-mooneyhttps://github.com/openstack/nova/blob/stable/rocky/nova/conf/upgrade_levels.py13:03
sean-k-mooneybut also that is the file you want13:03
sean-k-mooneythe default is unest which means use the latest vers this code knows13:03
sean-k-mooneyhttps://github.com/openstack/nova/blob/stable/rocky/nova/conf/upgrade_levels.py#L55-L6113:03
sean-k-mooney[upgrade_level]/compute=auto13:04
brinzhanggot it, thanks13:04
sean-k-mooneyin this case you want 5.113:04
sean-k-mooneyso you can just set that13:05
sean-k-mooneyyou also need to do it for the conductor13:05
sean-k-mooneyhttps://github.com/openstack/nova/blob/stable/rocky/nova/conf/upgrade_levels.py#L11713:05
brinzhangyeah, I will try, two ways: 1. set [upgrade_level]/compute=auto in nova compute node, and then to do live migration,13:05
sean-k-mooneyyou need to set it for both compute and conductor13:06
brinzhang2. replaced all nodes's code of nova, and then try again13:06
brinzhangack13:06
sean-k-mooneyi assume you have also backported all the ovo changes13:06
sean-k-mooneye.g. the change to the migrate_data object to store the numa toplogy13:07
brinzhangyes, I was backported all the bp's patches13:08
sean-k-mooneythere have been some bugfixes too but it more or less shoudl be fine13:08
sean-k-mooneybrinzhang: you are aware you can manually live migrate numa instance before too right13:09
sean-k-mooneywe just dont recaluate the xml13:09
sean-k-mooneyso if you have 1 free compute node you can live mgirate without thse backports to do a rolling upgrade13:09
sean-k-mooneyyou just need to ensure the cpus and hugepages exctra are free on the dest13:10
brinzhangyes13:10
brinzhangI will use two node to have a test, at leat two node, because I should ensure this feature is fine, otherwise, it will have some risk, it's not allow13:11
brinzhangLater I will search for related bug fixes and backport13:12
sean-k-mooneyhave you also backported sriov live migration i assume not13:13
brinzhangsriov live migration we completed it by ourself, dont need to backport ^^13:13
brinzhangIf we upgrade in future, and we will use the merged codes too13:14
sean-k-mooneythe reason i bring that up is numa migratio nmerged after sriov and at least 1 of the patches in the numa serises is adding compatiablty with sriov live migrfation13:14
sean-k-mooneyyour going from rocky to train right13:15
brinzhangyes, it contained, I was found your point13:15
sean-k-mooneytrain is the verssion that had both so once your on train you shoudl be fine13:15
brinzhangmaybe from rocky to ussuri13:15
brinzhangbecause we also want to use the pmem feature too ^13:16
brinzhangsean-k-mooney: thanks for your help13:17
brinzhangI will go home now, have a holiday sean-k-mooney, gibi :D13:17
gibibrinzhang: o/13:17
dansmithsean-k-mooney: compute=auto will try to select the version supported by the oldest compute service in the system, based on service_version13:41
sean-k-mooneydansmith:yes13:41
sean-k-mooneythe max version suspport by the oldest13:41
sean-k-mooneywhich is why i was saying to use 5.113:42
sean-k-mooneynot auto13:42
sean-k-mooneysince brinzhang is not update all the nodes13:42
dansmithah, you said [05:56:27]  <sean-k-mooney>so i think that will default to the newest version supported by the current node13:42
dansmithbut you meant that for if it's unset or something?13:42
sean-k-mooneyyes unset13:42
dansmithokay I was skimming the backscroll and thought you were saying that was the auto behavior,13:43
dansmithbut I see now13:43
*** lbragstad has quit IRC13:55
bbowensean-k-mooney: Do you know the latest on https://trello.com/c/f0lLrWxf/2465-swisscom-schweiz-agsev302710209bz1860395neutronrarp-packets-after-live-migration-are-dropped-by-br-link-due-to-a-timing-issue-be, I thought you were making a change and it was not on neutron team. was a hint in rhos-prio to go back to compute13:58
bbowen^now on neutron13:59
lyarwoodbbowen: wrong channel :)13:59
bbowenty13:59
*** vishalmanchanda has quit IRC14:01
*** tkajinam has quit IRC14:20
*** rcernin_ has quit IRC14:27
*** macz_ has joined #openstack-nova14:28
*** macz_ has quit IRC14:33
artomstephenfin, I think I'd like a video chat to explain https://review.opendev.org/#/c/756070/6/nova/virt/hardware.py if you have some time today or next week14:34
artomNot so much the change itself, more the context around it14:34
artomYou set `instance_cell.min_threads = threads_no`, and I'm trying to work backwards to where threads_no is coming from, and it's... hard :P14:44
*** iurygregory has quit IRC14:57
*** iurygregory has joined #openstack-nova14:57
*** macz_ has joined #openstack-nova15:01
*** abhishekk is now known as abhishekk|away15:06
openstackgerritBalazs Gibizer proposed openstack/nova master: Use the non polling notification waiter in func test  https://review.opendev.org/75844515:07
openstackgerritBalazs Gibizer proposed openstack/nova master: Create a fixture around fake_notifier  https://review.opendev.org/75844615:07
openstackgerritBalazs Gibizer proposed openstack/nova master: Use NotificationFixture for legacy notifications too  https://review.opendev.org/75844815:09
openstackgerritBalazs Gibizer proposed openstack/nova master: Test the NotificationFixture  https://review.opendev.org/75845015:10
openstackgerritBalazs Gibizer proposed openstack/nova master: Move fake_notifier impl under NotificationFixture  https://review.opendev.org/75845115:10
gmanngibi: I have doctor appointment and would not be able to join today meeting.15:13
gibigmann: ack, thanks for the headsup15:13
*** belmoreira has quit IRC15:18
*** mlavalle has joined #openstack-nova15:22
*** tbachman has joined #openstack-nova15:32
*** dklyle has joined #openstack-nova15:36
*** gyee has joined #openstack-nova15:39
lyarwoodstephenfin: you've been working on LM bugs recently with the periodic jobs right?15:42
lyarwoodstephenfin: did you find any issues with _sync_instance_power_state running *after* an instance had already paused?15:42
artomlyarwood, I think he's on PTO this EU afernoon - he cancelled a meeting we had with Brian and Erwan15:49
lyarwoodah forgot my bad15:49
lyarwoodthanks artom15:49
gibinova meeting starts in 10 minutes on #openstack-meeting-315:51
*** rpittau is now known as rpittau|afk15:52
*** lbragstad has joined #openstack-nova15:55
*** dave-mccowan has joined #openstack-nova15:56
*** k_mouza has quit IRC16:00
*** k_mouza has joined #openstack-nova16:11
dansmithmelwitt: ahh, maybe the separate joins are only happening when we grab multiple instances16:13
dansmithif that's the case, then just making the get-by-uuid path work the same would be easy16:14
melwittis there a specific part of the code you're looking at that makes you think that?16:15
*** k_mouza has quit IRC16:15
dansmithI shouldn't say "separate joins"... they're called "manual joins" in the db api, because they're separate queries16:15
dansmithmelwitt: look at this: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L127816:15
dansmithmelwitt: and this: https://github.com/openstack/nova/blob/master/nova/db/sqlalchemy/api.py#L132816:16
melwittahh16:16
dansmithwhen we moved flavor info into sysmeta, we caused a giant explosion on every instance list, right around icehouse,16:17
*** songwenping_ has joined #openstack-nova16:17
dansmithso we moved to this manual join method where we query the metadata tables separately, but in bulk, to avoid the n:m join16:17
dansmithso the single case is similar, just where n=1, but if m is large enough, still unideal16:18
sean-k-mooneywe basicaly need to use a subqurry for the userdata16:18
sean-k-mooneyand get the rest of the data normally16:18
*** k_mouza has joined #openstack-nova16:18
dansmithsubquery or separate query? not sure how the former will help16:19
sean-k-mooneyseperate query16:19
dansmithyeah, we do that on list, just don't on get, that's my point16:19
sean-k-mooneyyeah16:19
dansmithall the machinery is there from list, so get can just call it but with the single instance we care about16:20
sean-k-mooneyi didnt look a the code specificaly but this sounded familar as i think we already optimised this once before for that list case16:20
*** songwenping__ has quit IRC16:20
sean-k-mooneyits just one of those things that works fine for small number and then explode because its n*m complexity16:21
sean-k-mooneyin size16:21
dansmithfor list, yeah16:21
sean-k-mooneywell in this case its joing userdata with each metadata propertiy16:21
dansmithfor single get n=1, but m is large16:21
dansmithright16:21
sean-k-mooneyya so its still liniar instad of O(1)16:22
sean-k-mooneynot as bad but still kind of sucks16:22
dansmithlinearly big is still big :)16:22
*** k_mouza has quit IRC16:23
*** martinkennelly has joined #openstack-nova16:26
*** k_mouza has joined #openstack-nova16:27
openstackgerritmelanie witt proposed openstack/nova stable/ussuri: Follow up for cherry-pick check for merge patch  https://review.opendev.org/75846316:30
*** k_mouza has quit IRC16:31
*** eharney has quit IRC16:33
*** ociuhandu_ has joined #openstack-nova16:35
*** ociuhandu has quit IRC16:38
*** dtantsur is now known as dtantsur|afk16:39
*** ociuhandu_ has quit IRC16:40
openstackgerritBalazs Gibizer proposed openstack/nova master: Ignore PCI devices with 32bit domain  https://review.opendev.org/75669716:43
*** lpetrut has joined #openstack-nova16:44
sean-k-mooneygibi: i have seen devices that use teh 32bit domian that can be assigned to qemu16:45
sean-k-mooneyalthoguh i was using a patched qemu16:45
gibisean-k-mooney: interesting. Could you check if the patch included support for 32 bit domain? there is a link in the commit message for the qemu code16:46
sean-k-mooneyit was an internal intel build for unrelated hardware16:46
sean-k-mooneyit was never upstreamed :)16:46
gibimelwitt: improved https://review.opendev.org/75669716:46
sean-k-mooneygibi: looking at your patch i think its fine but it woudl be nice to add some docs for this limitation somewhere16:47
sean-k-mooneymaybe in the pci whitelist docs somewhere16:47
gibisean-k-mooney: we already limit the whitelist to 16 bit16:47
melwittgibi: thanks, will look16:47
sean-k-mooneygibi: but do we document that limitation or just do it in code16:48
gibisean-k-mooney: good point, we actually not https://docs.openstack.org/nova/latest/configuration/config.html#pci.passthrough_whitelist16:49
gibiI will add extra information to the config description16:49
sean-k-mooneyya we jsut say "PCI address of the device. Both traditional glob style and regular expression syntax is supported."16:49
sean-k-mooneyif we can add a note that we only supprot 16 bit domains ill be happy16:50
gibisean-k-mooney: I will do16:50
gibibut not today. As it is beer time for me now16:51
gibisee you all tomorrow16:51
gibio/16:51
sean-k-mooneygibi: redhatters will be off tomorrow16:51
sean-k-mooneyits a company day16:51
sean-k-mooneyjust an fyi16:51
gibisean-k-mooney: thanks for the heads up, enjoy your free day16:52
sean-k-mooneyill try to not spend it working on ci :)16:53
melwittsean-k-mooney: am I missing something or does that make all invalid configs ignored? maybe I just have a wrong understanding16:55
sean-k-mooneyi have not done a full review but ill take a look now16:56
sean-k-mooneyyour wondering about https://review.opendev.org/#/c/756697/4/nova/pci/devspec.py16:57
sean-k-mooneythe transfromataion of PciConfigInvalidWhitelist to PciInvalidDevice16:57
sean-k-mooneythis change actully does not take into account the glob "*:whatere" adresses ro the regex support16:58
*** lpetrut has quit IRC16:59
sean-k-mooneyself.domain = pci_addr.get('domain', REGEX_ANY) also does not truncate to just 16 bits today16:59
melwittI don't know this parsing code really, but I was just wondering would this mask all/most actual invalid configs16:59
*** derekh has quit IRC17:00
sean-k-mooneyso its cactaching PciConfigInvalidWhitelist17:01
sean-k-mooneywhich is only raised here https://github.com/openstack/nova/blob/master/nova/pci/devspec.py#L51-L6617:01
sean-k-mooneybut it would catch any case where the value exceed the max17:01
sean-k-mooneyor where it was invalid17:01
*** hamalq has joined #openstack-nova17:02
melwittyeah, that's where I'm wondering is there a way to tell when it's a 32bit domain parse fail? so that we could raise something different?17:02
sean-k-mooneywe can look at teh data we parsed17:02
melwittI see17:03
sean-k-mooneywe could also split the excptions17:03
melwittkinda like the "if v > maxval"17:03
sean-k-mooneyya so https://github.com/openstack/nova/blob/master/nova/pci/devspec.py#L57-L6617:03
sean-k-mooneythis is the two cases where its currently raised17:03
sean-k-mooneybut we can have the maxval branc raise a subclass of PciConfigInvalidWhitelist17:04
sean-k-mooneyand then catch that17:04
melwittyeah, so we are currently hitting the "if v > maxval" condition17:04
sean-k-mooneye.g. PciConfigInvalidWhitelistMaxExceeded17:04
sean-k-mooneyyes so we want to ignore those devices17:05
sean-k-mooneythe other case is shwere we get non hex adresses17:05
melwittso we want to ignore all that are > maxval17:05
sean-k-mooneyi guess we coudl ignore those too17:05
sean-k-mooneythese are comming from libvirt17:05
sean-k-mooneywell actully no theey are not17:05
sean-k-mooneyits the whitelist at this point althogh some of this code is common17:06
melwittok, that's where I was confused was, I was assuming we only wanted to ignore the ones > maxval. do we want to ignore non hex addresses? just saying that this patch seems to just make the validations all ignored. is there are reason to keep the validations?17:07
sean-k-mooneyam well we should not allow you to whitelist device that are invliad17:11
sean-k-mooneyi need to look at the bug17:11
melwittok. I'll try to write more comments17:13
sean-k-mooneyso i think the issue is that we sue thise function to filter the pci deivce returned by libvirt17:14
sean-k-mooneyso even if you dont have whitelist value that is 32bits it will explode when we try to filter the value17:14
*** k_mouza has joined #openstack-nova17:14
sean-k-mooneywe do nova.pci.devspec.WhitelistPciAddress.match()17:15
sean-k-mooneywe proably dont want to catch the exption where gibi is17:16
sean-k-mooneyinstead we proably want to only catch it and skip the device wehre we do the match17:16
melwittok, so this issue is NOT to do with the whitelist at all. I see17:18
melwittso the whitelist parsing code is reused to parse the devices returned by libvirt17:18
melwittok, I was missing that17:18
*** k_mouza has quit IRC17:19
sean-k-mooneyso here https://github.com/openstack/nova/blob/master/nova/pci/manager.py#L118-L12217:19
sean-k-mooneyyep we reuse https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/whitelist.py#L80-L8817:20
melwittok, this way we're sure we're not parsing a whitelist17:20
*** andrewbonney has quit IRC17:20
sean-k-mooneyso what we do is we build a device spec  object for every entry in the whitelist17:20
sean-k-mooneythen for every device returned by the vert driver we constuct a second one and see if tey match17:21
sean-k-mooneyits the libvirt address that is 32bit17:21
melwittbut the exception guessed raised in __init__ not in the match() call?17:22
melwitt*is17:22
*** brinzhang has quit IRC17:22
sean-k-mooneyfrom here https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/devspec.py#L217-L23817:23
*** brinzhang has joined #openstack-nova17:23
melwittohhhh17:23
melwittguh17:23
openstackgerritSylvain Bauza proposed openstack/nova master: Fix the vGPU dynamic options race  https://review.opendev.org/75847017:23
melwittso devices_json is a list of whitelist pci objects17:24
melwittmaybe not... I'll look. this is so confusing to me17:26
sean-k-mooneyits a json blob we construct for the data we get from libvirt17:26
sean-k-mooneythe nodedev xmls17:26
sean-k-mooneyit looks like a list of these https://github.com/openstack/nova/blob/131f7606c179bd08bde3b7ae0e6bb0b59acf1545/nova/tests/unit/pci/test_manager.py#L32-L4117:28
melwittyeah, just that device_assignable is from the Whitelist class17:28
sean-k-mooneyi have to look this up everythim but it generated here https://opendev.org/openstack/nova/src/branch/master/nova/virt/libvirt/driver.py#L7127-L715017:29
sean-k-mooneyi rewrote this recently17:30
sean-k-mooneybut we get all the nodedev form libvirt that are pci device ro nics17:31
melwittthanks17:31
melwittoh it's the dev_filter that's the whitelist, facepalm. sorry17:31
melwittI have gone through this before too, but still mess it up17:32
sean-k-mooneydev_filter is an instance of the Whitelist class yes17:33
sean-k-mooneywhic has one filed specs which holds all the whitelist entries17:33
sean-k-mooneythen we just do17:34
sean-k-mooney for spec in self.specs:17:34
sean-k-mooney            if spec.match(dev):17:34
sean-k-mooney                return True17:34
sean-k-mooney        return False17:34
sean-k-mooneyso if any of the entries match the json seriasition of the pci device form the virt driver w return true17:34
melwittok17:35
sean-k-mooneythat mach is the on defiend in WhitelistPciAddress17:35
*** k_mouza has joined #openstack-nova17:36
sean-k-mooneywhich does https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/devspec.py#L23817:36
sean-k-mooneywhic eventually expodes17:37
sean-k-mooneyit actully expoding here https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/devspec.py#L23717:37
sean-k-mooneywhen we contuct the dev object to compare agaisnt form teh adress we get form libvirt17:38
sean-k-mooneythat why its expoliding in init17:38
sean-k-mooneywe constuct the object form the string to reuse the parsing that was written for teh whitelist17:39
sean-k-mooneyso we dont have to have it twice17:39
melwittis pci_phys_addr the whitelist entry?17:40
*** k_mouza has quit IRC17:40
sean-k-mooneyno that is the pci adress form libvirt17:41
sean-k-mooneyself.pci_address_spec is the object form the whitelist17:41
sean-k-mooneyarent the names great17:41
sean-k-mooneyso easy to follow and not confusing at all17:41
melwitt /o\17:42
*** martinkennelly has quit IRC17:42
sean-k-mooneymelwitt: its not taking this branch https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/devspec.py#L231-L23417:43
sean-k-mooneybut it would explode if it did17:43
melwittwell, what I mean is, IIUC (which I probably don't) one of those construct of PhysicalPciAddress is "correctly" parsing a whitelist entry and the other one is parsing an address from libvirt. is my understanding wrong?17:43
sean-k-mooneythat is for pci device that are capable fo sriov17:43
melwittoh, ok. nevermind my question then17:43
sean-k-mooneyfor PF if you list a PF we allow all the VF if they are allocated to be used17:44
sean-k-mooneythat is what that branch is for17:44
sean-k-mooneyso if whitelist a PF its a shortcut to whitelist the VFs17:44
melwittthat makes sense17:44
sean-k-mooneyanyway the tl;dr  for gibi is we shoudl catch the excpetion here https://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/whitelist.py#L85-L87 instead17:46
sean-k-mooneyto that if should be in a try17:47
sean-k-mooneyand we retrun fause if we get the PciConfigInvalidWhitelist excption17:48
sean-k-mooneywhcih is what he is doing here https://review.opendev.org/#/c/756697/4/nova/pci/manager.py17:48
sean-k-mooneyback to your orginal question17:48
sean-k-mooneyhttps://review.opendev.org/#/c/756697/4/nova/pci/devspec.py17:48
sean-k-mooneyis still going to raise and excption if you input an invalid whitelist17:49
sean-k-mooneyso i think the compute agent will still fail to start in that case17:49
sean-k-mooneybut we support 3 different type of adresses.17:50
*** k_mouza has joined #openstack-nova17:50
sean-k-mooneygibi only exeteded this one since its the one we use for the virt driver objcts17:51
sean-k-mooneysince gibi is not catching the excption it will still fail here17:52
sean-k-mooneyhttps://github.com/openstack/nova/blob/2c3ff1004e8812da0bc4e5502762e6b9a7650846/nova/pci/whitelist.py#L52-L7917:52
sean-k-mooneywhen we are parsing the whitelist if it had a 32bit domain17:52
sean-k-mooneythis will raise the new excption17:53
sean-k-mooney   spec = devspec.PciDeviceSpec(ds)17:53
sean-k-mooneyline 7517:53
*** k_mouza has quit IRC17:55
gansohi folks! If anyone could please spare a few minutes to look at this stable/train backport, should be really simple and quick as the other ones: https://review.opendev.org/#/c/757870/18:21
*** artom has quit IRC18:22
*** tesseract has quit IRC18:22
*** artom has joined #openstack-nova18:22
gansomelwitt, lyarwood, elod ^18:22
*** ralonsoh has quit IRC18:22
melwittganso: ack will look later today18:24
gansomelwitt: thank you!18:24
*** ociuhandu has joined #openstack-nova18:55
*** ociuhandu has quit IRC18:59
*** gregwork has quit IRC19:26
elodganso: the backport looks good to me, just as it was ok to ussuri. +220:12
gansoelod: thanks!20:12
*** nweinber has quit IRC20:14
*** k_mouza has joined #openstack-nova20:50
*** k_mouza has quit IRC20:54
*** k_mouza has joined #openstack-nova20:54
*** raildo has quit IRC21:13
*** jobewan has quit IRC21:14
*** k_mouza has quit IRC21:14
*** slaweq has quit IRC21:31
*** brinzhang_ has joined #openstack-nova21:35
*** brinzhang has quit IRC21:39
*** rcernin_ has joined #openstack-nova22:02
*** rcernin_ has quit IRC22:19
*** slaweq has joined #openstack-nova22:22
*** hamalq has quit IRC22:32
*** hamalq has joined #openstack-nova22:33
*** slaweq has quit IRC22:38
*** tkajinam has joined #openstack-nova22:59
*** macz_ has quit IRC23:03
*** mlavalle has quit IRC23:23
*** tosky has quit IRC23:23
*** dklyle has quit IRC23:35
*** k_mouza has joined #openstack-nova23:51
*** k_mouza has quit IRC23:55
*** zzzeek has quit IRC23:55
*** zzzeek has joined #openstack-nova23:59
« Wednesday, 2020-10-14 Index Friday, 2020-10-16 »

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