| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-03-13 | |||
| 13:07:19 | sean-k-mooney | so im trying to make sure we dont break agilio_ovs in the process of enabling napatech | |
| 13:07:33 | sean-k-mooney | im just checkign how we determin its agilio_ovs today in nova | |
| 13:08:20 | sean-k-mooney | ok your current patch shoudl be ok as is | |
| 13:08:28 | sean-k-mooney | they use a seperate vif_type | |
| 13:09:07 | sean-k-mooney | justas_napa: dvo-plv: so based on the refactoring that you have already done following my intial feedback | |
| 13:09:19 | sean-k-mooney | i think the current solution shoudl be generic enough to work as is | |
| 13:09:40 | sean-k-mooney | we dont have any exsiting use of vif_type=ovs and virtio-forwarder | |
| 13:10:06 | sean-k-mooney | we can add a comment that if we need a diffent nameing scheme in the futrue then a config option can be added at that point | |
| 13:11:24 | justas_napa | I'm not sure I follow | |
| 13:11:37 | justas_napa | do you think we need further updates | |
| 13:11:53 | justas_napa | or are we going as-is? | |
| 13:11:54 | sean-k-mooney | justas_napa: let me rephase. your code is fine as is it just need tests and docs | |
| 13:11:58 | justas_napa | OK | |
| 13:12:50 | sean-k-mooney | ill try and review the spec this week. if i have not done so by thrusday ping me | |
| 13:13:43 | dvo-plv | We need to update spec file accodring this concept, I will ping you when it will be ready | |
| 13:13:53 | sean-k-mooney | my 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 correct | |
| 13:15:10 | sean-k-mooney | so the spec is pretty light on detail in general | |
| 13:15:25 | sean-k-mooney | it would be good to update it with links to https://docs.openvswitch.org/en/latest/topics/dpdk/phy/#representors | |
| 13:15:34 | sean-k-mooney | since that is really what you are tryign to enable | |
| 13:16:07 | justas_napa | sure. will do | |
| 13:16:12 | sean-k-mooney | i 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 have | |
| 13:21:46 | plibeau | bauzas: thx for the review, I have reply on your comment. https://review.opendev.org/c/openstack/nova/+/861172 | |
| 13:22:07 | sean-k-mooney | justas_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 for | |
| 13:22:09 | sean-k-mooney | nova 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:24:47 | dvo-plv | yes, sure, we will process this comment | |
| 13:24:49 | dvo-plv | I 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:29:03 | jrosser | should HW_ARCH_* trait be automatically set on compute hosts? | |
| 13:31:42 | sean-k-mooney | dvo-plv: we should be able to just pass the vhost-user socket path | |
| 13:32:28 | sean-k-mooney | jrosser: ideally yes but i don tknow if the libvirt driver does that today | |
| 13:33:11 | jrosser | my test suggests that for Z they're not there | |
| 13:34:50 | sean-k-mooney | then that was likely not implemtned | |
| 13:47:30 | opendevreview | sean mooney proposed openstack/nova stable/xena: Nova resize don't extend disk in one specific case https://review.opendev.org/c/openstack/nova/+/877260 | |
| 13:50:42 | opendevreview | sean mooney proposed openstack/nova stable/wallaby: Nova resize don't extend disk in one specific case https://review.opendev.org/c/openstack/nova/+/877284 | |
| 13:55:18 | dvo-plv | sean-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:56:12 | dvo-plv | I mean that we add additional parameter to the neutron conf to get abiltiy set scheme for socket name to add some flexibility | |
| 13:57:45 | dvo-plv | https://review.opendev.org/c/openstack/os-vif/+/859574/4/vif_plug_ovs/ovs.py#307 | |
| 13:57:46 | dvo-plv | here | |
| 14:11:24 | bauzas | Uggla: 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:48 | Uggla | bauzas, yes it is possible | |
| 14:12:10 | bauzas | cool | |
| 14:13:08 | bauzas | Uggla: (and other people wanting to discuss at https://review.opendev.org/c/openstack/nova/+/839401/24) meet.google.com/cah-soio-ard | |
| 14:13:12 | bauzas | shit | |
| 14:13:39 | bauzas | https://meet.google.com/cah-soio-ard | |
| 15:11:56 | opendevreview | Dan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2 https://review.opendev.org/c/openstack/nova-specs/+/877291 | |
| 15:20:19 | dansmith | bauzas: you ready for a 2023.2 specs directory patch? | |
| 15:23:40 | bauzas | dansmith: we should already have it | |
| 15:23:47 | bauzas | amirite ? | |
| 15:23:49 | dansmith | is it proposed? | |
| 15:24:07 | sean-k-mooney | https://github.com/openstack/nova-specs/tree/master/specs/2023.2 | |
| 15:24:09 | sean-k-mooney | its merged | |
| 15:24:23 | dansmith | wtf, I just fetched and had to create it myself | |
| 15:25:16 | dansmith | hrm, okay | |
| 15:25:23 | dansmith | must'n'tve worked or something | |
| 15:25:59 | sean-k-mooney | i assume your working on the set service_id in compute node table spec | |
| 15:26:10 | dansmith | yeah ^ | |
| 15:26:20 | sean-k-mooney | cool | |
| 15:31:16 | dansmith | I'm realizing maybe I should have tried harder to get this second phase into 2023.1 because of the SLURP rules, but oh well | |
| 15:37:47 | tobias-urdin | sean-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! | |
| 16:01:20 | bauzas | tobias-urdin: I think I said +2 for stable/zed, right? | |
| 16:01:39 | bauzas | correct, so we need one stable core ^ | |
| 16:01:56 | tobias-urdin | bauzas: yes! just need more, then moving on the the cherry-picks to older releases | |
| 16:10:21 | bauzas | ++ | |
| 16:12:53 | opendevreview | Dan Smith proposed openstack/nova-specs master: Add compute-object-ids spec for 2023.2 https://review.opendev.org/c/openstack/nova-specs/+/877291 | |
| 18:04:21 | opendevreview | Merged openstack/nova master: Unbind port when offloading a shelved instance https://review.opendev.org/c/openstack/nova/+/853682 | |
| 18:31:03 | gmann | elodilles: fixed your comment, please check https://review.opendev.org/q/I4e3e5732411639054baaa9211a29e2e2c8210ac0+status:open | |
| 18:39:44 | elodilles | gmann: looking | |
| 18:42:17 | gmann | thanks | |
| 18:53:34 | elodilles | looks good, thanks! | |
| 19:09:48 | opendevreview | Ghanshyam proposed openstack/nova master: DNM: testing 2023.1|2 unit tests job template https://review.opendev.org/c/openstack/nova/+/877320 | |
| 19:10:13 | opendevreview | Ghanshyam proposed openstack/nova stable/2023.1: DNM: testing 2023.1|2 unit tests job template https://review.opendev.org/c/openstack/nova/+/877262 | |
| 19:10:26 | opendevreview | Ghanshyam proposed openstack/nova stable/zed: DNM: testing 2023.1|2 unit tests job template https://review.opendev.org/c/openstack/nova/+/877263 | |
| 19:10:52 | gibi | Uggla: 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 stucks | |
| 19:15:44 | gibi | Uggla: 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:17:42 | gibi | Uggla: 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 mounted | |
| 19:18:27 | gibi | so it looks everything is OK but in the other hand the VM now has write access to a hypervisor directory without size limites | |
| 19:22:29 | gibi | Uggla: 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:45 | gibi | I cannot confirm the latter as I cannot mount the share in the first place | |
| #openstack-nova - 2023-03-14 | |||
| 05:18:09 | opendevreview | Amit Uniyal proposed openstack/nova stable/wallaby: fup: Print message logging uncaught nova-manage exceptions https://review.opendev.org/c/openstack/nova/+/877334 | |
| 08:45:14 | Uggla | gibi, Hi I agree that currently nothing prevent leaking the share if you delete the VM. | |
| 08:47:56 | bauzas | Uggla: you tested it ? | |
| 08:48:00 | bauzas | that was my question | |
| 08:48:04 | Uggla | yes | |
| 08:49:27 | bauzas | ok, so | |
| 08:49:32 | bauzas | https://review.opendev.org/c/openstack/nova/+/831193/24/nova/db/main/models.py#677 | |
| 08:50:01 | bauzas | that means the FK would be not relationed | |
| 08:51:31 | Uggla | gibi, I'm surprised regarding the mount error not tracked, any idea about what is causing the issue. Firewalling maybe ? | |
| 08:53:08 | gibi | Uggla: what manila config you used for testing? I tried the default that is DHSS=True + GenericDriver | |
| 08:55:17 | Uggla | gibi, DHSS=false LVM driver | |
| 08:55:48 | gibi | Uggla: is there a reason why GenericDriver + DHSS=true would not work? | |
| 08:55:56 | gibi | (I'm restacking with LVM DSSF=false now) | |
| 08:56:05 | gibi | *DHSS | |
| 08:57:01 | Uggla | gibi, to be honest I have not tested with DHSS=false thinking that in our context that was not necessary. So I don't know. :( | |
| 08:58:20 | bauzas | so, IMHO, we should delete the share mapping in the instance delete call | |
| 08:58:35 | bauzas | as we can't use the delete cascade SQL support | |
| 08:58:49 | Uggla | bauzas, yep and add the semaphore to avoid any race. | |
| 08:59:09 | bauzas | + in init_instance(), recreating the share like I said | |
| 08:59:30 | bauzas | and maybe a periodic for making sure we don't leak any shares | |
| 09:00:31 | bauzas | Uggla: can you look at what happens to the foreign key ? | |
| 09:01:59 | Uggla | bauzas, yes I'll try to simulate it again. | |
| 09:03:45 | bauzas | cool | |