| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Deprecate linux bridge plugin https://review.opendev.org/c/openstack/os-vif/+/941585 | 03:26 |
|---|---|---|
| opendevreview | Takashi Kajinami proposed openstack/os-vif master: Deprecate linux bridge plugin https://review.opendev.org/c/openstack/os-vif/+/941585 | 03:58 |
| gibi | dansmith, sean-k-mooney (maybe bauzas) can we get this over the line https://review.opendev.org/q/topic:%22bug/2097359%22 It is very closem | 11:23 |
| sean-k-mooney | so the only reason i didnt +w the bottom patch is i would like all 3 to merge togehter ot for the latter two to be squashed | 11:24 |
| sean-k-mooney | i am +2 on all 3 | 11:24 |
| gibi | I see, thanks. Then we need dansmith | 11:25 |
| sean-k-mooney | the second patch does not account for instnaces partly migrated by my intail fix patch which is why the thrid patch is imporant IMO | 11:26 |
| gibi | sean-k-mooney: yes 3rd is important, dansmith asked me to do it separately as it has bit more invasive changes hence the 3 separate patch | 11:31 |
| gibi | I will keep them separate but I'm OK to land them as a whole | 11:31 |
| sean-k-mooney | ack | 11:31 |
| opendevreview | Vasyl Saienko proposed openstack/nova master: Do not review https://review.opendev.org/c/openstack/nova/+/941823 | 12:38 |
| sean-k-mooney | stephenfin: gibi https://review.opendev.org/c/openstack/nova/+/939476 is the change to disable the heal info cache perodic | 12:42 |
| sean-k-mooney | stephenfin: bauzas: https://review.opendev.org/c/openstack/nova/+/938523 is the review for the schduler discover host perodic change | 12:42 |
| sean-k-mooney | can we try and get those approved today? | 12:42 |
| stephenfin | sure | 12:47 |
| bauzas | sean-k-mooney: sent to the gate, thanks | 13:21 |
| bauzas | btw. can some other core like gibi look at disabling heal_instance_info cache https://review.opendev.org/c/openstack/nova/+/939476 ? | 13:21 |
| dansmith | gibi: sorry didn't realize I hadn't circled back to that. looks like the bottom two are in the gate right? I'll get the third one in a bit after coffee | 14:28 |
| gibi | dansmith: thanks | 14:36 |
| gibi | dansmith: the first patch also needs a +w, but it is OK to get it after you reviewed the 3rd patch | 14:36 |
| tkajinam | sean-k-mooney, hi could you re-review https://review.opendev.org/c/openstack/nova/+/931582 when you have time ? | 14:37 |
| tkajinam | I had to add the ugly mock to fix failures caused by openstacksdk (which internally calls the same psutil interface) | 14:37 |
| sean-k-mooney | i tought we merged that already so sure | 14:38 |
| tkajinam | thanks ! | 14:39 |
| tkajinam | and also thanks for the discussions in os-vif patch | 14:39 |
| sean-k-mooney | i still need to create the deprecation patch for nova that disable the linux bridge job | 14:39 |
| sean-k-mooney | i assume you dont have one for that | 14:39 |
| sean-k-mooney | i added the topic to the irc meeting for tomorrow | 14:40 |
| tkajinam | you are correct | 14:40 |
| sean-k-mooney | ack ill push one later today | 14:40 |
| sean-k-mooney | im hoping we can get them merged tomorrow or wednesday and then do a release of os-vif | 14:40 |
| tkajinam | that sounds nice ! | 14:40 |
| opendevreview | Vasyl Saienko proposed openstack/nova master: Do not review https://review.opendev.org/c/openstack/nova/+/941823 | 16:46 |
| bauzas | dansmith: you had good concerns on https://review.opendev.org/c/openstack/nova/+/941601 | 17:20 |
| bauzas | can I try to clarify what I wrote ? | 17:21 |
| bauzas | because I stupidely copy/pasted a lot of docstrings from the metrics conf option which led to confusion | 17:21 |
| bauzas | basically, with https://review.opendev.org/c/openstack/nova/+/941601/1/nova/scheduler/weights/image_props.py the weight is a sum of subweights dependent on the per-property weigh | 17:22 |
| bauzas | that weight would then be multiplied by the main conf option | 17:23 |
| bauzas | so, yes, properties would be weighed between themselves, but then the main image_props_weight_multiplier would balance the weighed value against other weighers | 17:24 |
| bauzas | https://review.opendev.org/c/openstack/nova/+/941601/1/nova/tests/unit/scheduler/weights/test_weights_image_props.py shows you that | 17:25 |
| bauzas | I just configured the weight multiplier to 1.0 to keep it understandable, but say if I was using 2.0 for the multiplier, then each of the host weights would be doubled | 17:26 |
| dansmith | bauzas: replied to the comments on the firs tone | 17:26 |
| bauzas | okay, I think I then need to upload the first patch | 17:28 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: Add a new ImagePropertiesWeigher https://review.opendev.org/c/openstack/nova/+/940642 | 18:15 |
| opendevreview | Sylvain Bauza proposed openstack/nova master: per-property ImageMetaPropsWeigher https://review.opendev.org/c/openstack/nova/+/941601 | 18:15 |
| dansmith | bauzas: were you going to snip out the reno from the first patch or no? didn't see a reply about that, but otherwise ^ looks good | 18:16 |
| bauzas | dansmith: well, I just amended the original reno file with the latter patch | 18:17 |
| bauzas | so depending on what we merge, we'll either have one note or an amended note | 18:17 |
| dansmith | alright, I guess I thought that was less good but fair enough | 18:17 |
| bauzas | that said, if the latter goes merged after FF, then we could potentially need to create another reno | 18:17 |
| sean-k-mooney | gibi: Uggla i replied in https://review.opendev.org/c/openstack/nova/+/937649 what gibi found is a bug. i also explaind why false and "false" have different behavior | 18:18 |
| sean-k-mooney | tl;dr it because we do a json load on the dict because its really a multioptStr with json content | 18:19 |
| sean-k-mooney | https://bugs.launchpad.net/nova/+bug/1915282 | 18:19 |
| sean-k-mooney | bauzas:... i didn hit reply on my comment this morning | 18:20 |
| sean-k-mooney | but ill check if they are stilll relevent or not | 18:20 |
| sean-k-mooney | sort of but not worth a respin unless your fixing somethign else | 18:21 |
| sean-k-mooney | bauzas: if its after FF then it would be a seperate releae note anyway because the behavior would differ across release | 18:22 |
| sean-k-mooney | but amending it is fine IMO | 18:23 |
| sean-k-mooney | as long as its i nthe same release i think that is preferable | 18:23 |
| bauzas | I need to hardstop now, I'm basically fried | 18:23 |
| bauzas | but I'll look at your comments tomorrow for sure | 18:23 |
| bauzas | and yeah for the reno, it was just a tradeoff : if we merge both, that's simplier than an additional note | 18:24 |
| bauzas | if we can only merge the bottom patch, then I'll respin for not breaking our release notes | 18:24 |
| sean-k-mooney | i think im +2 on the first patch as well but im goign to step away for a sec and ill reivew both properly when i get back and leave +w then to gibi | 18:25 |
| sean-k-mooney | dansmith: i commeted on teh normailzeation for what its worth i think the code is correct | 18:52 |
| dansmith | I don't think the code is wrong as-is, it's just not what I was expecting | 18:54 |
| dansmith | I think it's also not wrong to normalize the actual ratio values | 18:55 |
| dansmith | totally independent of the cross-weigher normalization that happens | 18:55 |
| sean-k-mooney | correct but there is also intra weigher nomralistaion | 18:56 |
| sean-k-mooney | before the corss weigher nomalisation hapens we normaliase the wighet for each whost within a weigher | 18:57 |
| sean-k-mooney | this was added for the disk weigher orgianlly but we did it geneically for all weighers | 18:57 |
| dansmith | um, maybe I'm missing something | 18:58 |
| dansmith | if I set os_distro=5000 then it will score os_distro on the same scale as like disk usage or any other weigher in this situation, no? | 18:59 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/weights.py#L144-L146 takes all the final restuls for a given wieher and normaises them in a range of -1 to 1 | 18:59 |
| sean-k-mooney | no it wont | 19:00 |
| sean-k-mooney | for that host it that was all that was set 5000 would be the result of the wight for tha host | 19:01 |
| sean-k-mooney | then we will get the min/max values fo the weigher and normalise the weighte to the range for 0-1.0 | 19:01 |
| sean-k-mooney | https://github.com/openstack/nova/blob/master/nova/weights.py#L144-L146 | 19:01 |
| dansmith | okay I didn't think it worked that way, but that's good.. why can't we just use integers for the per-property weights then? Just seems cleaner than floats (as I noted on an earlier patch) | 19:02 |
| sean-k-mooney | we can | 19:02 |
| sean-k-mooney | it does not need to be a flot but it was for the metrics weigher | 19:02 |
| sean-k-mooney | so that you could exprss 0.5 | 19:02 |
| sean-k-mooney | if you wanted to make someting half as impronat | 19:03 |
| sean-k-mooney | you can obviouly just invert that and multiple the other value by 2 | 19:03 |
| sean-k-mooney | so "to be the same as the metric weigher" is the only reason to allow floats | 19:03 |
| dansmith | yeah, integers would just be better, but it doesn't matter that much | 19:04 |
| sean-k-mooney | it was copied form here https://github.com/openstack/nova/blob/master/nova/weights.py#L144-L146 | 19:04 |
| sean-k-mooney | so i dont really have a stong opipion on that point so what ever works for you, gibi and sylvian is fine by me | 19:05 |
| sean-k-mooney | in case you care about hte history of the normalisation change it was done in https://github.com/openstack/nova/commit/e5ba8494374a1b049eae257fe05b10c5804049ae but that not really impoant | 19:08 |
| mikal | Morning | 19:14 |
| sean-k-mooney | o/ | 19:14 |
| mikal | Apart from people being generally time poor, is there anything blocking the SPICE API changes right now that I should know about? I feel with the tempest test and devstack changes merged we should probably try and merge the matching API microversion sooner rather than later. | 19:17 |
| sean-k-mooney | so https://review.opendev.org/c/openstack/os-traits/+/940418 is still pending and we are coming up to the non client lib freeze so the followup serise for sound and usb is at risk. i think that an easy review ot land IMO but i dont knwo if melwitt will have time to revisit teh patch or if others will | 19:19 |
| sean-k-mooney | i have not looked at the rest in a few days | 19:19 |
| mikal | I think the API changes are more important as they're the most painful bit to keep rebasing, but I would link to try and land all of them if possible. I've been rebasing these things for over a year now. | 19:21 |
| melwitt | sean-k-mooney: for the os-traits patch I don't have an opinion about since I don't really know anything about that stuff. I asked questions in case it could spark yalls memory whether anything was missing or needed to be changed. so if no one else finds any problem about it I think I can +W with your SME +2 on there | 19:23 |
| melwitt | I will look at the API changes again. it can be more challenging if past comments are not addressed yet and we have to keep going back and forth on that. if that's all squared away and the tempest test is done and passing, that's good and helps a lot | 19:26 |
| sean-k-mooney | ack | 19:28 |
| sean-k-mooney | so on the testing font we have a few related patches | 19:28 |
| sean-k-mooney | i have patches up to move the nova-ovs-hybrid-plug job to debain and enabel spice (including speice testing in tempest) | 19:29 |
| sean-k-mooney | that involes change to nova and devstack and tempest | 19:29 |
| sean-k-mooney | melwitt: mikal split there serise in 2 1 for spice direct, and a second indepently mergable vid series for the usb/soudn devices | 19:30 |
| sean-k-mooney | in the spice serise they have a depends on agains the tepmest change and my ci testing series so that pulls everything toghter and showe it working end to end | 19:31 |
| melwitt | sean-k-mooney: ok cool. my understanding is we are focusing on the spice direct, the first 3 patches | 19:32 |
| sean-k-mooney | so https://review.opendev.org/c/openstack/nova/+/924844/36 is the patch that pulls everythign togheter and depend on the rest | 19:32 |
| sean-k-mooney | yes | 19:32 |
| melwitt | right, cool | 19:33 |
| sean-k-mooney | so one thing we might need to do is change the depns on order | 19:34 |
| sean-k-mooney | the devstack and tempest changes have merged | 19:35 |
| sean-k-mooney | so i shoudl proably merge my 2 jobs changes into one and rebase it on mikal's series | 19:35 |
| sean-k-mooney | actuly no | 19:36 |
| sean-k-mooney | it can merge as is | 19:36 |
| sean-k-mooney | in my serise i dont update the regex to enabel the new test | 19:36 |
| sean-k-mooney | that only happens in mikal's | 19:36 |
| mikal | sean-k-mooney: at least one of your patches needs a recheck too I think. | 19:36 |
| sean-k-mooney | i have a pep8 issue | 19:36 |
| melwitt | ack | 19:36 |
| sean-k-mooney | so i need to update mine anyway | 19:36 |
| sean-k-mooney | releasenotes/notes/make-virtio-the-default-spice-video-model-fff5189fa637d4bd.yaml:8: supprot ==> support | 19:37 |
| sean-k-mooney | i have a typo in the release note | 19:37 |
| sean-k-mooney | its in the bottom of my two paches so ill update them both now | 19:39 |
| sean-k-mooney | hum odd that was already fixed | 19:42 |
| sean-k-mooney | ill just rebase them and check again | 19:43 |
| opendevreview | sean mooney proposed openstack/nova master: move nova-ovs-hybrid-plug to deploy with spice and fix qxl default https://review.opendev.org/c/openstack/nova/+/940835 | 19:52 |
| opendevreview | sean mooney proposed openstack/nova master: Dont deploy n-spice on compute nodes. https://review.opendev.org/c/openstack/nova/+/940873 | 19:52 |
| sean-k-mooney | gerrit rejected my rebase as identical | 19:55 |
| sean-k-mooney | so i just reflowed the release note to be less then 80 cols | 19:55 |
| mikal | Oh they typo one? I pushed a fix for that to your review a week or so ago IIRC. | 19:56 |
| sean-k-mooney | ya for some reason pep8 didnt run again? | 20:04 |
| sean-k-mooney | it super odd | 20:04 |
| opendevreview | Merged openstack/nova master: Drop dependency on netifaces https://review.opendev.org/c/openstack/nova/+/931582 | 20:07 |
| opendevreview | Merged openstack/nova master: Reproduce bug/2097359 https://review.opendev.org/c/openstack/nova/+/940603 | 20:07 |
| opendevreview | Merged openstack/nova master: Update InstanceNUMACell version after data migration https://review.opendev.org/c/openstack/nova/+/940604 | 20:07 |
| opendevreview | Merged openstack/nova master: Update InstanceNUMACell version in more cases https://review.opendev.org/c/openstack/nova/+/940953 | 20:45 |
| opendevreview | Merged openstack/nova master: Add support for showing image properties in server show response https://review.opendev.org/c/openstack/nova/+/939649 | 20:46 |
| opendevreview | Merged openstack/nova master: Disable the heal instance info cache periodic task https://review.opendev.org/c/openstack/nova/+/939476 | 20:46 |
| sean-k-mooney | mikal: since https://review.opendev.org/c/openstack/nova/+/939649 was landed we will unfortunlty need to bump the microversion for the spice direct patch | 23:05 |
| opendevreview | Doug Goldstein proposed openstack/nova master: ironic: fix logging of validation errors https://review.opendev.org/c/openstack/nova/+/942019 | 23:19 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Add [libvirt]default_tpm_secret_security https://review.opendev.org/c/openstack/nova/+/940194 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Add [libvirt]supported_tpm_secret_security https://review.opendev.org/c/openstack/nova/+/940195 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Add hw_tpm_secret_security image property https://review.opendev.org/c/openstack/nova/+/940196 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Add hw:tpm_secret_security extra spec validation https://review.opendev.org/c/openstack/nova/+/940197 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Ensure secret security policy for TPM instances https://review.opendev.org/c/openstack/nova/+/941062 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: TPM: support instances with `host` secret security https://review.opendev.org/c/openstack/nova/+/941795 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Allow vTPM live migration for `deployment` secret security https://review.opendev.org/c/openstack/nova/+/925771 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: Modify vTPM LM block to block legacy instances https://review.opendev.org/c/openstack/nova/+/941483 | 23:26 |
| opendevreview | Artom Lifshitz proposed openstack/nova master: WIP: TPM: support instances with `deployment` secret security https://review.opendev.org/c/openstack/nova/+/942021 | 23:26 |
| mikal | sean-k-mooney: ummm, that will require a new tempest change as well then. That's very disappointing. | 23:32 |
| sean-k-mooney | ya i really didnt want ot merge that until your patch was in | 23:33 |
| mikal | https://review.opendev.org/c/openstack/tempest/+/942023 is the tempest renumbering. gmann you'll need to take a look at that one soonish please. | 23:40 |
| mikal | We wont see an end-to-end run though until I get a chance to renumber the nova API changes as well. | 23:41 |
| sean-k-mooney | strictly speakign that should have a depends on against the nova change | 23:41 |
| sean-k-mooney | the tempest changes are only ment to merge after the nova api change | 23:41 |
| sean-k-mooney | i was orginally surrpised to find the tempest patch meged eailier | 23:41 |
| mikal | Ok, I added that. | 23:43 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!