| opendevreview | Merged openstack/nova-specs master: Re-propose "Add maxphysaddr support for Libvirt" for 2023.2 Bobcat https://review.opendev.org/c/openstack/nova-specs/+/878753 | 01:42 |
|---|---|---|
| opendevreview | Vitalii Vrublevskyi proposed openstack/nova master: Napatech SmartNIC support https://review.opendev.org/c/openstack/nova/+/859577 | 08:28 |
| opendevreview | Vitalii Vrublevskyi proposed openstack/os-vif master: Openvswitch driver was extended https://review.opendev.org/c/openstack/os-vif/+/859574 | 08:28 |
| bauzas | as a reminder, today is the last day for approving specs | 08:38 |
| elodilles | bauzas sean-k-mooney auniyal : a bit of a distraction from the specs reviews: i have a quick question in the os-vif release patch: https://review.opendev.org/c/openstack/releases/+/887486/1#message-9bb674cb5826ef01bbf20c442ad18a7ae8e08bbd | 09:25 |
| sean-k-mooney | we do not really need a m2 release of os-vif. although it does not hurt let me take a look at the coment quickly | 10:11 |
| sean-k-mooney | ah the set default change | 10:11 |
| sean-k-mooney | elodilles: os-vif is release with intermediaries so we can do as many releases as we want as long as we have at least one per release | 10:13 |
| sean-k-mooney | that also means we dont have to align them to milestones so if we want to wait for https://review.opendev.org/c/openstack/os-vif/+/885127 we can | 10:13 |
| sean-k-mooney | elodilles: also just replied to https://review.opendev.org/c/openstack/os-vif/+/883015/1#message-fb3bacb0ce69979a469c69a136bc4dcfd89ae915 | 10:17 |
| sean-k-mooney | elodilles: also just commented on your specific question https://review.opendev.org/c/openstack/releases/+/887486/1#message-a1446c91db95121f23e1304625fb5a728b4c0b71 on the release patch | 10:22 |
| sean-k-mooney | happy to chat about this more when your around | 10:22 |
| sean-k-mooney | elodilles: for more context the only reason i called out the change in the upgrade section was because it does not take effect on existing running instance immideatly | 10:27 |
| sean-k-mooney | so simply updating os-vif is not enough the vms need tobe moved ro rebooted to have ovs port recreated | 10:27 |
| sean-k-mooney | so i didnt want operators to think cool i patch it so my performance will imporve | 10:27 |
| sean-k-mooney | it will for new instances or old ones once recreated via a hard reboot or move op | 10:28 |
| sean-k-mooney | i could have just called that out in the fixes section but my thinking was when you are upgrading you would wnat to know that you shoudl live migratie the instace or comunicate to your custoemres that a reboot will improve the perfermance if they are expirince low network througput | 10:29 |
| elodilles | sean-k-mooney: it's not a problem you added it in the upgrade section, it attracts more attention :) | 10:46 |
| elodilles | and thanks for the explanation | 10:46 |
| elodilles | i've +2'd the patch | 10:46 |
| sean-k-mooney | hehe thats the point of release notes | 10:46 |
| elodilles | yepp :) | 10:47 |
| sean-k-mooney | in terms fo the backprot woudl you prefer to remove the default for the config option | 10:47 |
| sean-k-mooney | we woudl have to slightly modify the test to set it and add a new test to assert its not set by default | 10:48 |
| sean-k-mooney | but i think that would be pretty simple | 10:48 |
| elodilles | well, it's not about my preference, really :) the thing is that a change should not change default behaviour when an operator upgrades to a new release | 10:48 |
| elodilles | especially if it's a change that could cause issues for operators | 10:49 |
| sean-k-mooney | ya the tricky part here is because a bug in ovs we did not see that theree was actully a change in behavior when we swapped form libvirt plugging to os-vif pluging the interface | 10:49 |
| sean-k-mooney | well it cant as far as i am aware | 10:49 |
| sean-k-mooney | the patch only sets teh linux-noop qos policy on the port if the port does not exist on teh bridge | 10:50 |
| sean-k-mooney | so this woudl only cause an issue if that was not supported by your ovs version | 10:50 |
| elodilles | and we don't always realize what can cause issues ;) so to be on the safe side, should chose opt-in config ;) | 10:51 |
| sean-k-mooney | to my knoldage this has been supproted for many many years in ovs | 10:51 |
| elodilles | (on stable branches) | 10:51 |
| sean-k-mooney | yep | 10:51 |
| sean-k-mooney | that is why i normally do this as two patches on master | 10:51 |
| sean-k-mooney | but i forgot this time since we had some heat to fix this downstream | 10:52 |
| sean-k-mooney | as i said im find with making it opt in in the backport if thats what other think we should do | 10:52 |
| elodilles | i see. yepp, it's better when it is separated into 2 patches | 10:52 |
| sean-k-mooney | there is no opt out in the master version because when libvirt was doing this it alwasy set the noop qdisc on the port | 10:54 |
| elodilles | sean-k-mooney: i mean, generally it is the best way of working to follow. of course if a team decides that this is fine like this, i'm not against then. | 10:54 |
| sean-k-mooney | of couse that was not documented anywhere which is why we missed it orgianlly | 10:55 |
| sean-k-mooney | i dropped my +2 anyway until we come to a concensus | 10:55 |
| elodilles | the opt-in version is always easy to backport, the opt-out ones need consideration by the team every time though. to put it this way. o:) | 10:56 |
| sean-k-mooney | correct but there is no opt out in master becasue we did not think opting out was a reasonable thing to do. that said i coudl see addign that option to master as a sperate patch and then backporting that as the stabel default. i.e. add "disable" to the enum and make that the default in stable | 10:58 |
| sean-k-mooney | basicaly the master version alwasy creates the port with linux-noop by default (you can enables other QOS by default too) and then all QOS config should then be doen via neutron | 10:59 |
| sean-k-mooney | instead of setting the default via /sys/... | 11:00 |
| sean-k-mooney | what changed in the kernel was in older kernels settign a default qos in /sys/... did not get applied to tap devices | 11:04 |
| sean-k-mooney | at somepoitn that change so any operator using that to set the default qos for there physical nic suddenly got that qos applied to the vm port on upgrading there kernel | 11:04 |
| elodilles | ACK, thanks for the explanation :) | 11:19 |
| * sean-k-mooney feels kind bad for talking about networkign in the nova channel :P | 11:30 | |
| sean-k-mooney | bauzas: gibi pushed my comments with a +1 on https://review.opendev.org/c/openstack/nova-specs/+/887011 | 12:08 |
| sean-k-mooney | can you take a look and let me knwo if this is somethign we are ok defering to the impelmetaion and fixing up after spec freeze | 12:08 |
| sean-k-mooney | i agree with the overall content and think we should proceed with this im just not convince by the storage of the keymanger uuid isn a flavor extra spec or image poroperty | 12:10 |
| sean-k-mooney | and i would like to disucss that with melwitt when she is back form PTO | 12:10 |
| sean-k-mooney | but i dont want to really hold the spec on that | 12:10 |
| bauzas | sean-k-mooney: okay, so you could accept the spec and just tell you would like to discuss about some implementation question after the spec freeze | 12:11 |
| opendevreview | Merged openstack/nova-specs master: Re-propose "Policy service role spec" https://review.opendev.org/c/openstack/nova-specs/+/881880 | 12:12 |
| sean-k-mooney | ya so if peopel are ok with that and potially updating the spec to refelct what we decied in the implemation reivew then i can approve as is | 12:12 |
| sean-k-mooney | i just dont think we should be providing a way for end users to specify the keymangaer secret uuid | 12:13 |
| sean-k-mooney | espically when the end user cant know if its barbican or something else | 12:13 |
| sean-k-mooney | we also dont want to force a image/flavor per instance | 12:14 |
| sean-k-mooney | and we obvioulys done want to share keeys between instnaces | 12:14 |
| sean-k-mooney | so recording the uuid somehwere in our db sure (ideally instance_system_metadta) | 12:14 |
| sean-k-mooney | but using flavor/image proeprty for that is a no in my book | 12:14 |
| bauzas | ++ | 12:18 |
| sean-k-mooney | ok i have +2w'd the spec and -2'd the implemenation patch that added the image property https://review.opendev.org/c/openstack/nova/+/870935/4 | 12:32 |
| sean-k-mooney | ill drop that once we chat about it with melwitt | 12:33 |
| opendevreview | Merged openstack/nova-specs master: Re-propose spec for ephemeral storage encryption https://review.opendev.org/c/openstack/nova-specs/+/887011 | 12:39 |
| bauzas | gibi: sean-k-mooney: other cores, we have 2 already +2'd specs, can you try to look at them today given the spec freeze ? https://review.opendev.org/q/project:openstack/nova-specs+status:open+label:Code-Review%253E%253D%252B2 | 14:56 |
| bauzas | also, I'm definitely not an expert of SmartNics, so could you please look at https://review.opendev.org/c/openstack/nova-specs/+/859290 ? | 14:57 |
| melwitt | sean-k-mooney, bauzas: the image property is for the case of the image created from a snapshot, that has no instance associated with it. it's associated only with the image. that's the problem I need to solve | 15:00 |
| opendevreview | Merged openstack/nova-specs master: Adds cleanup to remove dangling volumes https://review.opendev.org/c/openstack/nova-specs/+/878757 | 15:32 |
| bauzas | ralonsoh: I know it's pretty late in your day and I'm pinging you late but could you get a quick pass on https://review.opendev.org/c/openstack/nova-specs/+/859290 ? | 16:00 |
| bauzas | today is Nova spec freeze and I don't know whether the Neutron community accepts this | 16:00 |
| ralonsoh | let me check | 16:00 |
| bauzas | thanks a lot | 16:00 |
| ralonsoh | bauzas, ah yes, there was a missing question but is replied | 16:01 |
| ralonsoh | I'll +1 it | 16:01 |
| opendevreview | Merged openstack/nova-specs master: Propose tooling and docs for unified limits https://review.opendev.org/c/openstack/nova-specs/+/887014 | 18:40 |
| opendevreview | melanie witt proposed openstack/nova-specs master: Amend spec to add more details around encryption secrets https://review.opendev.org/c/openstack/nova-specs/+/887905 | 21:20 |
| melwitt | fyi sean-k-mooney ^ | 21:22 |
| opendevreview | melanie witt proposed openstack/nova-specs master: Amend spec to add more details around encryption secrets https://review.opendev.org/c/openstack/nova-specs/+/887905 | 23:02 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!