| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-09-01 | |||
| 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: Adds a repoducer for post live migration fail https://review.opendev.org/c/openstack/nova/+/854499 | |
| 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: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 | gibi | ack, then I'm not needed there :) | |
| 13:28:29 | bauzas | it was just an open thought | |
| 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 | sean-k-mooney | bauzas: let me rephase i dont want to merge only one of thoes two patches | |
| 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: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 | |
| 13:41:21 | dansmith | sean-k-mooney: so a patch in front that adds the flag to the reboot call, then the patch to make it regeneratable, then the api patch to do it for both types would be cleanest I think | |
| 13:41:47 | sean-k-mooney | dansmith: ya that sounds like a plan | |