Earlier  
Later
New Search
Posted Nick Remark
#openstack-nova - 2020-08-26
12:58:02 stephenfin gibi: Functional tests should have caught that. I don't know why it didn't for sure but I suspect the dumb mocking of that cleanup routine is the culprit though
12:58:20 sean-k-mooney lyarwood: did i also miss that i dont rememebr it
12:58:45 gibi stephenfin: if you can catch those things in func test then I can ease on my tempest requirements
12:58:47 bauzas lyarwood: I feel like I was going to be caught in Varennes
12:58:47 stephenfin lyarwood: you mean the question of backporting functional test rework changes
12:58:57 bauzas that's a coup !
12:59:09 lyarwood sean-k-mooney: I think you were there
12:59:11 lyarwood stephenfin: yeah
12:59:14 sean-k-mooney oh that
12:59:16 stephenfin what does a Frenchman know of coups
12:59:21 sean-k-mooney dont we always backport them
12:59:28 sean-k-mooney to keep as close to master as possible
12:59:37 sean-k-mooney to make future backports simpler
12:59:38 lyarwood not always
12:59:40 stephenfin sometimes yes, sometimes no
12:59:52 sean-k-mooney ok i guess it depend on how invasive it is
13:00:00 stephenfin lyarwood's suggesting we always do, even if there are quite a few, since it will ultimately make our lives easier
13:00:04 sean-k-mooney just being pargmatic about it
13:00:18 stephenfin and it's test only so impact on stability should be non-existent
13:00:26 lyarwood yup
13:00:32 bauzas I see
13:00:43 bauzas well, for functional test only changes, this could work
13:01:23 sean-k-mooney bauzas: i think it came up in the context of one of melwitt patches where she would have had to rewrite the tests if she did not backport a refactor patch
13:01:35 sean-k-mooney and i suggested she shoudl backport the refactor
13:01:44 sean-k-mooney to avoid that and keep the backport clean
13:01:48 stephenfin gibi: Also, it's always been broken, i.e. blame efried, not me :P https://review.opendev.org/#/c/639934/30/nova/virt/libvirt/utils.py@691
13:02:07 bauzas sean-k-mooney: which makes sense from a pragmatic pov
13:02:13 efried I blame reviewers
13:02:36 bauzas but again, we all sign-off for maintaining code for half a decade, refrain your style changes
13:02:58 sean-k-mooney bauzas: or backport them
13:03:25 bauzas we all = those who wear some head coverage with a color (that is less red and more blue)
13:03:27 sean-k-mooney the more consitent we are the eaiser it is to grock the code
13:03:36 efried I don't actually recognize that code; I may get to blame mdbooth
13:03:55 bauzas sean-k-mooney: I'd say that providing style changes in an implementation patch is terrible and we should avoid this
13:04:02 efried or cfriesen
13:04:06 gibi stephenfin, efried: I can take the blame found the issue :)
13:04:12 gibi finding even
13:04:15 sean-k-mooney right but type hints are not a style change
13:04:20 sean-k-mooney they are a type of testing
13:04:46 lyarwood have to say I disagree bauzas, we'd never be able to move the codebase forward otherwise
13:04:55 sean-k-mooney its technically a liniting enhancement
13:05:11 stephenfin as someone who's paid to work on the libvirt module, so do I
13:05:14 bauzas gibi: the story of my life https://i.pinimg.com/564x/c5/de/f4/c5def4bd565b44f4d27e1b7478dfa528.jpg
13:05:17 stephenfin so. much. cruft.
13:05:28 sean-k-mooney bauzas: hehe
13:05:33 lyarwood awww ./me hugs nova.virt.libvirt
13:05:45 stephenfin heh
13:05:47 bauzas lyarwood: stephenfin: I don't say we shouldn't modify code
13:06:08 sean-k-mooney bauzas: worst part of git blame. who wrote this crap.. damb it was was me...
13:06:21 stephenfin bauzas: well, take things like this https://review.opendev.org/#/c/631363/61/nova/tests/unit/virt/libvirt/fakelibvirt.py@135
13:06:32 efried Hah! Yes! I get to blame Chris! https://review.opendev.org/#/c/639934/7..8/nova/virt/libvirt/utils.py@630
13:06:35 stephenfin personally, I think that's a good thing
13:07:24 gibi efried: good, he is not around to pass it forward :)
13:07:32 efried Perfect
13:07:56 sean-k-mooney oh recursive=true
13:08:03 stephenfin I mean, I'm touching that code and the additional change makes the whole thing better
13:08:13 bauzas stephenfin: again, no disagreement, just saying that the more we can decouple style changes from implementation changes, the better it would be as we could pull those style changes down to the stable branches without also pulling fancy new features
13:08:35 sean-k-mooney stephenfin: did you see if this is need for livemiration by the way. i think we dont have too copy for live migration but we never validated that
13:08:49 sean-k-mooney stephenfin: so currently we do just to be safe
13:09:04 stephenfin sean-k-mooney: Currently I have live migration blocked because I haven't validated it
13:09:20 sean-k-mooney ok well when you do can you try commenting out the tpm data copy
13:09:22 stephenfin I do have a DevStack env on your cluster that I'll use to validate it once the rest has merged though
13:09:23 sean-k-mooney and see if it works
13:09:28 bauzas stephenfin: fortunately, git is smart enough to handle conflicts on blank lines
13:09:39 stephenfin sean-k-mooney: If we're talking about the same thing, then that's for cold migration
13:09:43 bauzas (or it should)
13:09:51 sean-k-mooney https://review.opendev.org/#/c/639934/30/nova/virt/libvirt/utils.py@691
13:09:56 sean-k-mooney the save and restore
13:10:16 stephenfin Yes, that is necessary. I checked the libvirt docs and included the relevant snippet in the commit message
13:10:31 sean-k-mooney its needed for cold migration
13:10:37 stephenfin Libvirt
13:10:38 stephenfin vTPM device [1].
13:10:38 stephenfin non-shared storage or other file based storages", which includes the
13:10:38 stephenfin migrations in libvirt lingo, do not currently support "copying
13:10:38 stephenfin won't do this automatically for us since cold migrations, or offline
13:10:43 sean-k-mooney stephenfin: but the qemu example seamed to imply it is not for live
13:10:55 sean-k-mooney but there was not clear statement of that
13:11:01 stephenfin ah, yes
13:11:02 stephenfin well
13:11:04 stephenfin <stephenfin> sean-k-mooney: Currently I have live migration blocked because I haven't validated it
13:11:10 stephenfin so I can try when I do the validation, sure :)
13:11:17 sean-k-mooney stephenfin: cool
13:11:38 sean-k-mooney stephenfin: it was an open question in the spec to be figured out when we do the implemnation so jsut reminding you
13:11:42 stephenfin bauzas: Fair. I guess it's just a matter of where to draw the line
13:11:59 stephenfin I'd be less conservative about this stuff that you are, I suspect
13:12:14 bauzas stephenfin: if any of us can just think every time we write on maintainability and upgrades, this would just be perfect.
13:12:35 bauzas stephenfin: totally, and I recognize me being a lame coworker
13:12:36 bauzas :p
13:13:43 stephenfin yeah, stop being so lame!
13:13:45 stephenfin ;)
13:14:17 sean-k-mooney stephenfin: https://github.com/qemu/qemu/blob/master/docs/specs/tpm.rst#migration-with-the-tpm-emulator
13:15:23 sean-k-mooney since there is no copy in those steps i was sepcualtign it was in the testvm.bin file
13:16:23 sean-k-mooney but its also posible this was on the same host so -tpmstate dir=/tmp/mytpm1 could be acceable
13:18:41 openstackgerrit Lucas Alvares Gomes proposed openstack/nova master: DO NOT REVIEW: Test OVN devstack module https://review.opendev.org/748226
13:50:30 openstackgerrit Lee Yarwood proposed openstack/nova master: Ensure source compute is up when confirming a resize https://review.opendev.org/699291
13:50:36 lyarwood gibi: ^ would you be able to take a look at this when you have time?
13:50:45 gibi lyarwood: sue
13:50:49 lyarwood gibi: thanks
13:50:51 gibi sure
13:51:14 lyarwood stephenfin: the rebase to remove the merge conflict just removed your +2 if you can readd

Earlier   Later