| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2019-11-11 | |||
| 20:39:17 | dansmith | sean-k-mooney: he's saying some user wanting to not be live migrated for risk, | |
| 20:39:25 | dansmith | not nova refusing to migrate vtpm-having instances | |
| 20:39:30 | sean-k-mooney | ah | |
| 20:39:34 | efried | either user (so in the flavor) or admin (conf opt) | |
| 20:39:39 | dansmith | if we can migrate them (which we can I think) then we should | |
| 20:39:46 | dansmith | efried: user doesn't control the flavor | |
| 20:39:48 | sean-k-mooney | thats a similar discustion to the spec fo exposing post copy | |
| 20:39:51 | dansmith | the *details* of the flavor | |
| 20:40:22 | dansmith | but yeah, I'd say the cloud should say "Flavors that end in -secu will never be live migrated for security reasons" and then just handle that policy themselves | |
| 20:40:30 | sean-k-mooney | in that spec they wanted a way to mark a vm as not using post copy to avoid the risk of remote execution over a netwrok link | |
| 20:42:38 | umbSublime | That's something I was asked today. Is there a way to make a vm/flavor _NOT_ live-migratable. Some of our users would for some reason want to opt-out of being live-migratable and understand what that means if we need to do maintenance on the host | |
| 20:45:13 | sean-k-mooney | fyi just looking at the libvirt level i dont see anything in https://libvirt.org/formatdomain.html that would alow us to mark a vm as not live migratable but we could do int at the nova level | |
| 20:45:39 | efried | hm, there's a side effect of the hack that's necessary to transfer the vtpm data meaning that you could disable migration by simply not setting those conf opts. | |
| 20:45:51 | dansmith | sean-k-mooney: I assumed he was looking for a nova tunable not libvirt | |
| 20:46:11 | sean-k-mooney | ya i just said i would look at both levels | |
| 20:46:21 | dansmith | ack | |
| 20:47:16 | sean-k-mooney | efried: which conf options? | |
| 20:47:37 | efried | https://review.opendev.org/#/c/639934/14/nova/conf/libvirt.py | |
| 20:47:42 | sean-k-mooney | the nova migration uri? | |
| 20:48:24 | sean-k-mooney | that would block you form using swtpm too however right | |
| 20:49:00 | sean-k-mooney | why is swtpm_group a choices field | |
| 20:49:14 | sean-k-mooney | that kindof ties distos hands | |
| 20:49:22 | sean-k-mooney | same for user i guess | |
| 20:50:51 | sean-k-mooney | efried: with may kolla-ansible hat on it also forces me to run the swtpm as a specific user or group which is not ideal | |
| 20:52:31 | efried | sean-k-mooney: IIUC the libvirt process takes care of starting the swtpm. | |
| 20:52:47 | efried | I mean, I didn't have to do anything special to start it. | |
| 20:53:15 | sean-k-mooney | ok but im not sure we should be resticting the options in nova | |
| 20:53:33 | sean-k-mooney | e.g. i think that should just be a string field | |
| 20:54:12 | sean-k-mooney | if you do "ps aux | grep swtpm" what user/group is it running as | |
| 20:55:46 | efried | some processes have a `tss` user, some have `root`. | |
| 20:56:28 | efried | sorry, the root ones are just qemu. They have swtpm CLI opts in 'em. | |
| 22:21:40 | efried | Not sure why I shouldn't determine the uid/gid based on the swtpm process running on the dest host. | |
| 22:22:35 | efried | I guess discovering which process that is could be a bit tricky. | |
| 22:26:13 | efried | oh, ahem, the swtpm processes correspond to instances; they don't exist until the tpm does, chicken/egg. | |
| 22:27:15 | sean-k-mooney | efried: is this in relattion to the config options or something else? | |
| 22:27:22 | sean-k-mooney | *relation | |
| 22:28:05 | efried | yeah, I was trying to figure out a way not to need the config options. | |
| 22:28:12 | efried | because they suck. | |
| 22:28:23 | sean-k-mooney | well why do we need them? | |
| 22:28:40 | sean-k-mooney | nova is not startign the swtpm binary | |
| 22:28:46 | sean-k-mooney | right? | |
| 22:28:51 | sean-k-mooney | that is handeled by libvirt | |
| 22:28:52 | efried | the tpm data file needs to be owned by the uid/gid of the swtpm server process | |
| 22:29:08 | sean-k-mooney | will libvirt create that for us? | |
| 22:29:21 | efried | It creates the process | |
| 22:29:39 | efried | it doesn't help us for it to create the file, cause it ain't the right one. | |
| 22:31:28 | sean-k-mooney | im not really a fan of nova having to manage those file permissions | |
| 22:31:40 | sean-k-mooney | espcially if the file is not own by the nova user | |
| 22:31:58 | efried | I agree. Summed up above as "suck". | |
| 22:32:36 | sean-k-mooney | could we make the files be owned by nova? | |
| 22:32:54 | sean-k-mooney | i have not really looked at how this works at a lowerer level | |
| 22:34:03 | sean-k-mooney | also we really should not have a generic recursive_chown function in nova. | |
| 22:34:14 | sean-k-mooney | *need to have | |
| 22:35:57 | sean-k-mooney | i dont nessisarly know of a better way but from a privsep point of view it sucks to have generic functions like that that take a path user and group | |
| 22:39:41 | sean-k-mooney | efried: is this just needed for the save_and_migrate_vtpm_dir and restore_vtpm_dir functions | |
| 22:39:57 | efried | yes | |
| 22:41:06 | sean-k-mooney | your kind of assuming that nova will have read acess to those folders in places | |
| 22:41:18 | sean-k-mooney | although you might be using privladged functions | |
| 22:42:10 | efried | I haven't really looked at that patch yet (I didn't write it) but I would assume I would need to use privileged operations on both sides. | |
| 22:42:42 | sean-k-mooney | i gues when you do fileutils.ensure_tree(swtpm_dir) to create the folder its in the libvirt instance dir and nova should have write acess to that | |
| 22:43:01 | sean-k-mooney | no all operations would need to be privaladged | |
| 22:45:22 | sean-k-mooney | so that patch is mainly for resize since cold migrate wont work and normal users cant call it | |
| 22:46:01 | efried | On the source, moving the data from its normal location to within the instance dir needs to be privileged to get at the data in its original location | |
| 22:46:01 | efried | Conversely on the dest, creating the real location needs to be privileged; and we should really make sure it's already owned by the right user before moving the contents back out. | |
| 22:46:12 | efried | so I don't see which operations there wouldn't need to be privileged. | |
| 22:47:05 | sean-k-mooney | the ensure_tree function call here https://review.opendev.org/#/c/639934/15/nova/virt/libvirt/utils.py@654 | |
| 22:47:56 | efried | cool, looks like that's the case, no? | |
| 22:48:17 | sean-k-mooney | it is i was just wondering if that need privlages and it was missing or not | |
| 22:49:57 | sean-k-mooney | i would not consider it that unusaly to assume nova can read and write to the virt driver instance folder without needing to elevate privaldges | |
| 22:51:19 | efried | yeah, that's a fair point. | |
| 22:51:32 | sean-k-mooney | its proably fine i was just trying to figure out in what context the config parmanter were used and the surronding context | |
| 22:52:32 | sean-k-mooney | in this case the reason nova has to hanel this is that in the cold migrate case libvirt wont copy these files for us. at least not by default | |
| 22:52:55 | efried | in the live migrate case it won't copy these files either. | |
| 22:53:37 | sean-k-mooney | the commit does not suggest that this is need for live migration | |
| 22:54:05 | efried | what's a "resize between nodes"? | |
| 22:54:18 | sean-k-mooney | your chageing the flavor | |
| 22:54:25 | sean-k-mooney | resize is generally between hosts | |
| 22:54:42 | sean-k-mooney | cold migrate is a special calse fo resize where the flaovr remains the same | |
| 22:54:49 | sean-k-mooney | at least in the libvirt driver | |
| 22:55:54 | dansmith | mriedem: if you're going to flip that to an cast always, I can +W what you have and you can do the error handling cleanup and cast in a change after this but before we turn it on | |
| 22:55:58 | dansmith | mriedem: if you're cool with that | |
| 22:56:37 | dansmith | mriedem: also, I think you'd've needed a reno to explain how migrate _can_ be different sometimes for invisible reasons anyway, so doing one to explain that it's always a little different now is equivalent/better | |
| 22:58:36 | mriedem | ok, i can build that into the series before the policy change at the end. as for docs, it would have been part of the cross-cell docs patch that i haven't fleshed out yet, but yeah it's easier to just say generically 'resize / cold migrate is now a cast all the time' | |
| 22:59:23 | mriedem | sean-k-mooney: the flavor always remains the same for cold migrate | |
| 22:59:25 | dansmith | mriedem: I'd prefer it go early in the series the next time you rebase, just so it merges closer to this conversation for context, but.. ack | |
| 22:59:29 | mriedem | the song....that's a different story | |
| 23:00:31 | mriedem | dansmith: ack, i likely need to rebase soonish anyway so i'll push it toward the bottom when i do | |
| 23:00:38 | dansmith | k | |
| 23:01:08 | sean-k-mooney | mriedem: yes that is what i intened to say | |
| 23:03:32 | sean-k-mooney | i was just trying to indicate that when a commit say "When doing a resize or a cold migration between nodes, this data needs to be copied ..." its not refering to live migration | |
| 23:08:28 | sean-k-mooney | efried: this is the qemu docs on the migration process https://github.com/qemu/qemu/blob/9f2ce35dfa4ea4a31dbb765dd02bed2500891887/docs/specs/tpm.txt#L324-L384 | |
| 23:09:35 | efried | IIUC all of that is handled under the covers | |
| 23:10:22 | sean-k-mooney | yes | |
| 23:10:42 | sean-k-mooney | but looking at it i am not sure they every transfer the tpm state independetly | |
| 23:11:29 | sean-k-mooney | it seams to be contained in the testvm.bin create by the migrate command | |
| 23:14:28 | openstackgerrit | Dustin Cowles proposed openstack/nova master: WIP: Provider Config File: Enable loading and merging of provider configs https://review.opendev.org/693460 | |
| 23:16:06 | sean-k-mooney | this would also seam to mesh with our assumtion of automatic stat transfer for the live migration case https://github.com/stefanberger/libvirt-tpm/commit/72299db63690aa4b74131f54398a665a51e3592f | |
| 23:41:44 | openstackgerrit | Eric Fried proposed openstack/nova-specs master: Spec: Ussuri: Encrypted Emulated Virtual TPM https://review.opendev.org/686804 | |
| 23:42:03 | efried | jroll, dansmith, sean-k-mooney: I think that's ready for review now ^ | |
| 23:42:40 | sean-k-mooney | ack. ill review it tomorrow | |