| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2025-05-22 | |||
| 01:11:19 | mikal | Like... it would be cool if "git review" added missing DCO entries like it does missing change ids. | |
| 01:11:29 | opendevreview | Michael Still proposed openstack/nova master: Protect older compute managers from sound model requests. https://review.opendev.org/c/openstack/nova/+/940770 | |
| 01:11:29 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra spec for sound device. https://review.opendev.org/c/openstack/nova/+/926126 | |
| 01:11:30 | opendevreview | Michael Still proposed openstack/nova master: libvirt: Add extra specs for USB redirection. https://review.opendev.org/c/openstack/nova/+/927354 | |
| 04:57:34 | opendevreview | sean mooney proposed openstack/nova master: move compile earlier https://review.opendev.org/c/openstack/nova/+/950516 | |
| 04:57:35 | opendevreview | sean mooney proposed openstack/nova master: WIP : Add mtty support to nova-next https://review.opendev.org/c/openstack/nova/+/922140 | |
| 04:57:35 | opendevreview | sean mooney proposed openstack/nova master: Add mtty/mdpy support for testing fake mdevs https://review.opendev.org/c/openstack/nova/+/898100 | |
| 05:03:15 | sean-k-mooney | mikal: "git commit -s" should be all you need to do but im not sure yet about an automatic way yet. most seam to recomemd a git alis like "git config --global alias.cs commit --signoff" but `git commit -s` vs `git cs` does not help much | |
| 05:04:05 | sean-k-mooney | mikal: you do not need ot sign comites with DCO | |
| 05:04:24 | sean-k-mooney | you can but git now supprot using ssh keys instead of GPG keys for sgining if you want to do that | |
| 05:04:29 | mikal | Yeah, that bit confuses me. What's to stop a forgery of a signoff if its just a text string in the commit message? | |
| 05:04:48 | sean-k-mooney | https://dev.to/janderssonse/git-signoff-and-signing-like-a-champ-41f3 | |
| 05:05:41 | mikal | Ahhh, so signoff != signing. | |
| 05:05:42 | sean-k-mooney | mikal: well gerrit will prevent forgin the authero but the commit message can be updated | |
| 05:05:56 | sean-k-mooney | signing commtis wont help because we dont use mege commits | |
| 05:06:13 | sean-k-mooney | so your commit will be rebasees/cherry picked anyway when its merged by zuul | |
| 05:06:35 | mikal | Well, at least this means I can still stick with my 25 year old GPG key! | |
| 05:06:42 | mikal | Its an antique and needs to be cherished. | |
| 05:07:13 | sean-k-mooney | so i have gpg siging setup or at least id did before i reinstalled my laptop.... | |
| 05:07:41 | mikal | I GPG sign tags on some personal projects, although I am unclear on how meaningful that is with such an old key. | |
| 05:07:43 | sean-k-mooney | but i tend not to use it because i had issue with geting it to prompt both locally and if i ssh into my work laptop | |
| 05:08:17 | sean-k-mooney | i.e. if i sshed in it woudl tyr ot pop up a unlock prome in a window on my work laptop instead of in the ssh terminal | |
| 05:08:53 | sean-k-mooney | i think i eventrally got it to work by alwasy forcign the gpg agent to use the tty for promting instead of the shel integration btu i remember that beign a bit of a pain | |
| 05:09:22 | sean-k-mooney | mikal: i mean if that key has never been leaked | |
| 05:09:26 | mikal | Yeah fair enough. I feel like GPG never really took off because its a huge pain in the ass. | |
| 05:09:31 | sean-k-mooney | its a as valid as min | |
| 05:10:12 | sean-k-mooney | ya it kind of used to be. i inally set it up because i wanted to be able to sgin tag for networking-ovs-dpdk and my emails | |
| 05:10:33 | mikal | Dr Google assures me that a 1024 bit DSA key is crackable by motivated players. So that's nice. | |
| 05:10:33 | sean-k-mooney | but signing emails with gpg keys causes all sort of issues at times | |
| 05:13:32 | sean-k-mooney | mikal: by the way git-review is nto the thing that adds the change id for you | |
| 05:14:04 | sean-k-mooney | mikal: we have configured our gerrit to server a commit hook which git review will download and install for you | |
| 05:14:31 | sean-k-mooney | im not sure if we can do something similar ot reject or amend your commit automticlly for you in a sane way | |
| 05:14:53 | mikal | Ahhh ok. My lived experience is "git review" saying "you're missing thingies" and adding them, but I've never looked at the mechanism all that hard. | |
| 05:15:34 | sean-k-mooney | ah https://gerrit.cloudera.org/Documentation/project-configuration.html#_require_signed_off_by | |
| 05:15:42 | sean-k-mooney | so ya we can just turn on the enforcement in gerrit | |
| 05:16:10 | sean-k-mooney | but im not sure about the auto fixign i suspect we woudl need to dig deaper | |
| 05:16:28 | sean-k-mooney | im sure git can suprpot this in a hook but we might need to write it | |
| 05:18:52 | mikal | The commit message hook appears to already understand "Signed-off-by trailers" based on a cursory reading? | |
| 05:19:45 | mikal | It uses https://git-scm.com/docs/git-interpret-trailers to do... things. | |
| 05:22:23 | mikal | Ahhh, ok. I think its trying quite hard to have the Change-Id before the signed off by trailers. Dr Google does suggest that if you turn on DCO enforcement on the project in gerrit then some amount of magic will happen. | |
| 05:27:11 | opendevreview | Abhishek Kekane proposed openstack/nova master: [DNM] Revert^2 "Support glance's new location API" https://review.opendev.org/c/openstack/nova/+/950623 | |
| 05:27:54 | sean-k-mooney | mikal: regarding pre-commit it does supprot writing "commit-msg" hooks which are pass a single parmeter the path to the file | |
| 05:28:36 | sean-k-mooney | mikal: so we coudl write one to either abort the comiit if Signed-off-by was not there ot ro add it for you before the commit was actully finalised | |
| 05:29:03 | mikal | As a forgetful person I quite like the idea of option two there. | |
| 05:29:10 | mikal | It is after all what we already do for change ids. | |
| 05:29:36 | mikal | And my global git config already knows my name and email address, which it needs to know for the log. | |
| 05:29:47 | sean-k-mooney | yep | |
| 05:30:16 | sean-k-mooney | its proably a good weekend hackign project which is also where we shoudl put it | |
| 05:30:30 | sean-k-mooney | in the hacking repo so we can share it acrross other repos | |
| 05:30:33 | mikal | It would also make the transition largely transparent... People do a git review, land a new version of the hook automatically, and blam they're compliant. Much more fun than herding cats. | |
| 05:31:09 | sean-k-mooney | we can add a new hook there and then enable it in the relevent repos https://github.com/openstack/hacking/blob/master/.pre-commit-hooks.yaml | |
| 05:31:42 | mikal | Although... Other projects also use signed-off-by to track who reviewed and approved a change. Is there any intent to do that as well? | |
| 05:34:25 | sean-k-mooney | mikal: that actully a missues of it | |
| 05:34:36 | sean-k-mooney | mikal: there are other standared lables for that | |
| 05:35:04 | sean-k-mooney | mikal: so no not currently however you can actully config gerrit to auto ammend the commit with that info to some degree | |
| 05:35:24 | sean-k-mooney | we had that enabeld downstream untile they moved use to gitlab..... | |
| 05:35:41 | mikal | I kind of like the history being all in one place to be honest. | |
| 05:36:30 | mikal | So yeah, review history in the commit would be cool. I don't really care what labels are used to be honest. I do agree the kernel seems to use "Signed-off-by", "Tested-by", "Reviewed-by", etc. | |
| 05:37:36 | sean-k-mooney | your ment to use Reviewed-by, Acked-by and Tested-by instead of sgined-off by to track that | |
| 05:38:33 | sean-k-mooney | so you coudl update the comit with the review vots and add acted by for the person that +w'd and tested-by by zuul (the verifed label votes) | |
| 05:39:17 | sean-k-mooney | however to do that gerrit would have to be sett to cerry pick mode which will alwasy cheryy pick the commit on to the target branch changign it git sha | |
| 05:39:38 | sean-k-mooney | which will mean you will not be able to knwo the sha to use for backportign until after it has merged because it will alwasy change | |
| 05:40:10 | sean-k-mooney | so to do that we loose our current cherry-picked form workflow/lines when backporting | |
| 05:42:49 | sean-k-mooney | mikal: the convetions for the by lines alll come form teh linke kernel and are part of there patch submission process https://github.com/torvalds/linux/blob/master/Documentation/process/submitting-patches.rst#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes | |
| 07:07:37 | sean-k-mooney | bauzas: just so your aware i rebased your mtty_suppot sersie and moved my patch to the bottom. | |
| 07:08:06 | sean-k-mooney | bauzas: with it i was able to fix the error you were hitting but there is a bug in your first patch https://review.opendev.org/c/openstack/nova/+/898100/11 | |
| 07:08:29 | sean-k-mooney | the error i got with my fix was libvirt.libvirtError: Requested operation is not valid: mediated device /sys/bus/mdev/devices/6dc8d190-2bc9-45ed-a4b8-d4f0708f92bb is in use by driver QEMU, domain instance-00000001 | |
| 07:09:56 | sean-k-mooney | so somehow nova tired to reuse the same mdev between to vms. i assuem that is a bug in https://review.opendev.org/c/openstack/nova/+/898100/11 that only affect virutal mdev device if not that woudl be a real bug in our current mdev supprot | |
| 07:10:26 | sean-k-mooney | i suspect it has somthign to do wtth how your trackign the virutal ones given they do not have a pci adress but im not sure | |
| 07:27:32 | mikal | sean-k-mooney: when you have a moment, I feel like the state of the VDI patches has fallen out of my brain. Are you now happy enough with the first two patches in the series that I could squash them into one and then do the compute protection change to the other set of extra specs? | |
| 07:29:44 | sean-k-mooney | i share your oxidation. ill need to quickly load context | |
| 07:30:11 | sean-k-mooney | you asking about squashing https://review.opendev.org/c/openstack/nova/+/940770/9 and the patch before it | |
| 07:30:14 | sean-k-mooney | or all 3 | |
| 07:31:19 | sean-k-mooney | mikal: i know dan prefers if we sepreate the object change form the driver change | |
| 07:32:12 | sean-k-mooney | so i think there overall prefence woudl be have the first patch containe the object and noticicaton object change. then move the rest of the changes in that to the "Protect older compute managers from sound model requests." change | |
| 07:32:18 | mikal | So at the moment the chain is: sound device extra spec -> compute manager protection for sound -> usb controller extra spec -> compute manager protection for USB (this last one not yet done) | |
| 07:32:43 | mikal | I had thought you'd want the first two squashed so you get the extra spec and the compute protection at the same time. | |
| 07:33:11 | sean-k-mooney | yes i want that btu we geneally prefer to have the object chagns in there won patch | |
| 07:33:24 | sean-k-mooney | so it would still be 4 patchees | |
| 07:33:27 | mikal | I think you're saying for those first two you'd instead prefer to see the first to squashed into one, which becomes the second patch, with the object and notification changes sitting before it? | |
| 07:33:53 | mikal | So: sound objs and notifications -> rest of sound -> usb objs and notifications -> rest of usb? | |
| 07:34:03 | sean-k-mooney | "add object changes for sound" -> "support soudn device" -> "add object changes for usb" "supprot usb" | |
| 07:34:09 | sean-k-mooney | yes | |
| 07:34:39 | mikal | Cool. I can do that. Are you happy with the content of the first two patches, just not the way they're presented? Or are there other changes that need to be made too? | |
| 07:35:02 | sean-k-mooney | mikal: in the unlikely case we need to revert this its easy to rever the driver change but we can really revert the object change | |
| 07:35:17 | sean-k-mooney | so having it be seperate just makes taht edge case or backport a littel simpler | |
| 07:35:47 | mikal | sean-k-mooney: that's fair. I'll do that refactor ASAPish if you're otherwise happy with the code. | |
| 07:36:39 | sean-k-mooney | ill need to look at the detail but i think i can review this series today | |
| 07:36:50 | sean-k-mooney | i dont think there was anything major last time i looked | |
| 07:37:05 | sean-k-mooney | so i dont really expect there to be any thing that need to be changed | |
| 07:37:09 | mikal | Yeah, certainly I can't see anything major in the comments unless I've missed something. | |
| 07:37:29 | mikal | Ok cool. I will do the refactor then and implement the compute manager protection for USB as well. | |
| 09:23:25 | noonedeadpunk | Hey folks! Got a question. Today realized that quite an unexpected behaviour is happening with live migrations. So on memory-intense VMs migration seems to fail on post-migration step. | |
| 09:23:57 | noonedeadpunk | The weird/enexpected thing, that as a result VM keeps shut down on both hosts and it moves to ERROR state | |
| 09:24:22 | noonedeadpunk | so decided to ask if this is smth quite well known? | |
| 09:24:44 | noonedeadpunk | As right now I think it's related to `live_migration_permit_auto_converge = true` | |
| 09:25:32 | noonedeadpunk | as issue happens only on memory intense VMs. So if you run stress inside VM - it will 99% end up in ERROR and shut down. If there's some reasonable applucation running - it's fine | |
| 09:26:17 | noonedeadpunk | So my guess is that if live_migration_permit_auto_converge kicks in, libvirt simply incapable to resume the VM on thye destination, but I'm not really sure if that is true | |
| 09:26:57 | noonedeadpunk | Thus before going to this rabbit hole wanted to ask if you might now anything about that :) | |
| 09:30:15 | frickler | noonedeadpunk: I assume you checked the nova-compute logs without success? how about the libvirt instance logs? but maybe wait for sean-k-mooney to chime in first | |