| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-04-02 | |||
| 15:21:32 | dansmith | path=$token seems far more hacky than actually providing a legit url as the path | |
| 15:21:38 | mnaser | yeah, that is valid | |
| 15:22:00 | mnaser | the only concern I have now is *hopefully* that works for both old and new novnc | |
| 15:22:47 | dansmith | not sure why that matters, since we control our package versions (and it's up to the distro to match), but as far as I can tell, they didn't change the path behavior | |
| 15:22:50 | dansmith | or claim not to | |
| 15:25:23 | mnaser | dansmith: nova's CI actually installs novnc from distro pkgs which is probably why this wasn't caught | |
| 15:25:33 | mnaser | and yeah, the path behavior seems to always have been there and continued to be there | |
| 15:25:44 | dansmith | oh? I thought we had an u-c for it | |
| 15:25:55 | mnaser | nope, discovered this yesterday | |
| 15:26:17 | mnaser | we have u-c for Websockify probably | |
| 15:26:21 | mnaser | but novnc isn't even a python package so yeah | |
| 15:26:41 | mnaser | dansmith: https://github.com/openstack-dev/devstack/blob/358cc122c3a6d30bf043b3e478790fd2773e9a88/.zuul.yaml#L220 | |
| 15:27:22 | dansmith | okay | |
| 15:27:29 | dansmith | yeah I guess that makes sense actually | |
| 15:27:31 | sean-k-mooney | mnaser: oh your right we set NOVNC_FROM_PACKAGE=True | |
| 15:35:30 | openstackgerrit | Mohammed Naser proposed openstack/nova master: wip: start using ?path=%3Ftoken%3D= |
|
| 15:35:44 | mnaser | I don't have time to follow up on this too much but I guess its a start for someone to go through and start a discussion | |
| 15:36:26 | dansmith | seems like melwitt has some interest at least | |
| 15:37:14 | mnaser | yeah, so maybe that's the initial copy pasta which will probably fail | |
| 15:39:21 | cfriesen | sean-k-mooney: do you know if nova is supposed to handle PCI passthrough for Intel QAT devices? | |
| 15:39:28 | sean-k-mooney | yes | |
| 15:39:37 | sean-k-mooney | it has worked since before icehouse | |
| 15:39:49 | sean-k-mooney | you can do both pf and vf passhtough | |
| 15:40:48 | sean-k-mooney | i think intel QAT device were perhaps the first usecase that pci passhtough was enabeld for. i know nic came after it | |
| 15:41:00 | cfriesen | sean-k-mooney: we're hitting a weird issue where they're hittting the SRIOV_VF clause in LibvirtDriver._get_device_type() and failing in pci_utils.get_ifname_by_pci_address(). Are we configuring something wrong? | |
| 15:42:08 | sean-k-mooney | cfriesen: do you have a physnet set in the pci whitelist for that device | |
| 15:42:27 | cfriesen | sean-k-mooney: not sure, can check. Should it have one? | |
| 15:42:34 | sean-k-mooney | no | |
| 15:42:43 | sean-k-mooney | that should only be set on nics | |
| 15:43:17 | sean-k-mooney | QAT we expect the type of the device to be PCI for both pf and vf in the pcimanager | |
| 15:43:19 | openstackgerrit | Colleen Murphy proposed openstack/nova stable/stein: Move create of ComputeAPI object in websocketproxy https://review.openstack.org/649374 | |
| 15:43:37 | openstackgerrit | Colleen Murphy proposed openstack/nova stable/rocky: Move create of ComputeAPI object in websocketproxy https://review.openstack.org/649375 | |
| 15:43:44 | sean-k-mooney | but its possible that the code we added for the bandwith based schduing is causing issues | |
| 15:44:49 | sean-k-mooney | cfriesen: https://github.com/openstack/nova/blob/master/nova/pci/request.py#L16-L39 | |
| 15:45:59 | cfriesen | sean-k-mooney: sweet, thanks | |
| 15:46:10 | sean-k-mooney | "product_id": "0443" is the VF and "product_id": "0442" is the pf | |
| 15:47:41 | sean-k-mooney | cfriesen: yep QAT was the thing that intoduce pci passthough https://github.com/openstack/nova/commit/fe67148234dba42468793f33c2ca83ce0616e824 so it really should work still | |
| 15:48:15 | cfriesen | sean-k-mooney: I expect it's a config issue. | |
| 15:49:02 | sean-k-mooney | cfriesen: its posible but the pci_utils.get_ifname_by_pci_address call was intoduce for stien for band with based schudling so there could be a bug. i dont have qat devices to test with | |
| 15:49:37 | sean-k-mooney | well the fucntion existed before we jsut use it in more places now | |
| 15:50:48 | efried | sean-k-mooney: this look kosher to you: https://review.openstack.org/#/c/635533/ | |
| 15:51:00 | efried | dansmith: ^ if you please? | |
| 15:51:35 | sean-k-mooney | xen is not really my thing but ill take a look | |
| 15:52:22 | efried | sean-k-mooney: it's not a xen thing, really an ssl thing. | |
| 15:54:07 | dansmith | it's not an ssl thing, it's a python/oslo thing AFAICT | |
| 15:55:21 | sean-k-mooney | ya its processutils exit code checking | |
| 15:55:34 | sean-k-mooney | but yes on second look it looks sane to me | |
| 15:56:32 | sean-k-mooney | efried: is there any change that the password could be loged to standard error if openssl did not like it | |
| 15:57:05 | openstackgerrit | Eric Fried proposed openstack/nova master: Add minimum value in max_concurrent_live_migrations https://review.openstack.org/648302 | |
| 15:58:07 | efried | sean-k-mooney: I would seriously hope there is no ssl command in existence that will echo back a password to you :) | |
| 15:58:34 | efried | dansmith: only peripherally. They're just changing from "anything on stderr means we should fail" to "nonzero return code means we should fail". | |
| 15:58:48 | dansmith | ...right | |
| 16:00:25 | sean-k-mooney | efried: yes | |
| 16:00:27 | dansmith | efried: the patch asserts that before, writing anything to stderr would cause it to raise, even if it exited with zero, rght? | |
| 16:00:35 | dansmith | I don't see where in oslo that behavior happens | |
| 16:00:43 | dansmith | (it'd be pretty dumb, which is why I'm curious) | |
| 16:01:41 | sean-k-mooney | dansmith: https://github.com/openstack/oslo.concurrency/blob/master/oslo_concurrency/processutils.py#L414-L424 | |
| 16:02:13 | efried | dansmith: It wasn't in oslo, they were triggering the exception here (in nova) based on stderr being nonempty. | |
| 16:02:16 | dansmith | sean-k-mooney: that's not what I'm asking about | |
| 16:02:40 | dansmith | efried: oh? I thought they were asserting that was oslo... | |
| 16:02:55 | dansmith | oh RuntImeError, I see | |
| 16:03:35 | sean-k-mooney | they are assertign that if openssl wrote anything to stderr then nova would raise the runtime error | |
| 16:04:15 | dansmith | efried: that makes it much more openssl-related than I thought, I just zeroed in on the process handling.. so I dunno, probably need someone much more familiar with openssl to validate those semantics | |
| 16:04:23 | sean-k-mooney | im guessing that if openssl ever decied to add a deperaction warning or somehting this caused issues for them | |
| 16:04:49 | dansmith | right, I definitely get that | |
| 16:05:37 | sean-k-mooney | i looks liek they jsut want to be a little more graceful and assume openssl follow the standard unix thing of if it returned and exit code of 0 it succeded | |
| 16:05:43 | sean-k-mooney | which i think is resonable | |
| 16:05:56 | sean-k-mooney | did the bug have an explcit example | |
| 16:06:25 | dansmith | sean-k-mooney: yes, that's all obvious :) | |
| 16:07:08 | sean-k-mooney | yes so the patch is sane yes to efried original quetion | |
| 16:07:24 | sean-k-mooney | the bug was reported as a deprecation warning as i gueesed | |
| 16:07:34 | sean-k-mooney | |RuntimeError: OpenSSL error: *** WARNING : deprecated key derivation used. | |
| 16:07:36 | sean-k-mooney | |Using -iter or -pbkdf2 would be better. | |
| 16:07:36 | dansmith | the original code didn't look at the return code for something security-related and so changing that behavior is potentially pretty impactful | |
| 16:08:40 | sean-k-mooney | that is possibly true yes in this specifc instance othe Runtime error that was raise i think its safe but you are concerend that there coudl be other case where it would not be | |
| 16:09:21 | dansmith | I'm saying they used to fail if anything was written to stderr and now they won't | |
| 16:09:26 | sean-k-mooney | i woudl hope openssl would not exit with code 0 for anything other then sucess but i dont know that for certine even if i belive it very likely | |
| 16:09:33 | dansmith | and depending on what is going on here, that could be, you know, a big deal | |
| 16:09:35 | sean-k-mooney | yes | |
| 16:09:42 | dansmith | depends on the command and what is going on | |
| 16:10:07 | sean-k-mooney | well we can see the command it fixed | |
| 16:10:30 | dansmith | I'm saying someone needs to go make that determination, IMHO and not just blindly approve this | |
| 16:10:38 | sean-k-mooney | we are encrypting input text with a shared key using ase-123-cbc | |
| 16:11:12 | sean-k-mooney | fair im still wondering why we are doing this via the shell in the first place | |
| 16:22:22 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Add functional regression test for bug 1669054 https://review.openstack.org/649386 | |
| 16:22:22 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Fix functional tests for USE_NEUTRON https://review.openstack.org/649385 | |
| 16:22:23 | openstackgerrit | Matt Riedemann proposed openstack/nova stable/pike: Do not persist RequestSpec.ignore_hosts https://review.openstack.org/649387 | |
| 16:22:23 | openstack | bug 1669054 in OpenStack Compute (nova) stein "RequestSpec.ignore_hosts from resize is reused in subsequent evacuate" [Medium,In progress] https://launchpad.net/bugs/1669054 - Assigned to Matt Riedemann (mriedem) | |
| 16:23:41 | openstackgerrit | Merged openstack/nova master: Correct lower-constraints.txt and the related tox job https://review.openstack.org/622972 | |
| 16:23:50 | openstackgerrit | Merged openstack/nova master: Adding tests to demonstrate bug #1821824 https://review.openstack.org/647957 | |
| 16:23:51 | openstack | bug 1821824 in OpenStack Compute (nova) "Forbidden traits in flavor properties don't work" [Undecided,In progress] https://launchpad.net/bugs/1821824 - Assigned to Magnus Bergman (magnusbe) | |
| 16:24:02 | dansmith | looks like dimitri did a fairly decent analysis of the openssl code, which is good, but I also wonder if we couldn't just ignore lines that start with "WARNING" or something from the stderr and retain some of the original behavor | |
| 16:24:11 | dansmith | anyway, I don't really have time to dig into it super deep | |
| 16:25:42 | openstackgerrit | Merged openstack/nova master: Add placement as required project to functional py36 and 37 https://review.openstack.org/649068 | |
| 16:34:22 | openstackgerrit | Merged openstack/nova master: libvirt: Use 'writeback' QEMU cache mode when 'none' is not viable https://review.openstack.org/641981 | |
| 16:35:19 | cfriesen | sean-k-mooney: apparently nova-compute chokes on startup with this particular QAT hardware even with no whitelist/alias entries in nova.conf. | |
| 16:41:20 | sean-k-mooney | cfriesen: ok then this is likely related to gibi_off's change to auto lookup the netdev name for bandwith based scheduling | |
| 16:42:42 | cfriesen | sean-k-mooney: yeah, confirmed that we're dying in the code gibi_off added in Dec 2018. | |
| 16:44:34 | cfriesen | sean-k-mooney: we're hitting this with the standard embedded Intel QAT, so it's going to cause grief with standard hardware | |
| 16:45:13 | sean-k-mooney | its proably this bit https://github.com/openstack/nova/commit/c02e213d507c830427a86d6a4bb4f7a2f5158590#diff-f4019782d93a196a0d026479e6aa61b1R5938 | |