| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-07-02 | |||
| 17:34:46 | efried | jroll: So what penick is saying, IIUC, is that we should blindly pass the TPM through, and worry about the orchestration layer to know what's on it at any given time. I.e. nova doesn't do any prep or checking or clearing or ownership assigning. | |
| 17:35:15 | efried | That sounds like a pretty big security risk to me. Kind of doubt it's going to fly, design-wise, with the Nova maintainers. | |
| 17:35:16 | dansmith | how is that not a terrible idea? | |
| 17:35:21 | efried | yeah | |
| 17:35:34 | dansmith | I mean, I get that it's the minimum required effort to get what you want, but... | |
| 17:35:39 | dansmith | doesn't seem like a smart decision at all | |
| 17:36:27 | jroll | right, it seems like our use case would be long-running VMs | |
| 17:36:40 | sean-k-mooney | efried: that is not too unsuerpising. older gpus had the same issue. unless the uefi firmware is specificlay coded to handel it pci passthou deveice can become unusable until they are reset ususally that reuires a host reboot. | |
| 17:36:40 | jroll | and/or these hosts would only be accessible by this tenant | |
| 17:36:51 | sean-k-mooney | and now that i have my phone back to cooking | |
| 17:37:30 | jroll | I'm not sure I agree that we should just let nova blindly land stuff on a TPM without checking it, fwiw | |
| 17:38:35 | jroll | I haven't thought through what it would take to have nova track if a TPM is 'usable' or not | |
| 17:38:36 | efried | Right. That's Nova abdicating a pretty serious burden of responsibility to the unknown upper layer. I mean, we have trust models in place that are a certain shape and this really seems to be breaking outside of that shape. | |
| 17:38:59 | efried | so given that that approach is likely to be -many, unless there's some other place we can go with this, we're probably going to have to declare it dead in the water. (aka "downstream it if you really need it") | |
| 17:39:36 | dansmith | I'd like to see more uses for it | |
| 17:39:54 | jroll | I assume assigning some sort of TPM_DIRTY trait isn't going to fly, right? | |
| 17:40:06 | dansmith | while I feel for the situation of trying to push people to vms by providing things that look like hardware, the "single long-lived vm per host" use case doesn't hold much weight for me | |
| 17:40:46 | efried | jroll: If we were going to remove it from service via placement-ism, we would probably use the reserved=total trick. | |
| 17:40:54 | efried | rather than a trait | |
| 17:41:20 | jroll | dansmith: that's fair. the reason we would do this on a VM instead of bare metal is for neutron security groups, boot from volume, etc | |
| 17:41:39 | efried | (and btw, implied in your question and my response is: one RP per TPM) | |
| 17:41:53 | dansmith | but we also need to know how to clean it and I think that depending on placement as a persistence mechanism for something like this is an abuse.. it'd be too easy for an operator to just delete that allocation not knowing why it's there, only to have us hand the tpm to something else | |
| 17:42:04 | jroll | or rather, that's why the "single long-lived vm per host" is a somewhat valid use case for us | |
| 17:45:10 | jroll | efried: I guess I'm not deep enough in nova to offer suggestions on how to manage this, I'll have to dig around some and maybe come back and talk about it | |
| 17:45:37 | jroll | unless "tracking the state of the hyp" is 100% not going to happen for something like this | |
| 17:46:12 | efried | jroll: We would have to do it with an external file of some kind. | |
| 17:46:28 | efried | unless there's a way to know by querying the device | |
| 17:46:40 | efried | or sysfs or whatever | |
| 17:46:54 | jroll | hmm ok | |
| 17:47:31 | efried | but sysfs is unlikely to track instance UUID for us | |
| 17:47:34 | dansmith | efried: I think you mean the database | |
| 17:47:50 | dansmith | efried: we don't need to introduce a new persistence mechanism on the host for this as a one-off thing | |
| 17:47:52 | efried | dansmith: Yeah, that would work :) | |
| 17:48:18 | dansmith | sysfs is ephemeral, so it's definitely not going to help us here :) | |
| 17:48:36 | efried | there would have to be some kind of db manage command to manually clear the state | |
| 17:48:53 | efried | because, like, we can't assume it's clear on a reboot or a compute service startup or whatever. | |
| 17:49:02 | dansmith | it's specifically not supposed to be clear on reboot | |
| 17:49:25 | efried | it's supposed to be: purge, reboot, now it's clear so manually say it's clear. | |
| 17:49:33 | dansmith | and if you're suggesting that we hand out tpms to an instance, and then when that instance is deleted, we're stuck until the operator manually clears the tpm and then frees it up with a manage command, | |
| 17:49:41 | dansmith | that's crazypants | |
| 17:49:43 | efried | yup, it would have to be that. | |
| 17:49:45 | efried | And I agree. | |
| 17:49:52 | dansmith | that's implementing half a feature because some guy asked for it and is fine with the manual cleanup | |
| 17:49:57 | dansmith | I'm -5 on that | |
| 17:50:12 | efried | (my earlier estimate of -many wasn't too far off then) | |
| 17:50:25 | efried | yeah, the right way to do this is vTPM | |
| 17:50:45 | efried | but as currently implemented that's not secure because the Master Key is stored on the hypervisor's disk. | |
| 17:51:08 | dansmith | yeah, solving it properly is making that more reasonable I think | |
| 17:51:16 | dansmith | which seems quite doable | |
| 17:51:18 | efried | They're figuring out some way around that in qemu, but it hasn't happened yet, so this was an attempt to work around it until that's available. | |
| 17:51:30 | dansmith | mmhmm | |
| 17:53:43 | efried | So jroll, it's really looking like this isn't going to happen, upstream, in this way. If there's enough $motivation$ to make it happen downstream anyway, let me know and I can advise a bit on the nova-isms and placement-isms that would need to be hacked together. | |
| 17:54:36 | jroll | efried: thanks | |
| 17:56:24 | efried | adrianc: whoops, https://review.opendev.org/#/c/659101/ is in merge conflict now. Since you're going to need a new PS anyway, want to take care of those couple of nits? | |
| 18:09:01 | openstackgerrit | Stephen Finucane proposed openstack/nova-specs master: Add 'flavor-extra-spec-image-property-validation-extended' spec https://review.opendev.org/638734 | |
| 18:13:53 | stephenfin | efried: Tackled those comments on the extra spec validation spec. Lemme know if what I want to do wrt key validation is not clearer now | |
| 18:14:16 | efried | ack | |
| 18:22:55 | markguz_ | hi all. Anyone out there got any experience with using NPIV and pci passthrough for share Fiber Channel HBAs amongst vms? | |
| 18:23:28 | markguz_ | i want to share the VFs but am not really clear on how it might be done. | |
| 18:23:56 | markguz_ | most if not all of the docs seem to focus on networking virtual functions | |
| 18:28:35 | markguz_ | there is a type=VF but i'm not clear how to share the vport | |
| 18:34:12 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Add PENDING vm state https://review.opendev.org/648687 | |
| 18:38:54 | openstackgerrit | Lee Yarwood proposed openstack/nova master: libvirt: Add a rbd_connect_timeout configurable https://review.opendev.org/667421 | |
| 18:52:06 | openstackgerrit | Eric Fried proposed openstack/nova master: [FUP] Follow-up patch for SR-IOV live migration https://review.opendev.org/659101 | |
| 18:54:17 | efried | adrianc: ftfy ^ | |
| 18:54:41 | efried | stephenfin: if you're still around, trivial re+A ^ | |
| 19:09:15 | openstackgerrit | Merged openstack/nova-specs master: Add PENDING vm state https://review.opendev.org/648687 | |
| 19:14:15 | openstackgerrit | Matt Riedemann proposed openstack/nova master: Update COMPUTE_STATUS_DISABLED from set_host_enabled compute call https://review.opendev.org/668743 | |
| 19:20:28 | mriedem | dansmith: could use another opinion here https://review.opendev.org/#/c/668743/1/nova/compute/manager.py@5047 | |
| 19:23:40 | dansmith | mriedem: do I need to read it or can I just give you an opinion? | |
| 19:23:59 | mriedem | like, cats > dogs? | |
| 19:23:59 | mriedem | sure | |
| 19:24:11 | dansmith | pizza > cookies | |
| 19:24:28 | mriedem | i'd agree with that | |
| 19:24:49 | mriedem | https://sallysbakingaddiction.com/chocolate-chip-cookie-pizza/ | |
| 19:24:56 | mriedem | a love child | |
| 19:28:39 | dansmith | mriedem: okay I'm not sure I get it | |
| 19:28:45 | dansmith | we have this set_host_enabled call already yeah? | |
| 19:29:05 | dansmith | and the base virt driver does what when you call it? raise not implemented? | |
| 19:29:11 | mriedem | yes, only used by the os-hosts api which was deprecated in 2.43 and only implemented by the xenapi driver | |
| 19:29:14 | mriedem | yes | |
| 19:29:39 | dansmith | oh, I see, different from service disable yeah? | |
| 19:29:43 | mriedem | yes, | |
| 19:29:48 | dansmith | right right, okay | |
| 19:29:49 | mriedem | but i'm going to use it for service disable/enable in the api change | |
| 19:29:56 | dansmith | yeah | |
| 19:29:57 | dansmith | so, | |
| 19:30:17 | dansmith | the thing you're concerned about is replicating the 501 "this thing doesn't support that" case for libvirt and v2.1? | |
| 19:30:38 | mriedem | it's a minor concern, but yeah | |
| 19:30:39 | dansmith | I guess I'm not sure why that matters.. the fact that libvirt does or doesn't support the thing has nothing to do with microversion 2.1 (or any version) | |
| 19:31:00 | mriedem | the version really only matters b/c the api no longer works with 2.43+ | |
| 19:31:06 | mriedem | for any driver | |
| 19:31:24 | dansmith | okay but.. why does it matter if someone calls it with v2.1 on libvirt and it works? | |
| 19:31:45 | mriedem | because it's a behavior change... | |
| 19:31:50 | dansmith | it's not | |
| 19:31:58 | dansmith | it's an implementation detail | |
| 19:32:14 | dansmith | if the thing (which the user can't see) supports it, it works, and if not it's 501 | |
| 19:32:40 | mriedem | or 400 if i just return None | |
| 19:32:41 | dansmith | they can't see that libvirt didn't support it before, got a 501, and that either now it does support it, or some other driver is being used | |
| 19:33:03 | mriedem | in this case the user == admin anyway, | |