| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2023-08-25 | |||
| 08:12:48 | kashyap | gibi: Just saw your ping earlier; will check (the bug https://bugs.launchpad.net/nova/+bug/2023035) | |
| 08:13:45 | gibi | kashyap: thanks! | |
| 08:20:42 | kashyap | gibi: Related: Can you get unblock the Workflow-1 here? : https://review.opendev.org/c/openstack/nova/+/871968 | |
| 08:21:09 | kashyap | Elod is on PTO and that W-1 doesn't apply (as he did based on a comment from Amit, and Amit took back his -1) | |
| 08:28:54 | opendevreview | Ryota, Ochiai proposed openstack/nova master: doc: Update the installation guide for RHEL and CentOS https://review.opendev.org/c/openstack/nova/+/892607 | |
| 08:49:11 | gibi | kashyap: do I understand correctly that we don't want to backport https://review.opendev.org/c/openstack/nova/+/869950 just https://review.opendev.org/c/openstack/nova/+/871968 ? | |
| 08:53:36 | gibi | btw I cannot remove elodilles_pto's -w you need infra's help for that | |
| 09:03:51 | gibi | kashyap: according to my sources Elod is back on Monday | |
| 09:12:23 | auniyal | yes kashyap, I was in doubt if this need to be backported too - https://review.opendev.org/c/openstack/nova/+/869950, | |
| 09:12:23 | auniyal | so the reason to remove +1 is to get attention of cores, as there was no backport patch for 869950 | |
| 09:14:17 | auniyal | gibi, can you please review this - https://review.opendev.org/q/topic:dangling-volumes | |
| 09:20:28 | kashyap | gibi: Yes, your understanding is correct. Only https://review.opendev.org/c/openstack/nova/+/871968 | |
| 09:20:46 | kashyap | auniyal: (It - 869950 - is not required.) | |
| 09:20:48 | opendevreview | Ryota, Ochiai proposed openstack/nova master: doc: Update the installation guide for RHEL and CentOS https://review.opendev.org/c/openstack/nova/+/892607 | |
| 09:25:38 | auniyal | ack | |
| 09:48:35 | dvo-plv | sean-k-mooney, I found that socket can be different for vendors. So I would like to use something like that https://paste.opendev.org/show/bKfuy2TzkygTyLtyW8TE/ | |
| 09:49:33 | dvo-plv | this method creates scoket name accodring to the formula which user can provide via binding port profile. So in the result we will get multinic support and each nix can have uniq socket name | |
| 09:50:15 | dvo-plv | this method we can call here to format correct socket name https://review.opendev.org/c/openstack/neutron/+/869510/13/neutron/plugins/ml2/drivers/openvswitch/mech_driver/mech_openvswitch.py#221 | |
| 10:44:23 | opendevreview | Merged openstack/nova master: Fix exception catch when volume mount fails https://review.opendev.org/c/openstack/nova/+/852795 | |
| 12:50:47 | sean-k-mooney | speaking of +1... gibi care to take a look at https://review.opendev.org/c/openstack/nova/+/873061/9 | |
| 12:51:14 | sean-k-mooney | i dont know if we need more disucssion on that in general but i also too a swing at removing spwan_n entirly in the followup patch | |
| 12:51:17 | sean-k-mooney | melwitt: ^ | |
| 12:52:27 | sean-k-mooney | that might need more discusion but i think its proably a good idea in the long run. | |
| 12:58:09 | gibi | sean-k-mooney: I will check. I see that it was improved since I last read it | |
| 13:55:54 | opendevreview | Balazs Gibizer proposed openstack/nova master: [functional] Avoid leaking greenlet in UnifiedLimits tests https://review.opendev.org/c/openstack/nova/+/892791 | |
| 13:56:28 | auniyal | Hi dansmith, can you please have another look on https://review.opendev.org/c/openstack/nova/+/881457 | |
| 14:17:50 | opendevreview | Balazs Gibizer proposed openstack/nova master: [functional]Fix remaining greenlet leaks https://review.opendev.org/c/openstack/nova/+/892793 | |
| 14:18:41 | gibi | sean-k-mooney: fixed the remaining functional leaks and set the env variable for the functionl env to enforce we don't introduce new ones. | |
| 14:23:05 | gibi | so we can patch the leaks without switching to spawn or using cast as call | |
| 14:23:26 | sean-k-mooney | we can yes | |
| 14:23:44 | sean-k-mooney | i just fixed them becuase i was force to do it when swaping to spawn | |
| 14:23:49 | sean-k-mooney | so i did it in one pathc | |
| 14:24:15 | sean-k-mooney | i was not swaping to spwan to fix tehm i was fixing them so we could swap to spawn form spawn_n | |
| 14:25:19 | sean-k-mooney | i am fine to proceed with your leak fix for the final test | |
| 14:25:25 | sean-k-mooney | and then rebase the spawn change on top | |
| 14:25:33 | sean-k-mooney | so it clearer what is required for that | |
| 14:25:45 | sean-k-mooney | your fixes are less invasive then mine | |
| 14:26:23 | gibi | cool | |
| 14:26:36 | gibi | in the long run I have no problem removing the spawn_n usage from nova | |
| 14:26:52 | gibi | as far as I remember there was no good reason to have it in the first place | |
| 14:27:14 | gibi | it is less resource to use spawn_n but I don't think it is significant compared to spawn | |
| 14:28:16 | sean-k-mooney | so thats the thing | |
| 14:28:34 | sean-k-mooney | technially since its just a greenlet it shoudl be | |
| 14:28:55 | sean-k-mooney | but i dont think in reality it has any really difference | |
| 14:29:04 | sean-k-mooney | and melwitt pointed out a thing i forgot | |
| 14:29:11 | sean-k-mooney | when we were talking to the eventlet maintainers | |
| 14:29:25 | sean-k-mooney | they mentioned that there was some undefiend behavior with raw greenlets | |
| 14:29:40 | sean-k-mooney | so im hoping there will be no effective difference performance wise | |
| 14:29:55 | sean-k-mooney | and correctness wise im hopeing for less edgecases by this change | |
| 15:23:25 | gibi | it is less resource to use spawn_n but I don't think it is significant compared to spawn | |
| 15:23:31 | gibi | sorry | |
| 15:24:06 | gibi | yeah I agree with the overall direction | |
| 15:33:07 | dansmith | auniyal: is it passing tests now? | |
| 15:34:05 | auniyal | yes CI passing :) | |
| 15:47:48 | dansmith | auniyal: one very small (but weird) formatting nit | |
| 15:47:58 | dansmith | auniyal: if you can fix that real quick I'll +2 | |
| 15:48:46 | opendevreview | Dan Smith proposed openstack/nova master: VERYWIP: Support setting alias on libvirt disks https://review.opendev.org/c/openstack/nova/+/892800 | |
| 15:48:46 | opendevreview | Dan Smith proposed openstack/nova master: VERYWIP: Set libvirt device alias for volumes https://review.opendev.org/c/openstack/nova/+/892801 | |
| 15:49:00 | dansmith | oh I guess kashyap isn't here | |
| 15:56:11 | sean-k-mooney | any reason for the ua- prefix for the volume id | |
| 15:56:19 | dansmith | auniyal: comments on the reproducer too | |
| 15:56:28 | dansmith | sean-k-mooney: required by libvirt (as noted in the commit message) | |
| 15:56:47 | dansmith | sean-k-mooney: interestingly if you omit that, it just ignores what you gave it and uses its own value with no warning :/ | |
| 15:57:07 | sean-k-mooney | .. of couse it does | |
| 15:57:48 | sean-k-mooney | im debating in my head the value fo doint that in the call site as you did vs prefixing it here https://review.opendev.org/c/openstack/nova/+/892800/1/nova/virt/libvirt/config.py#1205 | |
| 15:58:38 | dansmith | well, one reason is that libvirt gives everything auto-generated aliases if you don't specify one, | |
| 15:59:06 | dansmith | and so we'd need to strip out the ua- from the parsing if we do it on the input, and then there's no way to tell from the higher layers if it's one we set or libvirt set | |
| 15:59:23 | dansmith | default looks like: | |
| 15:59:23 | dansmith | <alias name='virtio-disk0'/> | |
| 15:59:36 | sean-k-mooney | ok thats good to know. | |
| 16:00:20 | sean-k-mooney | then one suggestion would be add a is a uuid_to_alais funciton | |
| 16:00:28 | sean-k-mooney | so we can just use taht everywhere | |
| 16:00:32 | sean-k-mooney | but thats a minor nit | |
| 16:01:06 | sean-k-mooney | i guess we wont alwasy have a uuid | |
| 16:01:13 | dansmith | right | |
| 16:01:18 | sean-k-mooney | for pci devie form flavor alaises for example | |
| 16:01:29 | dansmith | and root disk, if we wanted to set that | |
| 16:01:46 | dansmith | anyway, I mostly just wanted to make sure we could plumb that through and it would stick in libvirt | |
| 16:01:58 | sean-k-mooney | im not sure if we dont because we dont supprot detaching that but it would help for swap on reize perhaps | |
| 16:02:36 | sean-k-mooney | ya its good to get this basic proof of concept and find the "ua-" issue | |
| 16:02:37 | dansmith | I don't know that we need it at all, I'm just saying if we set the alias for those devices, we won't have a uuid | |
| 16:03:02 | sean-k-mooney | related question | |
| 16:03:20 | sean-k-mooney | do we know if libvirt has an api to get/remove a device via the alais | |
| 16:03:29 | sean-k-mooney | it would be nice if it did but probaly does not | |
| 16:03:45 | dansmith | idk, but it's not required I think | |
| 16:04:29 | sean-k-mooney | ya its likely not but it would be nice just to detach a volume by doing libvir.detach_device(alias=<volume uuid>) | |
| 16:04:39 | dansmith | https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDetachDeviceAlias | |
| 16:04:49 | dansmith | that will be *very* nice, IMHO | |
| 16:04:52 | sean-k-mooney | oh perfect | |
| 16:05:01 | sean-k-mooney | ya so that is what i was hoping for i think | |
| 16:05:32 | sean-k-mooney | it also renforice the idea that this is how libvirt want use to name/specify things to detatch ectra | |
| 16:05:43 | dansmith | up | |
| 16:05:48 | dansmith | also yup | |
| 16:07:01 | sean-k-mooney | so this was added in 3.9.0 which was released in late 2017 so when we first started lookign at teh lun issue it did not exist and when we looed at them last i dont know if we have a high enough min libvirt | |
| 16:07:16 | sean-k-mooney | but for master this looks like a nice cleanup over all | |
| 16:07:33 | sean-k-mooney | thanks for starting a poc of this | |
| 16:08:53 | dansmith | yeah, I'll get going on a spec just to lay some of this otu | |
| 16:09:10 | kashyap | dansmith: Hi! I hear you were looking for me :) | |
| 16:09:11 | dansmith | I'll have to survey where all we look up devices to see where we need to convert them to use this | |
| 16:09:26 | dansmith | kashyap: was just going to point you at this: https://review.opendev.org/q/topic:libvirt-dev-alias | |