| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-16 | |||
| 12:54:54 | openstack | bug 1836212 in OpenStack Compute (nova) "libvirt: Failure to recover from failed detach" [Undecided,New] https://launchpad.net/bugs/1836212 | |
| 12:55:04 | efried | gibi: on it | |
| 12:55:11 | efried | Anyone know where cfriesen has been lately? | |
| 13:12:53 | efried | gibi: are you still here? | |
| 13:17:07 | kashyap | efried: I was wondering, too. | |
| 13:17:12 | kashyap | Probably buried in downstream stuff | |
| 13:17:58 | efried | kashyap: Do you know anything about the libvirt patches to make vTPM more secure? | |
| 13:18:22 | kashyap | efried: I do, peripherially. And funny you ask! | |
| 13:18:40 | kashyap | Just today I was browsing that vTPM thread on upstream libvirt list | |
| 13:19:12 | kashyap | The patch series is at v5, and almost ready to be merged | |
| 13:19:18 | efried | LibVirt RFE: https://bugzilla.redhat.com/show_bug.cgi?id=1728030 | |
| 13:19:18 | efried | RH has already written a patch https://www.redhat.com/archives/libvir-list/2019-July/msg00584.html | |
| 13:19:18 | openstack | bugzilla.redhat.com bug 1728030 in libvirt "RFE: Add encryption of TPM emulator state" [Unspecified,New] - Assigned to libvirt-maint | |
| 13:19:22 | kashyap | efried: https://www.redhat.com/archives/libvir-list/2019-July/msg00854.html | |
| 13:19:31 | kashyap | "PATCH v5 00/20] Add support for vTPM state encryption | |
| 13:19:32 | kashyap | " | |
| 13:19:43 | efried | cool, I don't know how to read any of that, so "almost ready to be merged" is a good bit of info. | |
| 13:20:06 | kashyap | It has gone through a good few rounds of feedback from long-time reviewers upstream | |
| 13:20:12 | efried | when that happens, will there need to be any special affordance in the nova design to enable it? | |
| 13:20:31 | efried | like, do we have to (conditionally based on available libvirt version) do something different to the xml? | |
| 13:20:41 | kashyap | efried: Probably the config classes and XML exposure, and the relevant calls in libvirt/driver.py? | |
| 13:20:59 | efried | I guess at the very least we would have to expose a trait saying that "more secure vTPM is available here" | |
| 13:21:16 | kashyap | Right, on the version conditionals | |
| 13:21:52 | efried | But someone needs to be working on the code in the first place. It hasn't been touched since the PTG. | |
| 13:22:28 | kashyap | efried: Yeah, if the author has gone AWOL, then probably "punt it to backlog"? | |
| 13:22:36 | kashyap | When they come back, it can be revived. | |
| 13:22:38 | efried | no can do | |
| 13:22:53 | kashyap | Sorry, what do you mean? | |
| 13:22:55 | efried | more likely "find new owner asap" | |
| 13:22:58 | kashyap | Ah | |
| 13:23:03 | efried | downstream pressure | |
| 13:23:14 | kashyap | Ah-ha, nod. | |
| 13:23:41 | kashyap | It is a useful, self-contained solution -- for those who want it. | |
| 13:27:05 | kashyap | efried: You do know that the core vTPM stuff is merged in upstream libvirt, yeah? | |
| 13:27:22 | kashyap | (The series linked above is for the vTPM state being ecrypted) | |
| 13:27:24 | efried | kashyap: Yeah | |
| 13:27:49 | efried | like, the base design (what we have merged) could be completed any time somebody wanted to pick it up and run with it. | |
| 13:27:49 | kashyap | Yeah, just double-confirming. Just checked with DanPB, he said he hasn't looked at the v5 libvirt series yet | |
| 13:28:15 | artom | lyarwood, yeah, good point, lemme look into that | |
| 13:33:09 | sean-k-mooney | kashyap: has it been released in a libvirt version | |
| 13:33:22 | kashyap | sean-k-mooney: The vTPM state being encrypted? | |
| 13:33:26 | kashyap | No, it is still in review upstream. | |
| 13:33:35 | kashyap | (As noted above) | |
| 13:33:50 | sean-k-mooney | ok then we cant have a feature that depends on that untill its released. not just merged | |
| 13:34:00 | sean-k-mooney | or in this case still under review | |
| 13:34:17 | sean-k-mooney | but unencrypted vTPM can still proceed | |
| 13:34:59 | kashyap | Right, I know the rule. But as long as it is in an upstream libvirt release, it should damn well be fine to have a feature depend on a brand new upstream release | |
| 13:35:02 | sean-k-mooney | it shoudl hopefully be minor to extend unencypted vTPM to encyrpeted once the feature is released | |
| 13:35:16 | sean-k-mooney | yep | |
| 13:35:33 | sean-k-mooney | just need to be in a livirt releases tar | |
| 13:35:44 | sean-k-mooney | it doesnt need to be in a distro | |
| 13:36:10 | kashyap | Right. That's good. In the past there was an absolute bonkers discussion that libvirt release should "bake" in a distro for a month | |
| 13:36:18 | kashyap | ... and receive some "testing" and some such utter nonsense. | |
| 13:36:24 | kashyap | Glad we are doing away with that crap. | |
| 13:37:01 | sean-k-mooney | well the reason for "master is not enough" is like use other porject somtimes merge stuff and revert it before a release | |
| 13:37:19 | sean-k-mooney | so it has to be released but it does not need to be packaged | |
| 13:37:47 | kashyap | sean-k-mooney: Right, I'm with you on that -- so long as it's in a released tarball, we should be (and are) good. | |
| 13:41:13 | tssurya | thanks gibi :) | |
| 13:44:05 | artom | lyarwood, wrote some words on https://review.opendev.org/#/c/670645/2 | |
| 13:44:19 | efried | artom: btw, I did *not* get around to opening that bug yesterday. | |
| 13:44:25 | efried | in case you had a yen to do it | |
| 13:45:29 | artom | lyarwood, if you agree with said words, I shall do the needful for that as well | |
| 13:46:58 | lyarwood | artom: I am if we can run A against stable/stein, I'm not sure that we can | |
| 13:47:21 | lyarwood | artom: A being OVS hybrid same host | |
| 13:47:25 | sean-k-mooney | artom: that is the backport | |
| 13:47:36 | artom | lyarwood, any reason we can't add it to stein's experimental job? | |
| 13:47:45 | artom | sean-k-mooney, yeah, lyarwood is concerned about testing | |
| 13:47:49 | artom | (rightfully so) | |
| 13:48:09 | lyarwood | artom: yeah I just don't know if we just inherit that all from master or if we need to change stuff | |
| 13:48:13 | sean-k-mooney | you should be able to backport the DNM test | |
| 13:48:21 | lyarwood | I'll try experimental first and go from there | |
| 13:48:46 | sean-k-mooney | and merge in the neutorn-iptabels job too if needed | |
| 13:49:23 | artom | And since https://review.opendev.org/#/c/670593/1 popped up when looking through my reviews trying to find the one where efried'd bug manifested itself, it'd be cool if someone could look at that as well, but there's no "natural" person for that area (simple change, but involved understanding SRIOV and device tagging) | |
| 13:49:50 | sean-k-mooney | lyarwood: we do not have teh iptables job in the experimental pipline on stable/* | |
| 13:50:23 | lyarwood | sean-k-mooney: kk so we need a DNM change right? | |
| 13:50:32 | lyarwood | sean-k-mooney: sorry I wasn't sure which part you were talking about above :D | |
| 13:51:12 | sean-k-mooney | i have https://review.opendev.org/#/c/664442/ but i need to add the neutron iptables jobs and backport it | |
| 13:51:49 | artom | sean-k-mooney, you have to split the 2 changes as well | |
| 13:52:03 | artom | Because we need to test multi-host both without and with hybrid plug | |
| 13:52:05 | sean-k-mooney | i can create a version that will drop the unneeded jobs and just have teh two we need and propsoe the backpor if ye want | |
| 13:52:29 | artom | sean-k-mooney, I'd really appreciated if you handled covering all the test cases, yeah :) | |
| 13:53:31 | sean-k-mooney | so we need 3 or 4 test jobs | |
| 13:54:37 | artom | sean-k-mooney, 3, for cases A, B, D | |
| 13:54:46 | artom | A = neutron-tempest-iptables_hybrid | |
| 13:55:20 | artom | D = first half of https://review.opendev.org/#/c/664442/ (run revert tests in nova-live-migration) | |
| 13:55:26 | sean-k-mooney | b and d is the neutorn multinode job with and without iptables? | |
| 13:55:42 | artom | B = second half of https://review.opendev.org/#/c/664442/ (iptables in nova-live-migration/nova-multinode) | |
| 13:55:46 | sean-k-mooney | ok i can go do that now | |
| 13:56:24 | sean-k-mooney | *nova-multinode which was nova-live-migration | |
| 13:58:50 | mdbooth | Folks, I'm undecided on https://review.opendev.org/#/c/671023/ . It looks to me like a customer actually hit this race in practise while running a heat stack. However, in general I suspect it's fairly uncommon. My preference is generally to fix this stuff, but this kind of cleanup also tends to go ignored. Is a fix for this likely to be of interest? | |
| 14:00:27 | openstackgerrit | Merged openstack/nova master: Remove Rocky-era min compute trusted certs compat check https://review.opendev.org/669539 | |
| 14:01:05 | sean-k-mooney | assuming your runit test actully repoduced the race then it should be fairly staitforwad to show your patch is correct | |
| 14:01:37 | sean-k-mooney | so as long as the fix is not too invasive i dont see why this would be contovertial | |
| 14:01:45 | sean-k-mooney | but i dont know | |
| 14:03:10 | sean-k-mooney | its a race that is proably not hard to debug so if we dont fix it now it will proably bite us later | |
| 14:04:05 | mdbooth | sean-k-mooney: It will require a refactor of a method in libvirt.guest. Nobody likes refactors. | |
| 14:04:29 | sean-k-mooney | mdbooth: can you do it without changing the api signiture | |
| 14:04:42 | mdbooth | Of course, it's internal to the libvirt driver. | |
| 14:05:08 | sean-k-mooney | by api signiture i ment the signiture of the funtion | |
| 14:05:30 | mdbooth | Yes, probably, although the function signature does also need to be fixed. | |