| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-06-26 | |||
| 17:57:01 | tobias-urdin | colby_: do you get any errors or similar? do you have some more info about what you are running, the only issue we have currently with vgpu part is https://review.opendev.org/c/openstack/nova/+/838976 | |
| 18:03:34 | colby_ | tobias-urdin: unfortunately no errors other than no resources. I have looked all through system logs for any sort of errors. mdevctl shows all the devices still used and nvidia-smi vgpu -c show no vgpu available. | |
| 18:04:54 | colby_ | if I do the following command: echo "1" > nvidia-563/devices/8f5be0ec-3a4b-4e72-b613-4e93a27951a0/remove | |
| 18:05:24 | colby_ | then nvidia-smi -c shows available gpu and mdevctl shows it not being used any more. I am then able to spin up another vgpu instance | |
| 18:06:06 | colby_ | that is being done in the /sys/bus/pci/devices/<virtual function cpi address>/mdev_supported_types directory | |
| 18:06:32 | colby_ | we are on Centos8 stream and using Victoria version of openstack currently | |
| 18:07:42 | colby_ | example mdevctl output: 67e63f1e-07f2-474f-874c-826a024c10ec 0000:21:01.7 nvidia-563 manual | |
| #openstack-nova - 2022-06-27 | |||
| 07:15:57 | bauzas | good morning Nova | |
| 07:24:34 | gibi | o/ | |
| 07:35:34 | bauzas | one day off and a lot of emails to triager | |
| 08:18:18 | gibi | frickler: re https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013410 we have an upstream bug for that https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1013410 and it is being fixed by https://review.opendev.org/c/openstack/nova/+/845922 | |
| 08:18:32 | gibi | bah, I mean upstream bug https://bugs.launchpad.net/nova/+bug/1978817 | |
| 08:26:36 | gibi | colby_: I think we saw similar issues before but I don't find the relevan bug report now I suggest to ping bauzas as he might have more context with vgpus/mdevs | |
| 08:26:55 | bauzas | context ? | |
| 08:27:35 | gibi | bauzas: 19:49 < colby_> Hello everyone. we are experienceing a strange but with vGPU. We are using Virtoria on Centos 8 Stream with A40 gpus in SRIOV setup. Everything works correctly except when we delete an instance. It does not seem to release the virtualfunction mdev device. I can manually release them by echoing to the remove method of the mdev. So its like nova is not doing that. | |
| 08:27:55 | gibi | bauzas: that was from friday | |
| 08:30:35 | frickler | gibi: ah, cool, thx for the pointer | |
| 08:30:47 | gibi | frickler: I replied in the debian tracker too | |
| 08:32:39 | bauzas | gibi: colby_: yes we don't delete the mdevs | |
| 08:33:09 | bauzas | 4 years ago, I thought it was not needed | |
| 09:30:47 | Uggla | Hello, bauzas, gibi, sean-k-mooney https://review.opendev.org/c/openstack/nova/+831507, I have fixed latest comments from bauzas and I have verified it works fine with devstack. Can you go ahead with a new review round ? | |
| 09:31:07 | sean-k-mooney | gibi: we are tarcking that as a downstream bug | |
| 09:31:28 | sean-k-mooney | im suggesting that we shoudl free the mdev when we delete the vm | |
| 09:31:39 | sean-k-mooney | we could resue the mdev on the next vm creation but | |
| 09:32:03 | sean-k-mooney | it think that will be problematic if we start using mdevs more dynamicaly in the future | |
| 09:32:20 | sean-k-mooney | it would work for our current usecase but i would prefer to only allocate mdevs if they are in use | |
| 09:33:02 | sean-k-mooney | Uggla: gerrit says no | |
| 09:33:18 | sean-k-mooney | Uggla: specifically it give a 404 | |
| 09:33:27 | sean-k-mooney | and a | |
| 09:33:29 | sean-k-mooney | ¯\_(ツ)_/¯ | |
| 09:33:45 | Uggla | https://review.opendev.org/c/openstack/nova-specs/+/833669 | |
| 09:34:11 | sean-k-mooney | that works fine | |
| 09:35:13 | Uggla | Here is the good one: https://review.opendev.org/c/openstack/nova/+/831507 | |
| 09:36:40 | Uggla | sean-k-mooney, please look at the link ^ | |
| 09:38:03 | sean-k-mooney | ah it was miising a / | |
| 09:39:04 | sean-k-mooney | so looking a the spec you are just missing the parmater to pass to the grant api to resolve the locking issue | |
| 09:39:22 | sean-k-mooney | for manila | |
| 09:41:54 | sean-k-mooney | im not a huge fan of _unset_field_sentinel but i guess that works | |
| 09:47:17 | Uggla | sean-k-mooney, regarding manila yes. I have reviewed the spec locally, I will push it ASAP. Before the 5th hopping it will be approved and merged. | |
| 09:51:25 | Uggla | sean-k-mooney, fyi _unset_field_sentinel was proposed and agreed from gibi and bauzas, so I used that as an humble padawan. :) | |
| 09:51:54 | gibi | sean-k-mooney: about the sentinel, do you see a viable alternative? | |
| 09:51:58 | sean-k-mooney | right i just dont like how pervaisve the check is | |
| 09:52:44 | sean-k-mooney | gibi: nothing substaitally differnt i proably would have made it a module level constant and uppercase it rather then a class variable | |
| 09:53:04 | sean-k-mooney | and i might have done the test once and assigned it to a varible at the top of the function | |
| 09:53:41 | sean-k-mooney | no_az = new_az == AZ_SENTINAL | |
| 09:53:56 | gibi | sean-k-mooney: I have no hard opinion abou the location, could be on module level too. | |
| 09:53:57 | sean-k-mooney | if not no_az and new_az: ... | |
| 09:54:15 | sean-k-mooney | well its just a nit | |
| 09:54:24 | sean-k-mooney | as i said it does not really change anything | |
| 09:54:35 | gibi | on the test side, I agree, that could be factored out | |
| 09:54:53 | sean-k-mooney | just stypictlly i like constnats to eb UPPERCASE_WITH_UNDERSCORES | |
| 09:55:10 | sean-k-mooney | *stylisticly | |
| 09:56:50 | sean-k-mooney | Uggla: so realisticaly you do not need to change anything it just not an established pattern in our code base to have sentenial like this so it looks odd because its different then our normal pattern | |
| 09:57:28 | gibi | bauzas: ^^ look sean-k-mooney also feels it is not an established pattern :D | |
| 09:57:59 | gibi | sean-k-mooney: context: we had a bit of back and forth with bauzas around the sentinel | |
| 09:58:36 | sean-k-mooney | we dont currntly have the equivalent of std:optional in python/nova | |
| 09:59:10 | gibi | bauzas pointed out that the sentinel + kwargs is widely used in the nova rpc apis | |
| 09:59:10 | sean-k-mooney | i would kind of perfer to have that class but i know other recoile at c++ conventions | |
| 09:59:31 | sean-k-mooney | we use none as the sentinal there no? | |
| 10:00:31 | sean-k-mooney | i dont really like that the custom sentinal we use is not "truthy" so we can just use if directly | |
| 10:00:46 | gibi | I think we use kwargs not to send a value even | |
| 10:02:58 | sean-k-mooney | we use None however as the sentinal https://github.com/openstack/nova/blob/c53ec4e48884235566962bc934cbf292ad5b67b8/nova/compute/manager.py#L11017-L11019= | |
| 10:03:11 | sean-k-mooney | so what Uggla is doing is very differnt | |
| 10:04:31 | sean-k-mooney | the use of None as a sential for kwargs is idiomatic python | |
| 10:04:33 | Uggla | sean-k-mooney, fyi bauzas provided this as a sentinel example: https://github.com/openstack/nova/blob/master/nova/scheduler/rpcapi.py#L152 and https://github.com/openstack/nova/blob/master/nova/scheduler/manager.py#L145 so I try to respect that pattern. | |
| 10:05:37 | sean-k-mooney | i see | |
| 10:05:42 | sean-k-mooney | i still dont like that | |
| 10:05:47 | Uggla | :) | |
| 10:05:53 | sean-k-mooney | its kind of like raw gotos | |
| 10:06:05 | sean-k-mooney | we have if while ectra for a reason | |
| 10:06:42 | gibi | we cannot use None in the current case as None has a different meaning than the missing param | |
| 10:07:17 | gibi | as we want to be able to signal either unpin or no-change | |
| 10:07:20 | sean-k-mooney | yes im aware | |
| 10:07:32 | gibi | so we needed an extra value | |
| 10:07:49 | sean-k-mooney | yep that is what std:optional does in c++ | |
| 10:08:00 | sean-k-mooney | it give you a value that is outside the normal set | |
| 10:08:03 | sean-k-mooney | same with ovo | |
| 10:08:11 | sean-k-mooney | the field have an addtional unset value | |
| 10:08:37 | gibi | yeah, I don't like the ovo way of missing fields, as that breaks the class invariant for me | |
| 10:08:56 | sean-k-mooney | well i prefer it to this | |
| 10:09:13 | sean-k-mooney | the ovo way at least work with in and is encapulated | |
| 10:10:04 | gibi | I create an ovo class with predefined fields, and then I have to check at each access if the field exists, that seem against the fact that we predefine the fields | |
| 10:10:25 | sean-k-mooney | i dont like that we have to rememeber that this is not just an az its a union of an az and a sentinal | |
| 10:10:26 | gibi | anyhow I go back to review the unshelve patch | |
| 10:10:51 | gibi | sean-k-mooney: so you would add an extra param to the call instead? like az_provided ? | |
| 10:11:01 | gibi | that way we can avoid the sentinel | |
| 10:11:31 | gibi | new_az can be None to unpin or a valid AZ name, and az_provided boolean can be set if no AZ field is provided in the REST request | |
| 10:11:36 | sean-k-mooney | perhaps or wrap the parmters in a class | |
| 10:12:22 | sean-k-mooney | i would think that az_provided would be more readable | |
| 10:13:07 | sean-k-mooney | so ideally even if we had a sentinil i would like use to do that test once at the top of the function and defien a local az_provided varible | |
| 10:13:27 | sean-k-mooney | that better expresses the intent | |
| 11:37:58 | opendevreview | Amit Uniyal proposed openstack/nova master: Adds validation for hw machine type in host caps https://review.opendev.org/c/openstack/nova/+/847126 | |
| 11:40:36 | bauzas | Uggla: sean-k-mooney: gibi: sorry folks, I was cooking and lunching with my kid | |
| 11:42:00 | bauzas | sean-k-mooney: not sure I understand why you unlike the sentinel pattern | |
| 11:42:13 | bauzas | sean-k-mooney: and why you ask for a new parameter | |
| 11:42:29 | sean-k-mooney | bauzas: its very very error prone | |
| 11:42:38 | bauzas | ? | |
| 11:42:49 | sean-k-mooney | you need to keep the context that the vraiable is a union of a sential and the az | |
| 11:42:55 | sean-k-mooney | and everywhere you use it test for both | |
| 11:43:15 | sean-k-mooney | its very easy to miss that | |