| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2024-03-07 | |||
| 18:05:55 | dansmith | I then booted a new instance from that snap, saw that it ran qemu-img convert to (I think) unencrypt it before it uploaded to ceph to then create a vm image from, | |
| 18:06:06 | dansmith | but the disk is unreadable in the new instance.. like fails in BIOS | |
| 18:06:38 | dansmith | since this is ceph it's hard to examine the actual disk to see what the state is.. I should try to repro with qcow because I can poke at it easier | |
| 18:07:03 | dansmith | but (a) that's expected to work, right? | |
| 18:07:46 | dansmith | and (b) I'm thinking if a user does this, they don't realize that you just stripped the encryption from their snapshot image to upload it | |
| 18:08:16 | dansmith | or maybe I'm wrong about what was happening - perhaps we were just re-encrypting it with the new key? if had used qcow it'd be easier for me to see what happened | |
| 18:08:23 | dansmith | maybe it's in the logs actually | |
| 18:08:53 | melwitt | dansmith: hm, well it shouldn't be unencrypting it, convert is used to make a copy with a new key. but as we discussed I have to change all that and haven't yet | |
| 18:09:40 | dansmith | okay, I thought that maybe the "no support yet for encrypted backing files" meant we were de-keying it because it's the image we're based on | |
| 18:10:07 | melwitt | and yeah, it was expected to work, to be able to boot a new instance from the image. there might be something that got messed up when I refactored the things needed from review feedback on the bottom patches | |
| 18:10:37 | melwitt | well, the rbd patch is after the encrypted backing file support one | |
| 18:11:13 | dansmith | okay so, there's a command in the log which is what I was seeing when I watched it, where we convert _to_ luks _from_ raw | |
| 18:11:20 | dansmith | so I assumed that meant we had raw'd it | |
| 18:12:20 | melwitt | and that's the root disk, not swap or ephemeral? | |
| 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 | |