Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2024-07-22
18:47:55 dansmith figure out the migration of existing stuff later
18:48:01 melwitt sean-k-mooney: yeah it's a fingerprint not the image id (name in the cache)
18:48:10 dansmith right
18:48:48 sean-k-mooney ack i honestly have not looked at this closely enough to say of the top of my head. i was hoping we woudl have some of the image metadata stored like the format in the instastance_system_metadata or somethign like that
18:49:12 melwitt oh yeah, we do have all the image properties stored in instance sysmeta
18:49:14 sean-k-mooney that or the bdms. ill need to load contex tbefore ill be of any help
18:49:20 melwitt not sure if disk_format is always part of that
18:49:25 dansmith I think that's too weak, because we won't have that for things like a rescue image
18:49:36 melwitt yeah, true won't have for rescue
18:49:44 dansmith like, if we're going to make this holistic, it has to be more than just the root disk of an instance during boot
18:50:09 dansmith it would be bad if you could pollute the image cache by doing a rescue to a bad image and then a subsequent normal boot, etc
18:50:32 sean-k-mooney dansmith: well we will likely need to modify the image cache api to pass the format but im not really sure form where other then when we are downloadign the image
18:50:42 dansmith and, we won't know which images in the cache were/are for rescue or anything.. I think we have to get to a point where it's something we can rely on, else we will always have the legacy path
18:50:51 dansmith sean-k-mooney: right, that
18:50:55 dansmith is my point though,
18:51:00 dansmith we
18:51:10 dansmith have already decided what the dest filename will be, before we even fetch it
18:51:20 dansmith and changing that reaches *deep* backwards into the driver
18:51:35 dansmith it's sort of ... insane
18:51:50 sean-k-mooney ok then the alternitive is to write a new info file becied the orgianl with the format as the content
18:51:55 melwitt yeah, it is very disjointed
18:52:24 sean-k-mooney as ub _base/backing_file _base/backing_file.info
18:52:31 sean-k-mooney *as in
18:53:23 sean-k-mooney if we cant modify the filename but know what it is we can comptue the relitive name
18:53:42 dansmith the problem is that if we do that, we need to start copying it during migration
18:54:06 sean-k-mooney only if the image is deleted form glance
18:54:08 dansmith I also was hoping to make it a lot more obvious when we missed one
18:54:17 sean-k-mooney otherwise it would be download on the dest node instead of copying
18:54:18 sean-k-mooney but yes
18:54:51 dansmith right, you can see that hole right in front.. upload a bad image, boot, delete it, then migrate to exploit :)
18:54:56 sean-k-mooney am im just finsihing for today but ill sleep on it and take a look at the code tomrrow and see if anythign jump out at me
18:55:07 dansmith ack
18:55:41 melwitt dansmith: the fetch and disk image creation happen in Image.cache() which is called here https://github.com/openstack/nova/blob/master/nova/virt/libvirt/driver.py#L11500 could you maybe return the file format from the fetch func and then pass it into create_image to tack on the extension to the name before writing it?
18:55:58 dansmith melwitt: I think that's only some of the time
18:56:11 sean-k-mooney the only other optiopn that jumps otu at me at the moment is to use the a singel file for all the format info or a db like pythons dbm or sqlite modules
18:56:16 dansmith for ami,ari,aki we fetch direct I think
18:56:19 sean-k-mooney which i assume you dont wnat to do at least right now
18:56:43 dansmith and I thought there were also some of those backend drivers that raise and cause us to take the fetch direct path again, like on boot from snapshot or unshelve or something?
18:56:55 sean-k-mooney i.e. you want this to be local to the compute node and obvious form simple inspection fo the filesytem without special tools
18:57:07 dansmith sean-k-mooney: multiple files that get deleted with their parent seems cleanest if we're going that route
18:57:16 sean-k-mooney and it also needs to work when /var/lib/nova is on nfs
18:57:20 dansmith there is also the concern... yeah NFS
18:57:28 dansmith multiple files make locking less reliable
18:59:07 dansmith melwitt: I was initially going to return format from fetch_as_raw (and add the lookup/return to fetch) yeah
18:59:23 dansmith however we'd need another move there in try_fetch_image_cache() which feels wrong,
18:59:43 dansmith and also, there are cases where we don't go through that like the ami triplets I think
19:00:09 dansmith and of course that copy_from_host, which is going to be a problem either way
19:01:08 melwitt dansmith: move? the base image isn't created until Image.create_image (which is called from Image.cache)
19:02:16 dansmith yeah, if fetch writes it to where it was asked and returns the format, then _try_fetch_image_cache() would need to move it to $path.$format
19:04:06 dansmith unless we put that in image.cache, if that's what you mean?
19:04:12 dansmith same difference though I think
19:05:01 melwitt dansmith: no.. I'm thinking of how fetch isn't called until in Image.create_image so I was thinking you could adjust the target name before writing the file
19:05:10 dansmith either way we end up with a sequence of: 1. fetch image info, 2. forget the format and calculate the wrong filename, 3. fetch the image info again, 4. write to a different file
19:05:26 dansmith right, but ^
19:05:40 dansmith which is doable, it's just pretty stupid..
19:06:32 melwitt I guess I'm not understanding how that would be necessary, like forget the format and all that
19:06:51 dansmith we only have image_id here right?
19:07:21 melwitt if the fetch func returns the format I mean
19:07:34 melwitt *if you made fetch return the format
19:07:49 dansmith ...which will be done by re-fetching the image info yeah? :)
19:07:57 melwitt no, the first fetch
19:08:08 dansmith wut
19:09:51 melwitt like in Image.create_image, the prepare_template() function is the fetch function. I guess you would have to be able to adjust what's in the functools.partial though to do it, maybe that's something that is not possible?
19:11:08 dansmith I'm a bit lost here, but while we were talking I see that the kernel and ramdisk go through raw.cache() as well which means the cache function itself might work for those too as you say
19:11:51 dansmith but note that ephemeral and swap are created like that as well, so they'd all need to change so we get a stable format returned from their not-really-fetch-function-s
19:12:48 melwitt that is true, all of the fetch funcs would need to be modified to return format
19:13:33 dansmith also after this,
19:13:55 dansmith wait
19:16:39 dansmith I'm just trying to figure out if we need the filename anywhere else after we've made that call
19:17:05 dansmith seems like we imply it below that but maybe we never use it
19:17:36 dansmith just feels wrong to have one step change the filename we're going to use in the middle, because someone will come along and say "oh look I have filename here, I can do a thing"
19:18:56 dansmith er, yeah, because _create_image() is called and then we go create the guest's xml which will assume the image(s) are named like instructed right?
19:19:44 melwitt yeah.. error prone at best I guess. and maybe an actual problem when a request comes in for a cached image, I guess maybe you could still match it by fingerprint + glance reported disk_format and if found in cache, use what's in the cache
19:20:37 dansmith we can't though,
19:20:41 melwitt maybe.. have to think about it. you were only talking about adding extension to base images right? or are you meaning like all images
19:20:55 dansmith because we need to know the image's disk_format, unless you want a THIRD ping to glance :)
19:21:19 dansmith I think all images would be ideal, but I think the base images are the important case here
19:21:24 melwitt oh, right. hm
19:22:30 melwitt well, could you not assume the format of what you find in the cache? like you get a request, you calculate the fingerprint, look for the basename in the cache, if you find it you use the file with the extension on it
19:23:05 melwitt as the format. without querying glance for it
19:23:49 dansmith I mean, we can also just run our detection code on the images without the format every time.. the point is to try to harden this, not just layer on the guesses :D
19:24:23 melwitt no, the detection ran the first time the base image was written. thats' when you added the extension
19:24:33 melwitt but from then on, you don't need to look it up again
19:24:33 dansmith and part of the strategy here, I think, is to make sure we don't lose track of what we've validated and so yeah, doing /images/$image.*[0] is better than we have, it still seems like a hack
19:24:49 melwitt ok
19:25:04 dansmith I understand what you're saying, I'd just rather it be better than that
19:25:13 melwitt ack
19:26:05 dansmith I really was hoping to avoid just cluttering the _base dir with .info files, but perhaps that's just going to be easier because of how obsessed we are with filenames throughout the layers
19:26:18 dansmith I dunno, I'll have to think about it
19:27:10 dansmith also makes an admin's job harder if the next time we have a thing, they have to go read all the info files looking for which one(s) are qcows, then using that list to do some remediation action
19:27:37 dansmith like currently, we were unable to check and purge bad images from the cache if they're present and the operators need to do that
19:27:50 dansmith that's just a UX thing and not critical of course, but...it'd be nicer
19:28:25 dansmith I assume if we use .info files, we should dump json in there because as soon as we add this we'll need to store something else (honestly we should probably store image_id at least as well)(
19:33:45 melwitt yeah I agree .info files would be json. you could even have a nested list for all the files in one .info!
#openstack-nova - 2024-07-23
07:29:40 opendevreview Alexander Hild proposed openstack/nova master: feat: Include "vm uuid" in baseBoard asset tag in libvirt domain xml https://review.opendev.org/c/openstack/nova/+/924697
07:31:56 opendevreview Alexander Hild proposed openstack/nova master: feat: Include "vm uuid" in baseBoard asset tag in libvirt domain xml https://review.opendev.org/c/openstack/nova/+/924697
08:06:18 mikal sean-k-mooney: are you around? I think https://review.opendev.org/c/openstack/nova/+/922544 is ready for review but I have questions about if there needs to be a tempest test for it.
08:39:36 sean-k-mooney mikal: im not sure how you would tempet test that spice is secure. we recently tired to do that for vnc and vcrypt and we could not come up with a reasonable way ot do that just with a tcp connection becuase vnc uses tls to encypt the vnc packate payload not the entire tcp packet payload
08:40:53 sean-k-mooney so you can just connect without implementing the nvc protocal at least enought to negoicate the vencrypt session
08:41:43 sean-k-mooney unless there is a simple way to asssert that spice is using a secure connection without implemeting a portion of the spice protocoal i think unit and perhaps functional tests are enough

Earlier   Later