| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-03-07 | |||
| 18:14:02 | dansmith | we wouldn't run convert on ephemeral or swap because we're creating from scratch would we/ | |
| 18:14:51 | melwitt | oh, right. in earlier revisions I had it creating as raw blank and then mkfs and then encrypted it. but yeah I think I changed it to create luks off the bat | |
| 18:16:16 | melwitt | so it sounds like there must be a bug there that happened in the intervening time. I need to look into it | |
| 18:17:59 | melwitt | converting from raw to luks should only happen if the source image is unencrypted. and it would be interesting to see if it's erroneously getting converted to raw after pulling it down from ceph. I wonder if maybe i messed something up in the fetch_to_raw function. it's supposed to be a no-op if the source image is luks | |
| 18:18:30 | melwitt | that was something that got changed during addressing review feedback | |
| 18:21:24 | melwitt | I'll investigate | |
| 18:22:21 | dansmith | there's a raw disk in _base, but I think that might be the original cirros I created from | |
| 18:22:40 | dansmith | but yeah, I probably need to take ceph out of the equation here so I can grok what's going on | |
| 18:22:55 | dansmith | I'm trying to look at the log and I don't see anything with "-O raw" which is good | |
| 18:24:31 | dansmith | yeah I'm not sure maybe I misread a command line as a convert when it was something else | |
| 18:26:18 | melwitt | it would be 'qemu-img convert' in that case | |
| 18:27:22 | dansmith | yeah | |
| 18:27:40 | dansmith | we are doing a lot of qemu-img info execs with no input format, which is a problem, | |
| 18:27:49 | dansmith | not sure if that's old or new code though | |
| 18:28:30 | dansmith | okay let me redo this with no ceph so I can try to figure out they boot-of-a-snap is dead | |
| 18:29:16 | dansmith | or see if it's a ceph thing I guess | |
| 18:29:18 | melwitt | should be old, for qemu-img info | |
| 18:30:12 | dansmith | ack, I thought we had fixed those | |
| 18:31:06 | melwitt | I'm double checking. maybe I'm forgetting something | |
| 18:33:10 | melwitt | info itself didn't change but passing the format is optional, it's possible I added a call and didn't pass the format. I don't remember off the top of my head but just saying there's a possibility. I'll go back and fix that if I find it | |
| 18:36:25 | dansmith | oh I know it's optional, it's just a security bug to not provide the format | |
| 18:36:51 | dansmith | I guess maybe we had some info cases where we need to probe in the dark, as long as we're careful not to follow the links | |
| 18:36:57 | dansmith | convert is the one you absolutely have to always have | |
| 18:37:38 | dansmith | anyway, no need to rathole on that specifically, just noticed it | |
| 18:38:07 | melwitt | ack, thanks | |
| 20:10:04 | stblatzheim | dansmith: if you're around would be happy for review of https://review.opendev.org/c/openstack/nova/+/911070 :) | |
| 21:20:49 | artom | I think I need a bit of philosophical guidance from anyone still around... | |
| 21:21:09 | artom | What's the general opinion for changing Nova code _just_ to enable additional testing in CI? | |
| 21:21:29 | artom | Specifically https://review.opendev.org/c/openstack/nova/+/910022/ | |
| 21:22:47 | artom | CPU power management in libvirt is done via module-level functions, so I don't see a way to mock/patch/stub stuff to do live migration testing in functional tests, as both of the libvirt drivers on the source and destination host call into the same module, nova.virt.libvirt.cpu.api | |
| 21:23:22 | artom | I would essentially have to turn all that machinery into classes, and give each virt driver an instance of the class | |
| 21:23:32 | artom | Which would allow proper per-host mocking | |
| 21:23:49 | artom | And I feel like that's a pretty radical change | |
| 21:38:02 | dansmith | philosophically, I think we should design code to work well and to be testable (as the latter feeds into the former) | |
| 21:38:24 | dansmith | whether or not it's worth refactoring what you're looking at specifically is maybe a judgment call, but I wouldn't rule it out on principle or anything | |
| 21:39:39 | artom | So - thinking out loud here then. | |
| 21:39:49 | artom | 1. The scope is in-tree functional tests. | |
| 21:40:34 | artom | 2. The existing code is _not_ testable for live migration. Not without some pretty convoluted, probably fragile, and hard to understand hax that may not even work. | |
| 21:42:34 | artom | 3. Sounds like I have a case for refactoring. At least, it's not to be dismissed off-hand. | |
| 21:43:03 | artom | If I do it and it ends up getting shot down in review, at least that outcome wasn't immediately obvious when I started (now). | |
| 21:43:48 | dansmith | yeah I mean it's hard to say without a little more detail, but I'm too fried at this point in the day to dig into something else | |
| 21:44:03 | dansmith | but the words you're saying sound fine on their face | |
| 21:44:34 | artom | Yeah, that's the best I can hope for, at this point. Hard to argue about hypothetical without concrete code to look at. | |
| 21:44:58 | artom | At least I know I'm not way cray-cray with my reflex to refactor. | |
| 21:46:03 | dansmith | not *because* of this anyway :) | |
| #openstack-nova - 2024-03-08 | |||
| 05:44:06 | opendevreview | Jeffrey Zhang proposed openstack/nova master: Do not change nic queue count during live migration after vcpu hot resized https://review.opendev.org/c/openstack/nova/+/912082 | |
| 09:11:01 | opendevreview | melanie witt proposed openstack/nova master: Support create with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/870932 | |
| 09:11:02 | opendevreview | melanie witt proposed openstack/nova master: Support live migration with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/905512 | |
| 09:11:02 | opendevreview | melanie witt proposed openstack/nova master: Support (resize|cold migration) with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/870933 | |
| 09:11:03 | opendevreview | melanie witt proposed openstack/nova master: Support rescue with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/873675 | |
| 09:11:03 | opendevreview | melanie witt proposed openstack/nova master: Support rebuild with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/870939 | |
| 09:11:04 | opendevreview | melanie witt proposed openstack/nova master: Support snapshot with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/870937 | |
| 09:11:04 | opendevreview | melanie witt proposed openstack/nova master: Add encryption support to qemu-img rebase https://review.opendev.org/c/openstack/nova/+/870936 | |
| 09:11:05 | opendevreview | melanie witt proposed openstack/nova master: Support encrypted backing files for qcow2 https://review.opendev.org/c/openstack/nova/+/907961 | |
| 09:11:05 | opendevreview | melanie witt proposed openstack/nova master: Add backing_encryption_secret_uuid to BlockDeviceMapping https://review.opendev.org/c/openstack/nova/+/907960 | |
| 09:11:07 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Introduce support for raw with LUKS https://review.opendev.org/c/openstack/nova/+/884313 | |
| 09:11:07 | opendevreview | melanie witt proposed openstack/nova master: Support cross cell resize with ephemeral encryption for qcow2 https://review.opendev.org/c/openstack/nova/+/909595 | |
| 09:11:09 | opendevreview | melanie witt proposed openstack/nova master: Report ephemeral disk encryption in the metadata API https://review.opendev.org/c/openstack/nova/+/909945 | |
| 09:11:09 | opendevreview | melanie witt proposed openstack/nova master: libvirt: Introduce support for rbd with LUKS https://review.opendev.org/c/openstack/nova/+/889912 | |
| 09:11:11 | opendevreview | melanie witt proposed openstack/nova master: Documentation for ephemeral encryption https://review.opendev.org/c/openstack/nova/+/910034 | |
| 09:11:11 | opendevreview | melanie witt proposed openstack/nova master: Deprecate legacy ephemeral storage encryption using dm-crypt https://review.opendev.org/c/openstack/nova/+/909947 | |
| 09:11:13 | opendevreview | melanie witt proposed openstack/nova master: Consolidate vTPM and ephemeral encryption secret creation https://review.opendev.org/c/openstack/nova/+/912094 | |
| 09:13:24 | opendevreview | melanie witt proposed openstack/nova master: Documentation for ephemeral encryption https://review.opendev.org/c/openstack/nova/+/910034 | |
| 09:16:49 | melwitt | dansmith: ^ rebased, added some of the things we talked about, fixed the problem with rbd (I had missed updating a variable name that got changed). didn't add the decorator for checking key access yet | |
| 09:20:13 | opendevreview | melanie witt proposed openstack/nova master: DNM test ephemeral encryption + resize: qcow2, raw, rbd https://review.opendev.org/c/openstack/nova/+/862416 | |
| 10:33:50 | opendevreview | Merged openstack/python-novaclient master: Bump microversion to 2.96 https://review.opendev.org/c/openstack/python-novaclient/+/911575 | |
| 14:30:27 | dansmith | melwitt: fixed which problem, the reason I couldn't boot from the snap? | |
| 14:31:45 | fungi | bauzas: do you happen to know if nova had any development highlights to call out for 2024.1? today was supposed to be the deadline for collecting those, but we can usually work additions in if they're in the next week-ish | |
| 14:32:09 | bauzas | fungi: shit, I forgot | |
| 14:32:21 | bauzas | fungi: I'll work on it | |
| 14:32:27 | fungi | no worries, there's a ton of stuff going on (always) | |
| 15:42:48 | dansmith | melwitt: okay must be because using the same code you had before seems to work on qcow (i.e. boot from encrypted snap | |
| 16:00:43 | elodilles | bauzas: if you are still around, fyi: https://review.opendev.org/c/openstack/releases/+/912275 | |
| 16:01:26 | elodilles | bauzas: and this, respectively: https://review.opendev.org/c/openstack/releases/+/910940 | |
| 16:01:43 | bauzas | elodilles: thanks I was about to ask you to create a new release for the client | |
| 16:02:16 | bauzas | elodilles: +1d | |
| 16:04:49 | elodilles | thx too o/ | |
| 16:10:48 | dansmith | melwitt: so if I create an encrypted instance, snapshot it to an encrypted image, then boot an instance with an unencrypted flavor from that encrypted image, what do you expect to happen? | |
| 17:04:03 | melwitt | dansmith: yes, sorry, by "rbd problem" I meant the weird qemu-img convert and the inability to boot from the snapshot. for rbd only | |
| 17:06:04 | melwitt | dansmith: in that scenario I would expect the instance you booted to have encrypted disks, _unless_ the flavor is explicitly disabling encryption i.e. hw:ephemeral_encryption=false. if it's explicitly disabled in the flavor, the API should reject the request because of the conflict between flavor and image | |
| 17:30:29 | dansmith | melwitt: ah, okay lemme try that I guess | |
| 17:30:51 | dansmith | melwitt: so indeed, by default you get another encrypted stack which is maybe what you expect and maybe not | |
| 17:31:26 | dansmith | however, if your goal is to get an unencrypted image, you'd currently have no way out of that situation if there was no negative flavor for you to use | |
| 17:31:59 | melwitt | dansmith: the image has image properties requesting encryption so you should expect it | |
| 17:32:00 | dansmith | if we add the snapshot=(rekey,samekey,nokey) option then you could, but you'd have to jump through another snapshot hook to sort of free yourself from the overhead of the encryption | |
| 17:32:18 | dansmith | melwitt: I expect it for the image, but not the instance disk | |
| 17:32:49 | dansmith | like, I expected to have an encrypted qcow backing file, but unencrypted (i.e. full speed) disk when I write | |
| 17:33:07 | melwitt | dansmith: encryption is requested through image properties and flavors. the image has hw_ephemeral_encryption=true so it is expected to encrypt the disks | |
| 17:33:09 | dansmith | I know why I don't but that was what I was trying to do to see if it worked (before I realized obviously it wouldn't) | |
| 17:33:38 | dansmith | yeah, I understand why it's doing it, I'm just saying with my user hat on it was not what I was expecting | |
| 17:34:22 | melwitt | meaning you didn't expect the encrypted snapshot Nova made to have hw_ephemeral_encryption=true set? or you expected the flavor to override the image property? | |
| 17:35:35 | dansmith | no, the image has to have that set because it's encrypted, I was thinking the default was false in the flavor, but it's not, it's "default is do what the image says" ... which again I understand, | |
| 17:36:04 | dansmith | but if I'm a user and I'm handed an encrypted image and I want to boot from it but don't want to take the write hit for an encrypted disk, I'm stuck unable to get out of that loop | |
| 17:36:08 | dansmith | with just user tools | |
| 17:36:15 | dansmith | or without downloading, unkeying and re-uploading the image | |
| 17:36:52 | melwitt | no, the default is check the flavor and image and if either one says encryption=true, the disks get encrypted | |
| 17:37:38 | melwitt | if one says encryption=true and the other says encryption=false, that is invalid and the api should reject it | |
| 17:38:16 | dansmith | sure, but as a user, the flavor is the tool I'm given to control what the target environment looks like.. it's definitely confusing and grey with some stuff in both places for sure | |
| 17:38:44 | dansmith | I'm just saying, I expected to be able to break out of the loop without having to download and unkey the image myself | |
| 17:38:51 | melwitt | I am expecting to add the microversion for the snapshot api stuff btw, I put it in the spec reproposal. so there will be that | |