| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-08-31 | |||
| 11:45:54 | bauzas | whoami-rajat: well, the status should be 'complete' for the libvirt drivers, right? | |
| 11:48:12 | whoami-rajat | bauzas, yeah, there were too many, so wasn't sure, will update for all libvirt drivers | |
| 11:49:13 | bauzas | whoami-rajat: say 'unknown' for the ones you don't know | |
| 11:49:36 | bauzas | like ppc64, s390x and lxc | |
| 11:49:59 | whoami-rajat | to be honest, I'm not sure about different architectures | |
| 11:50:01 | whoami-rajat | ack will do | |
| 11:52:17 | gibi | I guess on those architecture where the libvirt virt driver supports boot from volume there the rebuild will work too. at least I don't remember any arch dependent code in the rebuild patches | |
| 11:53:05 | sean-k-mooney | the arch shoudl not matter | |
| 11:53:16 | sean-k-mooney | the virt types might | |
| 11:53:25 | sean-k-mooney | as i expect this to work for qemu and kvm | |
| 11:55:09 | sean-k-mooney | we could list them as unknon but i stongly suspect it will work on arrch64 at the very least or as gibi suggested anywhere wehre boot form volume is supported | |
| 11:57:19 | sean-k-mooney | whoami-rajat: we support attach and detach volume on all teh kvm and qemu combindations | |
| 11:58:18 | sean-k-mooney | do you have any reason to belive that it would not work or would depend on the architecure | |
| 11:59:14 | gibi | we dont have a row in the matrix about boot from volume support | |
| 11:59:38 | sean-k-mooney | ya i noticed that | |
| 11:59:52 | sean-k-mooney | this could be adressed in a followup patch yes | |
| 11:59:58 | gibi | yes | |
| 12:00:05 | gibi | the whole matrix thing is just doc | |
| 12:00:12 | gibi | that can be done even after FF | |
| 12:00:16 | gibi | before RC1 | |
| 12:00:25 | gibi | or as a doc bugfix later | |
| 12:00:52 | whoami-rajat | sean-k-mooney, i don't think so, if BFV works then this should work too as we're doing some attach detach operations and on cinder side we're doing dd to copy image data to volume | |
| 12:01:06 | whoami-rajat | more or less same as what we do in BFV | |
| 12:01:26 | sean-k-mooney | ack thats what i tought woudl be the case | |
| 12:01:43 | whoami-rajat | so should i just mark all kvm qemu drivers as supported? | |
| 12:01:46 | sean-k-mooney | i think we could mark them complete unless we get a bug report saying it does not work | |
| 12:02:05 | whoami-rajat | ack, will update that | |
| 12:02:05 | sean-k-mooney | thats what i would do but bauzas might perfer unknonw | |
| 12:02:46 | bauzas | I'm not opiniateds | |
| 12:03:12 | bauzas | and I thought, given whoami-rajat was about to update his API change, it was OK to ask for this for the API chbange | |
| 12:03:18 | bauzas | and not by a FUP | |
| 12:03:39 | sean-k-mooney | either is fine | |
| 12:03:52 | gibi | I'm OK with the patch as is if we go with a FUP, or I can reapply my +2 if it is respined | |
| 12:03:53 | sean-k-mooney | i was waitign for zuul to report before revieing after the microversion change | |
| 12:05:19 | opendevreview | Rajat Dhasmana proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | |
| 12:05:34 | whoami-rajat | does this look good? ^ | |
| 12:06:08 | sean-k-mooney | yes i think so | |
| 12:06:51 | bauzas | +2d | |
| 12:07:07 | sean-k-mooney | libvirt-lxc wont work since attach/detach volumes is missing but thats a nit | |
| 12:07:23 | sean-k-mooney | unknonw is fine | |
| 12:09:18 | gibi | we need a +2+A on the first patch then the series will land | |
| 12:09:49 | sean-k-mooney | doing that now | |
| 12:09:52 | sean-k-mooney | was just revieing it | |
| 12:10:08 | sean-k-mooney | whoami-rajat: thanks for fixing the typos and renaming the detach funciton | |
| 12:10:51 | whoami-rajat | sean-k-mooney, np, i had to rebase so was just avoiding followups | |
| 12:11:10 | sean-k-mooney | whoami-rajat: and sorry for the micorversion hassel. stacking the changes was ment to prevent fighting for the same microversion but in this case it did not help | |
| 12:12:08 | opendevreview | Takashi Natsume proposed openstack/nova master: Add missing descriptions in HACKING.rst https://review.opendev.org/c/openstack/nova/+/853054 | |
| 12:12:18 | opendevreview | Takashi Natsume proposed openstack/nova master: Add a hacking rule for the setDaemon method https://review.opendev.org/c/openstack/nova/+/854653 | |
| 12:12:27 | whoami-rajat | sean-k-mooney, i can understand there were some last minute decisions to be made, everything is fine until the changes get in so no problem :) | |
| 12:36:35 | opendevreview | Rajat Dhasmana proposed openstack/nova master: Add API support for rebuilding BFV instances https://review.opendev.org/c/openstack/nova/+/830883 | |
| 12:37:34 | whoami-rajat | ^ there was a functional test failure due to the method name change in compute manager, everything else should is running fine | |
| 12:38:56 | sean-k-mooney | ack ill re reivew after donwstream call | |
| 12:39:07 | sean-k-mooney | if gibi or bauzas dont do it before | |
| 12:39:22 | gibi | I'm on it | |
| 12:40:11 | whoami-rajat | thanks | |
| 13:42:12 | dansmith | gibi: wanna hear something funny? | |
| 13:51:40 | gibi | dansmith: sure | |
| 13:52:09 | dansmith | gibi: I would have bet my lunch on user_data being in the set of things we don't query out by default and only load if required | |
| 13:52:23 | dansmith | I remember extensive discussions about it around icehouse when all this was being done | |
| 13:52:30 | dansmith | because of it's size | |
| 13:52:49 | dansmith | but I think we're actually *always* pulling that out and *always* sending it over the bus right now, which is insane because it's almost never used | |
| 13:53:21 | dansmith | *and* in any of the joined queries where we duplicate the instance fields (like why we stopped joining the metadata queries), we'd be duplicating up to 64k of user data in the DB response... | |
| 13:53:59 | sean-k-mooney | dansmith: we had a cve related to that in the past | |
| 13:54:10 | sean-k-mooney | that should be fixed now | |
| 13:54:16 | dansmith | sean-k-mooney: related to what? | |
| 13:54:33 | gibi | I don't see any special casing for user_data so probably you are right we are always pulling it out | |
| 13:54:35 | sean-k-mooney | pulling GBs of results into a join when you have large userdata | |
| 13:54:52 | dansmith | okay, then fixed how? | |
| 13:55:04 | dansmith | because AFAICT, we're always pulling it | |
| 13:55:07 | sean-k-mooney | https://review.opendev.org/c/openstack/nova/+/758928/ | |
| 13:55:49 | dansmith | sean-k-mooney: that's metadata | |
| 13:56:00 | dansmith | but you're saying that because we were also loading user_data that was much larger I guess? | |
| 13:56:24 | sean-k-mooney | yep so i think we fixed the once case wehre we had a really large join as a result | |
| 13:56:53 | dansmith | yeah so that patch will maybe avoid some duplication of it, but we don't need to be pulling it out of the DB except when we need it | |
| 13:57:14 | sean-k-mooney | yes thats also fair we dont | |
| 13:58:25 | sean-k-mooney | i guess it would be nice to do as a performace enhancement in general | |
| 13:58:34 | gibi | does it also mean that we can save the updated user data on the compute side now? | |
| 13:58:37 | sean-k-mooney | but also possibly reducing the change of another cve | |
| 13:58:38 | dansmith | well, it could seriously cut down on our rabbit load | |
| 13:58:57 | sean-k-mooney | where the user data is large yes | |
| 13:59:04 | dansmith | gibi: yeah I assume that if that patch works, it works because somewhere in reboot the instance gets saved, probably due to state | |
| 13:59:15 | dansmith | gibi: no test coverage of that though that I can see :) | |
| 13:59:17 | sean-k-mooney | unfortunetly people that use heat tend to also abuse the userdata | |
| 13:59:29 | sean-k-mooney | we have had requests in teh past to increae the limmit even more | |
| 13:59:32 | gibi | dansmith: ahh yes as we dont call save on the api side | |
| 13:59:37 | sean-k-mooney | we said no the last few times it came up | |
| 13:59:58 | dansmith | sean-k-mooney: yeah, I bet people are bzipping stuff to keep it under the limit :D | |
| 14:01:27 | sean-k-mooney | oh by the way the limit of medium text is 64MB not 64KB but i think we limit to 64KB but now i want to go check | |
| 14:01:41 | sean-k-mooney | we use mediumtext in the db schema | |
| 14:01:46 | sean-k-mooney | i think we put the limit in the api | |
| 14:01:59 | dansmith | we are limiting in the api yeah | |
| 14:02:06 | dansmith | at least in this patch | |
| 14:02:58 | sean-k-mooney | our api ref say Configuration information or scripts to use upon launch. Must be Base64 encoded. Restricted to 65535 bytes. | |
| 14:03:12 | sean-k-mooney | which is what i expected | |
| 14:03:25 | dansmith | btw, I've seen this fail several times in the last day: https://de836787b7e59a5adc13-298f4365cc798f0001a632f171eb41d6.ssl.cf2.rackcdn.com/831219/22/check/nova-multi-cell/9d8aa66/testr_results.html | |
| 14:04:14 | dansmith | it complains of a missing host, which the test is unshelving to by name, so I assume that's a test bug or something | |
| 14:04:16 | sean-k-mooney | Compute host ubuntu-focal-rax-dfw-0030919238 could not be found. | |
| 14:04:20 | sean-k-mooney | ya | |
| 14:04:28 | sean-k-mooney | so tempest config error maybe | |
| 14:05:15 | dansmith | so anyway, on the user_data thing, I think it would be a good idea for us to en-lazy that by default and only load it in the api and metadata api by default and I bet we'll see some rabbit load relaxed | |