| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-01 | |||
| 10:12:58 | whoami-rajat | https://review.opendev.org/c/openstack/python-openstackclient/+/831014 -- for quick reference | |
| 10:13:42 | sean-k-mooney | alredy reviewed | |
| 10:14:02 | sean-k-mooney | it looks good to me stephenfin ^ your an osc core care to take a look | |
| 10:17:58 | opendevreview | Amit Uniyal proposed openstack/nova master: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/854499 | |
| 10:17:59 | opendevreview | Amit Uniyal proposed openstack/nova master: [compute] always set instnace.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/791135 | |
| 11:14:55 | sean-k-mooney | ricolin: can you update https://review.opendev.org/c/openstack/nova/+/844507/18/nova/virt/libvirt/driver.py#12202 | |
| 11:15:10 | sean-k-mooney | if you can make that change im +2 on both patches | |
| 11:15:14 | sean-k-mooney | bauzas: ^ | |
| 11:15:45 | sean-k-mooney | ill see if i can quickly test your changes in devstack too | |
| 11:15:56 | sean-k-mooney | to confirm this work as intended | |
| 12:46:21 | bauzas | sean-k-mooney: I can review the traits change once ricolin updates it | |
| 12:46:45 | ricolin | Thanks sean-k-mooney: bauzas will do it ASAP | |
| 12:46:51 | bauzas | cool | |
| 12:46:52 | sean-k-mooney | in its current form its not wrong its just over complciated | |
| 12:47:09 | sean-k-mooney | ricolin: i think if you adress that everything else looks ok | |
| 12:47:14 | sean-k-mooney | and it can land before FF | |
| 12:47:37 | ricolin | sean-k-mooney: sounds great | |
| 12:47:39 | sean-k-mooney | i have not had a change to test it locally yet but i expect it to work | |
| 12:47:48 | opendevreview | Amit Uniyal proposed openstack/nova master: [compute] always set instnace.host in post_livemigration https://review.opendev.org/c/openstack/nova/+/791135 | |
| 12:47:48 | opendevreview | Amit Uniyal proposed openstack/nova master: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/854499 | |
| 12:48:03 | sean-k-mooney | ricolin: i assume you ahve booted a vm with this code and validated the iommu is present | |
| 12:55:49 | opendevreview | Rico Lin proposed openstack/nova master: Add traits for viommu model https://review.opendev.org/c/openstack/nova/+/844507 | |
| 12:56:32 | ricolin | sean-k-mooney: done | |
| 12:58:00 | ricolin | and yes, I'm sure IOMMU is there. Just not sure I test Traits in the right way | |
| 13:01:56 | bauzas | ricolin: sean-k-mooney: reviewing | |
| 13:02:06 | ricolin | thanks bauzas | |
| 13:23:51 | sean-k-mooney | bauzas: gibi if ye want to proceed with ^ im more or less happy with it. the release note shoudl be in the seond patch | |
| 13:24:12 | sean-k-mooney | but as long as we land both of them its not an issue | |
| 13:24:26 | bauzas | sean-k-mooney: I just said +1 for an upgrade question | |
| 13:24:36 | sean-k-mooney | i replied | |
| 13:24:43 | sean-k-mooney | thats intentional | |
| 13:25:26 | sean-k-mooney | altough dansmith might point out we could have also added a min compute service check for this instead of using the traits | |
| 13:26:11 | sean-k-mooney | we normally did not dod that in the past however | |
| 13:26:21 | sean-k-mooney | so i think this is all good to go | |
| 13:26:31 | gibi | the trait based capability scheduling looks OK to me | |
| 13:26:45 | gibi | I think dansmith had issues using the capability trait outside of the scheduling | |
| 13:26:55 | dansmith | if we're already scheduling, then traits make plenty of sense | |
| 13:26:57 | dansmith | right | |
| 13:27:10 | sean-k-mooney | yep its in the existing pre filter | |
| 13:27:23 | gibi | bauzas: do you need my +2 or you will send it in? | |
| 13:27:39 | sean-k-mooney | before we started to do these check with placment we would have landed on the host and failed there with hypervior too old or similar | |
| 13:28:11 | bauzas | sorry folks was in 1:1 meeting | |
| 13:28:20 | bauzas | sean-k-mooney: thanks, will reply then with +2 | |
| 13:28:29 | bauzas | it was just an open thought | |
| 13:28:29 | gibi | ack, then I'm not needed there :) | |
| 13:28:51 | bauzas | I just want to make sure that operators know they need to upgrade all their hosts if so | |
| 13:29:03 | bauzas | but that's understandable | |
| 13:29:32 | sean-k-mooney | well its as you said | |
| 13:29:44 | sean-k-mooney | the dont actully have to but it will be capsity limited | |
| 13:30:12 | sean-k-mooney | but that kind of to be expected that you cant use new feature on old nodes | |
| 13:30:20 | bauzas | yup, agreed | |
| 13:30:31 | bauzas | at least with traits | |
| 13:30:36 | sean-k-mooney | well even without | |
| 13:30:54 | bauzas | without, we ask for compute service checks in general | |
| 13:31:00 | sean-k-mooney | yep | |
| 13:31:09 | sean-k-mooney | but that is why i said the traits patch chould be first | |
| 13:31:21 | bauzas | well, if both merge for Zed, I'm cool | |
| 13:31:32 | sean-k-mooney | and the other one secodn and why i said in the current order they need to merge togather | |
| 13:31:51 | bauzas | yes and no | |
| 13:32:14 | bauzas | you could merge the first, this is just you won't be able to get the feature until you upgrade all | |
| 13:32:14 | sean-k-mooney | bauzas: let me rephase i dont want to merge only one of thoes two patches | |
| 13:32:25 | dansmith | gibi: sean-k-mooney: are we FFEing the user_data stuff such that I should try to bang out the rest of the RPC stuff ASAP? | |
| 13:32:32 | bauzas | won't be able to *be sure* to get the feature | |
| 13:32:43 | bauzas | dansmith: eeek, context ? | |
| 13:32:47 | bauzas | oh, the rpc call | |
| 13:33:02 | bauzas | lemme just send to the gate ricolin's work | |
| 13:33:09 | sean-k-mooney | bauzas: that an di guess i need to rev my follow up patch with actul tests | |
| 13:33:45 | sean-k-mooney | dansmith: im not agasint doint that if you think you will have time | |
| 13:34:04 | sean-k-mooney | but that probaly the main RFE that i think FFE might make sense for | |
| 13:34:20 | dansmith | sean-k-mooney: I was expecting to see a rev of the patch to move the regen to after the instance is destroyed and fix the actual writing that was failing | |
| 13:34:30 | dansmith | so I hadn't rebased my RPC stuff yet | |
| 13:34:31 | sean-k-mooney | the only other one might be the PCI series ebut have nto looked at it for two days | |
| 13:34:40 | dansmith | but that has to happen first right? | |
| 13:34:52 | sean-k-mooney | oh i wrote a ptach to fix config drive | |
| 13:35:10 | dansmith | oh is that ths? https://review.opendev.org/c/openstack/nova/+/855351/1 | |
| 13:35:16 | sean-k-mooney | yes | |
| 13:35:41 | gibi | sean-k-mooney: you are +2 up until the healing patches in the PCI and that is the realistic target there. If stephenfin will have no time to review them today then I will ask for an FFE for that. We let the scheduling part slip in any case | |
| 13:35:51 | bauzas | the mutable userdata has API impact | |
| 13:35:52 | dansmith | okay but that would need to be squashed or go in front right? | |
| 13:35:59 | bauzas | but I'm OK with FFE'ing if needed | |
| 13:36:16 | dansmith | bauzas: it would also be about half not-very-reviewed code at this point based on the look of the configdrive patch | |
| 13:36:30 | dansmith | so not really "just didn't get reviewed in time" | |
| 13:36:49 | sean-k-mooney | dansmith: yes or they split the patch in to non config drive and config drive | |
| 13:37:20 | dansmith | that would mean two microversions for effectively the same thing | |
| 13:37:30 | sean-k-mooney | dansmith: i basically started my patch so they would have a refernce for what needed to be done | |
| 13:37:36 | dansmith | unless we do the config regen and rpc ahead of exposing int the api | |
| 13:37:39 | bauzas | dansmith: I think we reviewed it good, but we got a bone | |
| 13:37:59 | bauzas | so, I'm OK with giving more time to review that bone fix | |
| 13:38:06 | dansmith | bauzas: I'm saying the code that needs to be written to make it landable would probably double the actual code in the patch | |
| 13:38:09 | sean-k-mooney | dansmith: yep we coudl do that so intialy it would not be invokeable from the api btut he code would be in place | |
| 13:38:30 | dansmith | sean-k-mooney: yeah that seems better to me | |
| 13:38:32 | bauzas | dansmith: then, we need to take this extratime to balance the risks and maybe not merge it | |
| 13:38:52 | bauzas | or decide we only merge half the things | |
| 13:39:12 | sean-k-mooney | dansmith: so you could proably combin your rpc code into the pathc i started then we could flip the order of them | |
| 13:39:13 | gibi | we could merge the part that support user data update with non config drive instances. | |
| 13:39:26 | sean-k-mooney | but that also means we need to other pepoel to agree to review this | |
| 13:39:33 | sean-k-mooney | since you and i are basicaly out at that point | |
| 13:40:12 | gibi | I can review the patches (I will be around 18:00 CEST today but I can spend time early tomorrow too) | |
| 13:40:31 | gibi | * I will be around until 18:00 CEST today | |
| 13:40:47 | dansmith | sean-k-mooney: right, that's a problem too | |