| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-02-23 | |||
| 15:12:53 | dansmith | bauzas: okay, can you slap a quick comment on there? I'll work with him to split after the next rev | |
| 15:13:06 | dansmith | bauzas: I assume this is still on the slate for merging yeah? | |
| 15:13:12 | bauzas | dansmith: doing it nbow | |
| 15:13:20 | dansmith | bauzas: <3 | |
| 15:13:27 | sean-k-mooney | mfo: teh secvure boot firemware shoudl not require secure boot to function it just supports it | |
| 15:13:56 | sean-k-mooney | mfo: so nova should not need to have any logic to select the non secure boot version | |
| 15:14:37 | sean-k-mooney | mfo: i aslo done se any fix againt master | |
| 15:14:51 | bauzas | dansmith: done | |
| 15:14:59 | dansmith | thanks | |
| 15:15:01 | bauzas | I explained why it's important to split | |
| 15:15:03 | sean-k-mooney | you seam to have started with a sable only patch https://review.opendev.org/c/openstack/nova/+/828980 | |
| 15:15:20 | sean-k-mooney | which i dont think is correct | |
| 15:18:40 | mfo | sean-k-mooney, thanks for looking. i'll follow up on the sec boot firmware (no) needs that you mentioned. and this is "fixed" on wallaby and later/master w/ the implementation of secboot spec/support, due a refactor (this a bit buried in the commit msg, but the research has been done to go w/ stable-only. :) | |
| 15:19:22 | mfo | and, seconding pmonteir above, thx for answering so quickly! | |
| 15:19:28 | sean-k-mooney | mfo: the secure booth ovmf image shoudl work without secure boot enabled howevdr | |
| 15:19:59 | sean-k-mooney | we should not need to filter like this form my understandig | |
| 15:20:06 | sean-k-mooney | do you know why this is requried | |
| 15:20:18 | mfo | ack, i'll be following up on that. | |
| 15:20:55 | sean-k-mooney | if we can get a statement as to why this is requried we could proably proceed with the minimal stabel only backport | |
| 15:21:11 | sean-k-mooney | but that obviouly has risk too | |
| 15:21:28 | sean-k-mooney | i agree that backportign the secure boot feature is likely not viable | |
| 15:22:25 | mfo | er, it was my assumption that it didn't work bcz of secboot ovmf image requirements; but per your statement, it might as well be something w/ ubuntu's qemu or ovmf pkgs. which i'll go check/debug further on why secboot.fd isn't booting. | |
| 15:22:32 | sean-k-mooney | but we woudl at least need som unit tests to vover this change | |
| 15:24:33 | mfo | sean-k-mooney, understood. you mean something like just booting VMs in bios and uefi mode, or something more specific like checking the rendered libvirt xml and check for the ovmf image used too? | |
| 15:25:02 | sean-k-mooney | so to test https://review.opendev.org/c/openstack/nova/+/828979/2/nova/virt/libvirt/driver.py#5834 | |
| 15:25:19 | sean-k-mooney | we should ensure in a unit test that we include a ovm path that has .secboot.fd | |
| 15:25:30 | sean-k-mooney | as the first option | |
| 15:25:38 | sean-k-mooney | and that the non secboot.fd path is chosen | |
| 15:25:59 | sean-k-mooney | we should also check what appens if all the path have secboot.fd | |
| 15:26:31 | sean-k-mooney | also i wonder if all distos use the same nameing convention | |
| 15:27:11 | mfo | got it. | |
| 15:27:32 | chateaulav | so i have additional package installs defined in the stack.sh for my proposed CI to ensure devstack has the supported requirements. is that the correct place or is there a better method due to how it builds. these only need installed on the compute | |
| 15:27:35 | opendevreview | Rajat Dhasmana proposed openstack/python-novaclient master: Add parameter to rebuild boot volume https://review.opendev.org/c/openstack/python-novaclient/+/827163 | |
| 15:29:06 | mfo | sean-k-mooney, since we're testing against a limited option set (the hardcoded paths array), there's only that nameing convention to check (if i got your point right :) | |
| 15:29:25 | bauzas | wow, time flies and I'm on the policy changes | |
| 15:29:28 | bauzas | ... | |
| 15:31:15 | sean-k-mooney | mfo: so on centos 9 there is no version aviiable without secureboot form the set | |
| 15:31:49 | sean-k-mooney | '/usr/share/OVMF/OVMF_CODE.secboot.fd' is the only one of the 3 that is present | |
| 15:32:30 | sean-k-mooney | mfo: there is a singel non secureboot capable image at /usr/share/edk2/ovmf/OVMF_CODE.cc.fd | |
| 15:32:35 | sean-k-mooney | but that will not be checked | |
| 15:33:12 | mfo | sean-k-mooney, ok. so, if for some reason that patch is still required, we're better checking if there's another option available before ignoring secboot.fd. | |
| 15:33:27 | sean-k-mooney | yes | |
| 15:33:44 | sean-k-mooney | so redhat will be releaseign osp 17 based on stable wallaby on rhel 9 later this year | |
| 15:34:05 | sean-k-mooney | and the current patch would break uefi i belive in that case | |
| 15:34:32 | sean-k-mooney | however i dont know if we have the same issue where we woudl need to use the '/usr/share/OVMF/OVMF_CODE.fd' image if secure boot is not requested | |
| 15:34:52 | bauzas | gmann: around ? | |
| 15:34:56 | bauzas | or maybe dansmith ? | |
| 15:34:59 | bauzas | context is https://review.opendev.org/c/openstack/nova/+/828670/4/nova/api/openstack/compute/server_groups.py | |
| 15:35:08 | gmann | bauzas: hi | |
| 15:35:11 | bauzas | we'll change the API behaviour | |
| 15:35:27 | bauzas | as you need to be in the right project to getting the server groups | |
| 15:35:32 | bauzas | it's OK for me | |
| 15:35:32 | mfo | hmm, but this isnt an issue in wallaby, right? as it uses a diff method to pick the ovmf files, based on descriptor files from qemu (eg, /usr/share/qemu/firmware/) | |
| 15:35:33 | bauzas | but, | |
| 15:35:41 | mfo | sean-k-mooney, ^ | |
| 15:35:50 | dansmith | bauzas: that's *your* project id | |
| 15:36:02 | bauzas | given it will change the behaviour, do we all agree we don't need a microversion ? | |
| 15:36:06 | dansmith | bauzas: so it should be no different than today | |
| 15:36:20 | bauzas | dansmith: well, | |
| 15:36:27 | sean-k-mooney | mfo: ack ya it might not be an issue there | |
| 15:36:30 | bauzas | previously you were giving none as a value | |
| 15:36:33 | mfo | sean-k-mooney, ok cool. | |
| 15:36:36 | sean-k-mooney | as you said its using a differnt method | |
| 15:36:49 | bauzas | dansmith: tomorrow, you'll get an exception, right? | |
| 15:36:49 | sean-k-mooney | but we need tobe careful not to break other distors is my point | |
| 15:37:04 | sean-k-mooney | mfo: so we might need to sort the options or soemthing instead | |
| 15:37:11 | dansmith | bauzas: this is already limited to admin yeah? | |
| 15:37:12 | mfo | sean-k-mooney, absolutely; thx for your insight on that. | |
| 15:37:38 | bauzas | dansmith: ah i see | |
| 15:38:08 | mfo | sean-k-mooney, i'll go check the secboot ovmf image boot issues, and abandon/update the patches as appropriate. | |
| 15:38:11 | bauzas | dansmith: you'll get all the servergroups for your project id | |
| 15:38:15 | dansmith | bauzas: this is just providing a basically no-op target object so that the check that includes admin and project_id works | |
| 15:38:18 | mfo | sean-k-mooney, thanks again for you help and advice on this. | |
| 15:38:23 | gmann | bauzas: dansmith humm this is "all_projects server group list" which we said to allow all admin from any proejct | |
| 15:38:40 | dansmith | bauzas: wait, are you talking about server groups or all_projects? | |
| 15:38:50 | gmann | bauzas: dansmith if we do not pass project_id then it will fail as PROJECT_ADMIN expect project id | |
| 15:38:50 | bauzas | dansmith: then my only concern is why it's for "all_projects" ? | |
| 15:39:03 | bauzas | dansmith: my question is for https://review.opendev.org/c/openstack/nova/+/828670/4/nova/api/openstack/compute/server_groups.py | |
| 15:39:05 | dansmith | oh, I was looking at the wrong window, not the one from your link sorry | |
| 15:39:21 | bauzas | dansmith: here, we go into all_projects but we restrict to the specific project_id | |
| 15:39:49 | bauzas | I don't see why we need it then | |
| 15:39:53 | gmann | bauzas: it is context_project id so any admin requesting can pass the policy | |
| 15:40:04 | dansmith | bauzas: it's just because the target needs to have the things we're checking | |
| 15:40:05 | gmann | we can see here, legacy admin is allowed https://review.opendev.org/c/openstack/nova/+/828670/4/nova/tests/unit/policies/test_server_groups.py#224 | |
| 15:40:07 | dansmith | it doesn't do any filtering | |
| 15:40:10 | gmann | yeah | |
| 15:40:29 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Add functional tests to reproduce bug #1960412 https://review.opendev.org/c/openstack/nova/+/830010 | |
| 15:40:30 | opendevreview | Alexey Stupnikov proposed openstack/nova master: Run clean up calls when queued live migration is aborted https://review.opendev.org/c/openstack/nova/+/828570 | |
| 15:41:24 | bauzas | gmann: dansmith: this, I understand but why are we targeting the specific project_id ?. | |
| 15:41:37 | bauzas | in the context I mean | |
| 15:41:53 | dansmith | it's *your* project_id, not an instance project_id or a server group project_id | |
| 15:41:57 | dansmith | so it will always match | |
| 15:42:11 | dansmith | because it's in the rule | |
| 15:42:14 | bauzas | dansmith: it's noop, right? | |
| 15:42:18 | gmann | bauzas: that is our default checks as per PROJECT_ADMIN check_str which require project_id to be present | |
| 15:42:37 | dansmith | yes, but since we re-use that rule elsewhere, we need a project_id in there that will match, since we don't care about actual project_id checking for this resource | |
| 15:42:56 | dansmith | it's confusing for sure.. it's one of the reasons I *hate* our policy engine | |
| 15:42:59 | gmann | https://github.com/openstack/nova/blob/master/nova/policies/base.py#L118 | |
| 15:43:09 | dansmith | it's just impossible to look at and understand without study each time | |
| 15:43:10 | bauzas | dansmith: OK, I trust you then | |