| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2022-07-27 | |||
| 14:46:39 | gibi | my wife tend to cut mine | |
| 14:46:39 | bauzas | cheaper and better | |
| 14:46:55 | gibi | that is the cheapest :) | |
| 14:47:28 | bauzas | yeah but if I use a hair mower, then my hair will have issues :) | |
| 14:48:19 | bauzas | I prefer to have a pro using scissors | |
| 15:48:14 | artom | gibi, so rollback_live_migration_at_destination() is the cast that contains the mutated migrated context, right? | |
| 15:48:26 | artom | So what if we just change it to a call instead of cast to make it blocking? | |
| 15:48:41 | artom | That way, we're sure that by the time we drop the claim, we're out of the mutated context? | |
| 15:49:16 | gibi | that is a dirty thing that will mean we will have a short time window where the db contains the wrong info, then then the second call will return the db content to the correct state | |
| 15:49:23 | gibi | so we will have a smaller race window | |
| 15:49:26 | gibi | but we still have the race | |
| 15:50:02 | artom | You're assuming that mutating the context saves to the DB? | |
| 15:50:05 | artom | I don't think that's the case | |
| 15:50:16 | gibi | we mutate the context and then save the instance today | |
| 15:50:25 | gibi | just the mutate is not an issue | |
| 15:50:57 | gibi | the instance.save happens in _cleanup in the driver | |
| 15:51:06 | gibi | and that is run under the mutated context | |
| 15:52:55 | gibi | links to the code are here https://bugs.launchpad.net/nova/+bug/1982497/comments/2 | |
| 15:53:08 | artom | gibi, oh, right | |
| 15:53:32 | artom | We mutate the context in the compute manager, and in that context we eventually call down to instance.save() in the libvirt driver | |
| 15:53:34 | gibi | hence my second patch that forbids instance.save under the mutated context to avoid this in the fututre | |
| 15:53:41 | gibi | artom: yes | |
| 15:54:00 | artom | Wait, so why did you jump through the sync loops in your reproducer test? | |
| 15:54:05 | artom | The Condition(), for example? | |
| 15:54:56 | gibi | I needed a special serialization order to make sure that we avoid the case when we save the wrong state, then right after we save the good state. | |
| 15:55:25 | artom | Ah | |
| 15:55:53 | gibi | _rollback_live_migration makes the DB invalid then drop_move_claim_at_destination fixes it. | |
| 15:56:31 | gibi | so in that order it is just a small race window (same as if we would make _rollback_live_migration a call instead of a cast) | |
| 15:57:24 | gibi | interestingly in a real environment _rollback_live_migration tend to take longer and finishing _after_ drop_move_claim_at_destination probably due to _rollback_live_migration doing disk IO that can be slow | |
| 15:58:34 | artom | I mean we could just remove the instance.save() from the cleanrup()... | |
| 15:58:41 | artom | And count on the compute manager doing it later... | |
| 15:58:43 | artom | Sounds fragile | |
| 15:59:03 | gibi | artom: yepp that is one way, we can do that and then figure out the fallout | |
| 16:00:04 | gibi | it seems we have that instance.save as delete_instance_files() can fail and we want to retry | |
| 16:00:11 | gibi | later in a periodic if that fails | |
| 16:02:06 | gibi | unfortunately _cleanup not just set instance.cleaned but also init instance.system_metadata['clean_attempts'] that is used in _run_pending_deletes | |
| 16:02:56 | gibi | and honestly not 100% sure if that is the only instance update we do during the full destroy codepath | |
| 16:03:34 | gibi | I think that is the only instance.save but maybe other pieces of code also changes the instance that we would loose if we remove the instance.save | |
| 16:08:07 | melwitt | gmann: fyi the tempest tests for the volume extend coverage have merged, so I think the nova patch to start running the tests is ready for review https://review.opendev.org/c/openstack/nova/+/843700 | |
| 16:08:29 | gmann | melwitt: checking | |
| 16:08:37 | melwitt | thanks! | |
| 16:57:42 | sean-k-mooney | im going to go nap soon i think but i might review the keypair change before or after | |
| 16:57:53 | sean-k-mooney | is that the next microverion in the queue | |
| 17:14:30 | gibi | t is | |
| 17:14:31 | gibi | it is | |
| 17:14:49 | bauzas | I'm just about to upload the OSC patch in 5 mins | |
| 17:14:57 | bauzas | after creating the relnote | |
| 17:15:10 | bauzas | so we'll have all the meat for 2.92 except the tempest tests | |
| 17:15:27 | sean-k-mooney[m] | ok cool | |
| 17:21:57 | bauzas | WTF | |
| 17:22:20 | bauzas | [sbauza@sbauza python-openstackclient]$ git review -s | |
| 17:22:20 | bauzas | Problems encountered installing commit-msg hook | |
| 17:22:20 | bauzas | The following command failed with exit code 255 | |
| 17:22:20 | bauzas | "scp -P29418 sbauza@review.opendev.org:hooks/commit-msg .git/hooks/commit-msg" | |
| 17:26:13 | bauzas | ok, fixed by following https://www.mediawiki.org/wiki/Gerrit/Alternatives_to_git-review | |
| 17:27:28 | bauzas | there it goes : | |
| 17:27:31 | bauzas | gibi: sean-k-mooney: https://review.opendev.org/q/topic:bp%252Fkeypair-generation-removal | |
| 17:27:44 | bauzas | all the changes are up | |
| 17:42:29 | sean-k-mooney[m] | gmann: holding +w to leave bauzas respond to your comments | |
| 17:42:49 | sean-k-mooney[m] | i think they could be adressed in a followup | |
| #openstack-nova - 2022-07-28 | |||
| 01:39:59 | opendevreview | melanie witt proposed openstack/nova master: imagebackend: Cache the key manager when disk is encrypted https://review.opendev.org/c/openstack/nova/+/826756 | |
| 04:34:51 | opendevreview | OpenStack Proposal Bot proposed openstack/nova master: Imported Translations from Zanata https://review.opendev.org/c/openstack/nova/+/851337 | |
| 06:09:11 | gibi | good morning | |
| 08:39:31 | bauzas | sean-k-mooney: gmann: gibi: thanks for your comments on https://review.opendev.org/c/openstack/nova/+/849133 will respin | |
| 09:06:08 | opendevreview | Sylvain Bauza proposed openstack/nova master: api: Drop generating a keypair and add special chars to naming https://review.opendev.org/c/openstack/nova/+/849133 | |
| 09:06:44 | bauzas | gibi: sean-k-mooney: gmann: reviews welcome on the respined keypair api change, just fixed gmann's points and some doc | |
| 09:07:08 | bauzas | https://review.opendev.org/c/openstack/nova/+/849133/7..8 | |
| 09:13:32 | sean-k-mooney | that still looks fine to me so readded +2 | |
| 09:18:54 | kashyap | Can anyone figure out what's the failure here: https://zuul.opendev.org/t/openstack/build/de15983e71c8421c8623f892daaed414 | |
| 09:18:59 | opendevreview | Balazs Gibizer proposed openstack/nova master: Allow enabling PCI tracking in Placement https://review.opendev.org/c/openstack/nova/+/850468 | |
| 09:19:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Retry /reshape at provider generation conflict https://review.opendev.org/c/openstack/nova/+/851358 | |
| 09:19:00 | opendevreview | Balazs Gibizer proposed openstack/nova master: Heal missing PCI allocation in the resource tracker https://review.opendev.org/c/openstack/nova/+/851359 | |
| 09:19:16 | gibi | sean-k-mooney: fyi ^^ I have preliminary allocation healing support | |
| 09:19:54 | gibi | sean-k-mooney: the yesterday's reshape headache was caused by a missing retry logic for reshape when the local provider tree cache is stale | |
| 09:20:09 | gibi | so my code was correct :D | |
| 09:20:50 | gibi | now I will help auniyal then I will look at the keypair patch then look at the double mocking issue | |
| 09:22:30 | admin1 | sean-k-mooney, how do I validate "admin1: no in that config nova should upload the snapshot via the glace http api" ? .. it looks like its calling the http api and getting the error .. | |
| 09:43:25 | sean-k-mooney | admin1: did you check glance to see if the error is on its side. | |
| 09:44:36 | sean-k-mooney | the broken pip could be form reading the image from disk, uploading the image to glance or possible else where | |
| 09:45:24 | sean-k-mooney | but first stpe is proably to determin if there is an error on the glance side or if its just in nova | |
| 09:55:08 | kashyap | bauzas: Since you looked at the master change, can you also please ACK the stable-{wallaby, xena, yoga} backports a the bottom here, please: https://review.opendev.org/q/topic:bug%252F1982853 | |
| 09:55:16 | kashyap | Master change merged | |
| 10:04:54 | bauzas | kashyap: ack | |
| 10:26:48 | kashyap | bauzas: gibi: Both of you gave +2, but neither set Workflow :) - https://review.opendev.org/c/openstack/nova/+/851205 | |
| 10:26:57 | kashyap | (stable/xena) Thanks | |
| 10:27:07 | bauzas | because again, we need to hold for yoga | |
| 10:27:35 | bauzas | don't fight for xena, just relax with a yoga :p | |
| 10:28:09 | bauzas | ok, probably one of the worst puns I made | |
| 10:28:51 | kashyap | Heh | |
| 10:29:10 | kashyap | bauzas: Ah, yes. I thought you +2ed yoga too | |
| 10:29:24 | bauzas | I did +W for yoga | |
| 10:29:44 | bauzas | now that our master is merged | |
| 10:30:22 | kashyap | Yes, you did; I missed to refresh; thx! | |
| 10:30:31 | kashyap | bauzas: Also check wallaby, please, while you have the context | |
| 10:30:45 | kashyap | It has gibi's ACK - https://review.opendev.org/c/openstack/nova/+/851206 | |
| 10:30:55 | bauzas | I'll visit the wallabies | |
| 10:31:40 | bauzas | you sur'ly (ussuri) | |
| 10:32:01 | bauzas | and then I'll take the train | |
| 10:32:26 | bauzas | oh, but again, I missed Victoria | |
| 10:34:04 | kashyap | Heh | |