Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2025-01-17
14:37:32 sean-k-mooney i agree this is a bug of some kind
14:37:50 sean-k-mooney its not clear to me what the best path forward is in ths short term
14:38:08 sean-k-mooney long terme there shoudl be no backing file and or no data in the backing file
14:38:24 dougszu thanks Sean, that's all helpful. I think as you say, the easiest fix is to just not support created the FS on the backing file
14:38:32 dougszu *creating
14:39:18 dougszu I will have a think about a fix for existing VMs. I think you can rebase the top layer onto a new backing file for one.
14:39:20 sean-k-mooney we wanted to make that change to reduce the security footprint of nova
14:39:56 sean-k-mooney dougszu: we might be abel to flatten the image on migration, im not sure
14:40:03 dougszu It also avoids issues with for example Nova making a v5 XFS FS and then booting a C7.9 image with XFS 4 support etc
14:40:50 dougszu yeah, flatten is also interesting
14:44:52 sean-k-mooney so i think this bug is another reaons to implement https://blueprints.launchpad.net/nova/+spec/default-ephemeral-format-unformated sooner rather then later
14:45:07 sean-k-mooney i cant commit to doing that this cycle but i might see if i can find time to try
14:49:16 dougszu ^ that looks good. I'd be happy to test/review/help out.
14:50:30 sean-k-mooney gibi: i think this is the relevent fucntion https://github.com/openstack/devstack/blob/38f8f4da4556c11bf526392359ed6c14b45d87ea/inc/meta-config#L79
14:52:22 gibi sean-k-mooney: thanks!
14:53:09 sean-k-mooney the comment says it shoudl create the file if it does not exist which is the behviaor i recall i think its just failing because /etc/placement is not there for it to create the file under hence the mkdir -p comment
14:55:37 gibi OK I will dig
14:58:54 opendevreview Dan Smith proposed openstack/nova master: Handle GPT image detection https://review.opendev.org/c/openstack/nova/+/933928
14:58:55 opendevreview Dan Smith proposed openstack/nova master: DNM: Test with latest oslo.utils https://review.opendev.org/c/openstack/nova/+/933444
15:01:27 sean-k-mooney gibi: found it https://review.opendev.org/c/openstack/devstack/+/922846
15:01:53 sean-k-mooney gibi: i know i debugged that at some point before
15:02:00 sean-k-mooney *knew
15:06:43 gibi sean-k-mooney: awesome, I will put a depends-on on it to see if this helps.
15:07:10 gibi sean-k-mooney: also an easy win review about whitebox in periodic queue https://review.opendev.org/c/openstack/nova/+/833453
15:08:15 sean-k-mooney oh cool artom updated that
15:08:39 sean-k-mooney done
15:08:55 sean-k-mooney we can review it for stablity i guess and see how it goes
15:10:51 gibi yepp, bauzas tend to check the periodic state during our weekly meeting so we will see
15:11:06 sean-k-mooney speakign of easy wins im also approving https://review.opendev.org/c/openstack/nova/+/939453/1 to disable the heal perodic in nova-next
15:11:23 sean-k-mooney when you have time https://review.opendev.org/c/openstack/nova/+/939476 is the follow up about changing the default
15:11:49 sean-k-mooney i was going to add that to next weeks team meeting ot get import
15:25:53 gibi ack
15:57:24 dansmith ugh.. this is wrong: https://docs.openstack.org/nova/latest/configuration/config.html#libvirt.snapshot_image_format
15:58:01 dansmith the default is not "the same type as source image" it's "same type as the ephemeral backend is using"
15:59:27 opendevreview Balazs Gibizer proposed openstack/nova master: [CI][nova-next]test with placement ac breadth-first https://review.opendev.org/c/openstack/nova/+/937275
16:31:21 sean-k-mooney dansmith: i have not at the code but perhaps "souce iamge" in this context was inteded to refer to "the format fo the file on disk that was snapshoted" rather then glance
16:31:33 sean-k-mooney which would match what you are stating
16:31:49 dansmith well, that's what the code is doing, but I don't think that's how someone would read that doc text
16:33:05 dansmith glance people also had assumed that nova snapshotted in the format of the original (source glance) image
16:33:14 sean-k-mooney ya without reading the docs i woudl have assuemd nova woudl use the same format in glance as the image was on disk but iagree that not what i would take away form that text
16:34:07 sean-k-mooney well novas current behaivor is not really surpsing to me nessisarly
16:34:30 sean-k-mooney but i agree the docs are incorrect
16:35:02 dansmith it's not really surprising to me either knowing the code and how things work under the covers, I just think that's not what the reader of the doc would think
16:35:24 sean-k-mooney this is also kind of glossing over images_type=rbd
16:35:29 dansmith and so when it was asserted that the format would be the same as the source (glance image) and I read that doc, I assumed that's what we were doing
16:35:35 dansmith yeah, also that
16:36:13 sean-k-mooney do you know what happens if you set this and you use images_type=rbd ?
16:36:37 sean-k-mooney it woudl be reasonabel to assuem it either has no effect (proably what happens today)
16:36:41 dansmith I assume you mean also where we share a pool
16:36:51 dansmith if sharing, we don't even run this code
16:36:55 dansmith if not, we'll do the same thing, It hink
16:36:55 sean-k-mooney but it woudl also be reasonable to assume we woudl actully upload an image in the requested format
16:37:19 dansmith yeah, good point
16:37:37 dansmith someone thinks they can set this to get qcow2 images even for things on ceph...
16:38:23 sean-k-mooney im no really arguing either way just that the config option seams somewhat probelmatic on other fronts
16:38:43 sean-k-mooney i kind of get why it exists
16:39:05 sean-k-mooney i.e. use force raw for the hypervior but store all snapshots in qcow to save space
16:39:06 dansmith yeah, problematic indeed, since it will cause us to unwrap a qcow2 into a raw with qemu
16:39:47 dansmith I'm not sure that would really happen unless qemu does empty space detection when you convert, and even if it did, it wouldn't work unless the guest OS had *actually* zeroed sectors
16:40:18 sean-k-mooney rigtht im thiniking of qcows sparce file supprot
16:40:33 sean-k-mooney but i also dont know if we enable that when we create the snapshot
16:41:08 sean-k-mooney but other then that usecase i dont really see a valid reason to ever set this
16:41:58 dansmith apparently it does zero detect and collapse (just tried) but still.. only helps if the guest zeroes the unused sectors or never touches them
16:42:35 dansmith anyway I'm on this because of our need to upload to glance with an accurate (i.e. raw vs gpt) format
16:43:00 sean-k-mooney im not sure if operators would scream but im inclind to say remove this current config option
16:43:18 sean-k-mooney im unsure if we shoudl have an option to say "match glance vs match file on disk"
16:43:51 dansmith well, that's what default is now
16:43:54 sean-k-mooney the kinder thing to do woudl be just set the glance type to what we are updating and continue to supprot this option
16:44:01 dansmith oh, you mean two options
16:44:02 dansmith I see
16:44:21 sean-k-mooney well this is config driven api behviaor right
16:44:46 dansmith I'm not really sure "match glance" is necessary, I just think it's more visible from the user's perspective and also helps us say "nope, you booted from gpt, so you stay gpt"
16:44:48 dansmith yes, it is
16:44:49 sean-k-mooney as an end user i woudl kind of expect the snapshot to be in the same format as the orgianl image
16:44:56 dansmith config-driven api behavior across two services
16:45:06 sean-k-mooney as an opterator that does nto want the iamge convertion overhead on every boot
16:45:13 sean-k-mooney i proably want it to match the file on disk
16:45:24 dansmith right, but, you upload a vmdk, it gets converted to raw, and snapshotted as a qcow2
16:46:01 sean-k-mooney to some degree as long as the data and the format listed in galnce aling that actully ok
16:46:01 dansmith hard to say what behavior will please everyone, but it's also frustrating to have so much variability here
16:46:09 dansmith it's just more reason the ephemeral stuff is a mess
16:46:37 sean-k-mooney i mean one way around it would be to add the format to the snapshot api
16:47:06 sean-k-mooney but im not conviced that good to expose
16:47:21 sean-k-mooney for one i dont know if it woudl work for any other virt dirver
16:47:49 sean-k-mooney but ignoring that operator might now want ot allow that and it will also comlicate this code even more so...
16:48:12 dansmith nah, that would allow users to tickle qemu-img bugs
16:49:36 sean-k-mooney yep whihch seams bad. its better to let them convert it themselve after they download
16:49:45 sean-k-mooney well safeer perhaps not better
16:49:49 dansmith yeah
16:50:14 sean-k-mooney dansmith: so are you just going to update the help text and make sure the format in glance is set correctly for now?
16:50:23 dansmith so basically, I think we need to inspect (with format inspector) the snapshot image if the target format is raw, and then upload to glance as raw or gpt depending
16:50:46 sean-k-mooney well snapshot is only for the root disk
16:50:56 dansmith it is set in glance correctly, AFAICT, but yeah, when I add that behavior I can update the text of the config option
16:51:04 sean-k-mooney so it will be gpt or one of the other formats but not raw right
16:51:41 dansmith well, in almost all cases yes, but presumably if you're running something really strange in your guest, or doing direct kernel boot with just a whole-disk filesystem...
16:51:57 dansmith as I said before, I think we should deprecate that behavior (at least require a full disk image even for direct kernel boot)
16:52:07 dansmith but presumably OS/2 in a guest could be a thing
16:52:26 dansmith even solaris on sparc has been GPT since 2010, and the arm server spec says GPT
16:52:39 sean-k-mooney hum im sure there are some bbs vms on opnestack somewhere
16:52:46 dansmith so the things you might be running in there that could legit be not on GPT is dwindling

Earlier   Later