Monday, 2023-03-13

__ministryAbove bug happened when I do resize in same host compute. So, never had "it was not supported". Can you help me fix this bug?02:22
plibeauhello guys, I need your help on this change to merge also on neutron side: https://review.opendev.org/c/openstack/nova/+/86117208:34
opendevreviewKonrad Gube proposed openstack/nova-specs master: Re-propose using extend volume completion action for 2023.2  https://review.opendev.org/c/openstack/nova-specs/+/87723308:39
dvo-plvHello, Sean. I would like to discuss this blueprint https://review.opendev.org/c/openstack/neutron/+/869510 . Previously we implemented our separate linkvirt driver, but you prefer to move to the default openvswitch driver. We made our internal poc and published it to the opendev to simplify our discussion on how it would be better to be implemented09:03
Ugglagibi, hello, ouch you gave me a lot of homework. :)09:09
bauzasUggla: fwiw, starting to review your series again :)09:12
Ugglabauzas, cool thanks. So I will wait for your comments before changing anything.09:13
bauzasack09:14
bauzasUggla: so, I have like the same concerns than gibi here09:51
bauzasthe first patch is only getting a soft -1 because I'd want you to test some deletion behaviour09:52
bauzasapart from this, I'm OK09:52
bauzasnow, I have more concerns for the objects patc09:52
bauzaspatch09:52
bauzasit looks to me that we should discuss how to use this object between the services09:53
bauzasand what it could do09:53
Ugglabauzas, ok10:27
bauzasUggla: I need to get my daughter in 10 mins but we can discuss that in the afternoon if you want10:28
Ugglaok sounds good10:28
bauzasI'll ping you later around 2pm CET10:29
opendevreviewRajesh Tailor proposed openstack/nova master: Fix duplicate cell creation with same name  https://review.opendev.org/c/openstack/nova/+/87694010:47
dvo-plv@sean-k-mooney, are you here ?12:05
sean-k-mooneyo/12:10
sean-k-mooneydvo-plv: hi yes i am 12:11
dvo-plvI would like to discuss this blueprint https://review.opendev.org/c/openstack/neutron/+/869510 . Previously we implemented our separate linkvirt driver, but you prefer to move to the default openvswitch driver. We made our internal poc and published it to the opendev to simplify our discussion on how it would be better to be implemented12:15
dvo-plvbtw, I can not figure out, how to mention user here?12:15
sean-k-mooneyas in to ping people12:16
sean-k-mooneyon irc12:16
sean-k-mooneyor something else12:16
dvo-plvyes, how to ping pople here. How do you do it with me?12:19
sean-k-mooneyjust dvo-plv  or dvo-plv:12:19
sean-k-mooneydvo-plv: so like this12:19
sean-k-mooneydoes that highlight for you12:19
sean-k-mooneybasicaly most client auto highlight on the nic you are connected with and you can configure others12:20
dvo-plvi see, thank you12:20
sean-k-mooneythe : at the end generally gets added if you tab complete but it shoudl not be required12:21
dvo-plvso ,if you have a time, I would like to discuss solution how to support our nic propperly12:21
sean-k-mooneymy client will match on my nic regardless or prefix or sufix12:21
sean-k-mooneydvo-plv: so im in two minds12:21
sean-k-mooneyif you intend to someday upstream supprot for your nic to vanilla ovs then it should be in the generic openvswich/ovn drivers in tree12:22
sean-k-mooneyif you plan to maintain a fork then your orginal approch is fine12:22
sean-k-mooneyin general if the way you connect to a network backend it common acorrss vendors we like to have only one implemation of that12:23
sean-k-mooneyin your specific case i see there are some special requriements around the name of the vhost-user socket12:23
sean-k-mooneyi suspect that is specific to your vswith implmation12:24
dvo-plvyes, we have our own dpdk driver what requires some specific moments12:25
sean-k-mooneyas currently written i think your patch might break the exiting virtio-forwarder code12:26
dvo-plvwhen you are talking about original approach. You mean separate driver or implementation support with openvswitch12:26
sean-k-mooneywhen i was suggesting using a singel ml2/driver for both i was suggesting ensuring your nic works with the dpdk_user supprot in vaniall ovs12:27
sean-k-mooneyand i was asking if that was what you were trying to enable12:27
sean-k-mooneythe answer to the second quetion is no12:27
sean-k-mooneyyou are not trying to enabel to upstream ovs feature12:28
sean-k-mooneyyour are trying to enable your vendor specific version12:28
sean-k-mooneyin which case usign an out of tree ml2 dirver is appropreiate12:28
sean-k-mooneythat way you do not need to worry about compatiablity with netonomes implamation of virtio forward12:28
sean-k-mooneynova just uses the vhost-user path provided by neutron so as long as you set it to the correct value that should be transparent to nova12:29
sean-k-mooneydvo-plv: have dpdk removed or raised the limit on dpdk type ports above 32 yet12:32
sean-k-mooneydvo-plv: what i would suggest is adding a VIF_DETAILS_VHOSTUSER_NAPATECH_PLUG12:37
sean-k-mooneyso in the port binding details set a flag to indicate taht its napatech12:37
sean-k-mooneycontinue to use vnic_type=vritio-forwarder12:38
justas_napaHi. To answer the question regarding ovs support - it really depends on the end user requirements12:39
sean-k-mooneyin nova check for both and in a new if in os_vif_util implement the logic you require12:39
justas_napawe maintain our own OvS to support features like QinQ and some special QoS schemes12:40
sean-k-mooneyjustas_napa: the in tree ml2/ovs and ml2/ovn dirver are only ment to supprot ovs form openvsiwtch.org12:40
sean-k-mooneyso if the basic virtio-forward fucntionality is not upstream to vanilla ovs it should be in a seperatem ml2 driver and os_vif driver12:41
justas_napaI think we are OK to limit ourselves to vanilla ovs12:41
justas_napaunless adding support for our own OVS is trivial12:41
sean-k-mooneywell vaniall ovs has two ways to enabel vhost-user but the proposal is not using either of them12:42
sean-k-mooneyhttps://docs.openvswitch.org/en/latest/topics/dpdk/vhost-user/12:42
sean-k-mooneyinstead you are created dpdk vdevs https://docs.openvswitch.org/en/latest/topics/dpdk/vdev/12:43
sean-k-mooneythere is supprot for dpdk representor prots12:44
justas_napaso then it's custom ml2 and os_vif driver12:45
sean-k-mooneyhttps://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors12:45
sean-k-mooneyam i think so.12:46
justas_napayes, we are heavy users of representors12:46
sean-k-mooneyif you go with the out of tree ml2/os-vif drivers then all that is requried in nova is a minor change to call them12:46
sean-k-mooneyso ok maybe we need to level set12:46
sean-k-mooneyjustas_napa: the dpdk type port that you are creating for use with virio-forward12:47
sean-k-mooneyis that a vf representor12:47
sean-k-mooneyas descirbed here https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors12:47
justas_napayes12:47
justas_napabecuase all dataplane is offloaded to FPGA12:47
sean-k-mooneyok so if that is what we are enableing and the supprot in upstream ovs also works with your nics we can enabel that in the intree support12:48
sean-k-mooneyso next question you require the vhost-user socket path to have a specific format to corralate teh vhost-user path to the dpdk representor correct12:48
justas_napaI don't think we require specific path, just the socket name12:49
sean-k-mooneythe orginal requriements when vhsot-user was first added to ovs was the socket path's final segment and port name must be the same12:49
sean-k-mooneyright so for normal vhost-user when it was first added the socket-name and port name had to be the same12:50
sean-k-mooneylater we added a socket path to ovs to remove that limiation as part of supproting vhost-user-client mode where qemu is the server12:50
justas_napa-chardev socket,id=char0,path=/usr/local/var/run/stdvio5,server12:51
justas_napaso we are flexible on thje path, but the last part is stdvio<#VF>12:51
sean-k-mooneyya so your dirver breaks the convention that the final segment of the name matches the name of the ovs port which is fine12:52
sean-k-mooneyack12:52
sean-k-mooneyhttps://review.opendev.org/c/openstack/neutron/+/869510/2/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#21712:52
sean-k-mooneyso that is why you need this code12:52
justas_napayep12:53
sean-k-mooneyso the problem i see is the pci_slot is always going to be set for vnic_type virtio-forwarder12:53
sean-k-mooneybut we only want to take the else branch if its napatech12:53
sean-k-mooneyso ninstead of the current if we shoudl add a config value to the agent and check if that is set12:54
sean-k-mooneyso just like 12:54
sean-k-mooney        sockdir = agent['configurations'].get('vhostuser_socket_dir',12:54
sean-k-mooney                                              ovs_const.VHOST_USER_SOCKET_DIR)12:54
sean-k-mooneyyou shoudl add something like sockdir = agent['configurations'].get('vhostuser_socket_name_scheme','port_name')12:55
sean-k-mooneyand for napatech you woudl set vhostuser_socket_name_scheme to VF_index12:55
sean-k-mooneyor something liek that12:55
justas_napaOK12:55
justas_napadvo-plv - does this work for us?12:56
sean-k-mooneyi need to think about the nova patch a bit but i think if you do that the nova patch is fine as is12:57
justas_napaack12:57
sean-k-mooneywell it needs test but the core functionality is proably correct12:57
dvo-plvin our solution, socket name value is depends on the pci slot12:58
dvo-plvThis value vhostuser_socket_name_scheme will be located at the neutron conf file12:58
sean-k-mooneyyes used by the neutron_l2_agent12:59
sean-k-mooneyit will need to get added to the agent report12:59
sean-k-mooneywhich will make it avaiable to the ml2 driver during binding12:59
sean-k-mooneylike this https://github.com/openstack/neutron/blob/master/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py#L379-L38013:00
sean-k-mooneyits how we pass the per host vhostuser_socket_dir and datapath_type info today13:00
sean-k-mooneydvo-plv: looking at teh os-vif code i think that is also ok just needs tests13:01
sean-k-mooneyjustas_napa: dvo-plv  my concern basically is if we enable this and we use vaniall ovs-dpdk will it still work13:02
dvo-plvSo, instead of extending ovs with virtio-forwarder, you suggest to add new parameter ( socket scheme ) and we will create port with vhost user vif type, and based on a new parameter, it will affect socket path13:03
justas_napaas long as user does not try to setup qinq and advance qos, it will work 13:03
sean-k-mooneydvo-plv: not quite13:03
sean-k-mooneydvo-plv: keep your ml2/ovs driver patch as it is using virtio-forwarder13:03
sean-k-mooneyi think that is the correct vnic_type13:03
sean-k-mooneyim suggesting not basing the format on if the pci_slot it set13:04
sean-k-mooneywe shoudl either have a config option for it or base it on vnic_type virtio-forwarder13:04
sean-k-mooneyim just checkign something13:04
sean-k-mooneywe might not need to modify your patches at all13:05
dvo-plvI see,  make this function agent_vhu_sockpath able to change the structure of socket's name if it is specified in the config file 13:06
sean-k-mooneyyep13:06
sean-k-mooneyso im trying to make sure we dont break agilio_ovs  in the process of enabling napatech13:07
sean-k-mooneyim just checkign how we determin its agilio_ovs today in nova13:07
sean-k-mooneyok your current patch shoudl be ok as is13:08
sean-k-mooneythey use a seperate vif_type13:08
sean-k-mooneyjustas_napa: dvo-plv: so based on the refactoring that you have already done following my intial feedback13:09
sean-k-mooneyi think the current solution shoudl be generic enough to work as is13:09
sean-k-mooneywe dont have any exsiting use of vif_type=ovs and virtio-forwarder13:09
sean-k-mooneywe can add a comment that if we need a diffent nameing scheme in the futrue then a config option can be added at that point13:10
justas_napaI'm not sure I follow13:11
justas_napado you think we need further updates13:11
justas_napaor are we going as-is?13:11
sean-k-mooneyjustas_napa: let me rephase. your code is fine as is it just need tests and docs13:11
justas_napaOK13:11
sean-k-mooneyill try and review the spec this week. if i have not done so by thrusday ping me 13:12
dvo-plvWe need to update spec file accodring this concept, I will ping you when it will be ready13:13
sean-k-mooneymy general feedback is we need test to go along with the functional changes you have made and based on a quick review of the surrounding code and the questions you answered here i think the current approch is correct13:13
sean-k-mooneyso the spec is pretty light on detail in general 13:15
sean-k-mooneyit would be good to update it with links to https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors13:15
sean-k-mooneysince that is really what you are tryign to enable13:15
justas_napasure. will do13:16
sean-k-mooneyi was hevially invovled in enabling dpdk supprot in openstack in general so most of the other core reviews dont have the same levle of context that i have13:16
plibeaubauzas: thx for the review, I have reply on your comment. https://review.opendev.org/c/openstack/nova/+/86117213:21
sean-k-mooneyjustas_napa: dvo-plv: one general comment on the spec (this is hard to get right by the way) the spec is intended to capture all the info require so that if you could not complete the feature someone else with a familararity with nova could compelte it. given the code is already written and looks mostly correct what you shoudl really focus on is providing enouch context for13:22
sean-k-mooneynova reviews who dont really have famiariaty with ovs/dpdk/vf representors. basically add som short context paragraphs explaing what the technology is and how you are reusing/extendign the existing functionaly in nova/os-vif so that its simpler for other cores to review.13:22
dvo-plvyes, sure, we will process this comment13:24
dvo-plvI have some concenrs about this patch https://review.opendev.org/c/openstack/os-vif/+/859574/4/vif_plug_ovs/ovs.py How properly we should pass scheme to the os-vif, it hould be part of the vif in _plug_vf method?13:24
jrossershould HW_ARCH_* trait be automatically set on compute hosts?13:29
sean-k-mooneydvo-plv: we should be able to just pass the vhost-user socket path 13:31
sean-k-mooneyjrosser: ideally yes but i don tknow if the libvirt driver does that today13:32
jrossermy test suggests that for Z they're not there13:33
sean-k-mooneythen that was likely not implemtned13:34
*** d34dh0r5- is now known as d34dh0r5313:45
opendevreviewsean mooney proposed openstack/nova stable/xena: Nova resize don't extend disk in one specific case  https://review.opendev.org/c/openstack/nova/+/87726013:47
opendevreviewsean mooney proposed openstack/nova stable/wallaby: Nova resize don't extend disk in one specific case  https://review.opendev.org/c/openstack/nova/+/87728413:50
dvo-plvsean-k-mooney: does it will be flexible for another usage, if we pass socket path and parse it, getting vf_num for representor port 13:55
dvo-plvI mean that we add additional parameter to the neutron conf to get abiltiy set scheme for socket name to add some flexibility13:56
dvo-plvhttps://review.opendev.org/c/openstack/os-vif/+/859574/4/vif_plug_ovs/ovs.py#30713:57
dvo-plvhere13:57
bauzasUggla: sorry I said to you that we could discuss about your series at 2 pm CET,  but I was doing another stuff, do you want to discuss this now ?14:11
Ugglabauzas, yes it is possible14:11
bauzascool14:12
bauzasUggla: (and other people wanting to discuss at https://review.opendev.org/c/openstack/nova/+/839401/24) meet.google.com/cah-soio-ard14:13
bauzasshit14:13
bauzashttps://meet.google.com/cah-soio-ard14:13
opendevreviewDan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2  https://review.opendev.org/c/openstack/nova-specs/+/87729115:11
dansmithbauzas: you ready for a 2023.2 specs directory patch?15:20
bauzasdansmith: we should already have it15:23
bauzasamirite ?15:23
dansmithis it proposed?15:23
sean-k-mooneyhttps://github.com/openstack/nova-specs/tree/master/specs/2023.215:24
sean-k-mooneyits merged15:24
dansmithwtf, I just fetched and had to create it myself15:24
dansmithhrm, okay15:25
dansmithmust'n'tve worked or something15:25
sean-k-mooneyi assume your working on the set service_id in compute node table spec15:25
dansmithyeah ^15:26
sean-k-mooneycool 15:26
dansmithI'm realizing maybe I should have tried harder to get this second phase into 2023.1 because of the SLURP rules, but oh well15:31
tobias-urdinsean-k-mooney: if you have some time over this week can you check the mdev naming fixes proposed to stable branches, starting with zed https://review.opendev.org/c/openstack/nova/+/866152 and the parent patch and cherry-picks w/ parents, ty!15:37
bauzastobias-urdin: I think I said +2 for stable/zed, right?16:01
bauzascorrect, so we need one stable core ^16:01
tobias-urdinbauzas: yes! just need more, then moving on the the cherry-picks to older releases16:01
bauzas++16:10
opendevreviewDan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2  https://review.opendev.org/c/openstack/nova-specs/+/87729116:12
* bauzas ends his day now, see you tomorrow17:11
opendevreviewMerged openstack/nova master: Unbind port when offloading a shelved instance  https://review.opendev.org/c/openstack/nova/+/85368218:04
gmannelodilles: fixed your comment, please check https://review.opendev.org/q/I4e3e5732411639054baaa9211a29e2e2c8210ac0+status:open18:31
elodillesgmann: looking18:39
gmannthanks18:42
elodilleslooks good, thanks!18:53
opendevreviewGhanshyam proposed openstack/nova master: DNM: testing 2023.1|2 unit tests job template  https://review.opendev.org/c/openstack/nova/+/87732019:09
opendevreviewGhanshyam proposed openstack/nova stable/2023.1: DNM: testing 2023.1|2 unit tests job template  https://review.opendev.org/c/openstack/nova/+/87726219:10
opendevreviewGhanshyam proposed openstack/nova stable/zed: DNM: testing 2023.1|2 unit tests job template  https://review.opendev.org/c/openstack/nova/+/87726319:10
gibiUggla: what am I missing. I try to test your manial series I have an active nfs share in manila. I have a stopped VM in nova. When I associate the share with the VM the compute tries to mount the share but it seems it stuck. https://paste.opendev.org/show/bFyX8c5xjnPQdPk9YEiK/ I also tried to manually mount on the host but that also stucks19:10
gibiUggla: btw after the API time outs the share remains in "inactive" state in nova side so I think nova thinks that the mount was successfully but it probably isn't: https://paste.opendev.org/show/bQlQVJaLOF1fhfuQR6ND/19:15
gibiUggla: then when I start the VM with openstack server start it happily starts up an uses the empty directory as the share where the nfs share should have been mounted19:17
gibiso it looks everything is OK but in the other hand the VM now has write access to a hypervisor directory without size limites19:18
gibiUggla: I can also confirm my suspicion that if an active VM with an active share is being deleted then we leak the active share_mapping in the DB and therefore probably leaking the mount on the hypervisor too https://paste.opendev.org/show/bs44BjHHcFkICNNlqw9R/ 19:22
gibiI cannot confirm the latter as I cannot mount the share in the first place19:22

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