Design Session: Using Gitlab MR for Xen

Design Session: Using Gitlab MR for Xen

[Deb Giles]

[-- Attachment #1.1: Type: text/plain, Size: 267 bytes --]

Hello,

Attached are the notes and public chat record for the Design Session: Using
Gitlab MR for Xen.

Thanks,

Deb

Deb Giles
Event Manager
The Linux Foundation
+1.503.807.1876 (Time Zone: Central Time)

*Schedule a Meeting with Me* <https://calendly.com/debgiles>

[-- Attachment #1.2: Type: text/html, Size: 1938 bytes --]

[-- Attachment #2: bbb-Shanghai[public-chat]_2021-5-27_13-33.txt --]
[-- Type: text/plain, Size: 16875 bytes --]


            [11:45] Welcome to Shanghai!For help on using BigBlueButton see these (short) tutorial videos.To join the audio bridge click the phone button.  Use a headset to avoid causing background noise for others.This server is running BigBlueButton.
[11:45] To invite someone to the meeting, send them this link: https://xensummit.linuxfoundation.org/b/shanghai
[11:48] Deb Giles - LF Events Team: If you'd like screen share abilities, just let me know and I can make you presenter
[11:50] penny zheng: so many arm folks lol
[11:58] Juergen Gross (jgross): Deb, can you please make me presenter?
[11:59] Deb Giles - LF Events Team: You're all set!
[12:00] Roger Pau Monne: I wanted to ask whether the Linux kernel/QEMU code could be accepted without any spec change?
[12:07] OleksandrT: Initially I created new "vdisk" property for virtio disk, but was asked to reuse existing "disk" configuration in libxl if possible)  https://lists.xenproject.org/archives/html/xen-devel/2021-05/msg01174.html
[12:13] stacktrust: Important for Automotive, FuSA, security
[12:19] Christopher Clark: https://openxt.atlassian.net/wiki/spaces/DC/pages/1333428225/Analysis+of+Argo+as+a+transport+medium+for+VirtIO
[12:20] Christopher Clark: https://openxt.atlassian.net/wiki/spaces/DC/pages/1696169985/VirtIO-Argo%2BDevelopment%2BPhase%2B1
[12:20] Christopher Clark: https://openxt.atlassian.net/wiki/spaces/DC/pages/1348763698/VirtIO%2BArgo
[12:24] Damien Thenot (dthenot): Shared memory would be interesting for DPU too
[12:25] Christopher Clark: https://www.platformsecuritysummit.com/2018/speaker/pratt/
[12:27] Christopher Clark: ack on different perf characteristics for different hardware architectures
[12:28] Demi Marie Obenour: RISC-V only has 32-bit atomics
[12:28] Demi Marie Obenour: Long-term fix is probably to change the guest-visible ABI
[12:30] Demi Marie Obenour: To not require atomics in shared memory
[12:30] Jason Andryuk: @Andrew, you use netfront/back, but your dom0 nic scatter/gathers over the guest's granted  network frames directly?
[12:30] Andrew Cooper: yeah
[12:31] Andrew Cooper: a consequence is that dom0 never touches the mapping, so never sets the A bit, so you can skip the TLB flush on the unmap side
[12:31] Andrew Cooper: and this is great for performance
[12:31] Jason Andryuk: Fancy :)
[12:33] Roger Pau Monne: note you need to patch Xen to not do the flush on unmap if the A bit isn't set :)
[12:37] Andrew Cooper: in some copious free time, we need to upstream a version of that which doesn't trust dom0 to be non-malicious
[12:38] Stefano Stabellini: This is WindRiver KVMtools that does shared memory for virtio https://github.com/OpenAMP/kvmtool
[12:39] Roger Pau Monne: I think both
[12:39] Daniel P Smith: oh, i misunderstood, @stefano i thought you said there was a move to a copy mechanism
[12:41] Stefano Stabellini: @daniel, yes I meant copy to a pre-shared memory region
[12:42] Stefano Stabellini: sorry for the confusion
[12:42] Daniel P Smith: okay, no worries. thank you!
[12:42] Wei Liu: That's in the newer spec right?
[12:44] Deb Giles - LF Events Team: This is a 1 minute warning before the next design session is scheduled to start
[12:45] Christopher Clark: Thanks for the discussion
[12:46] Daniel P Smith: thank you juergen for hosting!
[12:47] Deb Giles - LF Events Team: We can if you'd like!
[12:47] Deb Giles - LF Events Team: Thedy've been saved
[12:48] Stefano Stabellini: @daniel you have mail
[12:49] julien-f: GitLab does have an API, not sure how useful for Merge Requests it is though
[12:50] Stefano Stabellini: MR are not the issue. We could use them today. The important issue where do the reviews go.
[12:51] Bobby Eshleman: +1 the first setup is not intuitive at all
[12:51] julien-f: Yeah, I was talking about MR reviews :-)
[12:51] Olivier Lambert: haha yeah Bobby I remember it took you some time to be setup
[12:52] Scott Davis: Comments in the MR would work for reviews, no?
[12:52] Stefano Stabellini: yes but they don't work well if someone else is replying to emails
[12:52] Ian Jackson: MR comments aren't attached to particular commits.
[12:53] Samuel Verschelde: Ian: I think they are if you comment from the commit itself
[12:53] Ian Jackson: The forges (gitlab github) let you do review line-by-line in code via their UI but it is really web ui only and has its own problems.
[12:54] Stefano Stabellini: the issue is that we ran a test and wasn't integrating very well with the email workflow. I can't remember exactly why. Was it because the notification emails didn't retain the author of the comment?
[12:54] Bertrand Marquis: following pending patches is also a lot easier in a web ui, the filtering is a lot easier
[12:55] Olivier Lambert: @Stefano : well the test was half baked I would say
[12:55] Olivier Lambert: that's why we should do an on-boarding to be sure it's truly done "the best way" possible
[12:55] Olivier Lambert: with people with a bit of Gitlab experience
[12:55] Olivier Lambert: and see then if it's OK or not
[12:56] Julien Grall: Why are we sticking to gitlab?
[12:56] Stefano Stabellini: it is open source and we already use the CI-loop there
[12:56] Julien Grall: Or is the problem because of the web UI in general?
[12:56] Bertrand Marquis: git push is more simple then git send-email :-)
[12:56] Bertrand Marquis: from what i recall the problem was due to the fact that it was not possible to do both UI and mail reviews in parallel
[12:56] Julien Grall: Stefano: Right, I am trying to figure out if people are against web UI in general or just gitlab.
[12:57] Bertrand Marquis: definitely gitlab is not the only answer, there might be others
[12:57] Christopher Clark: I like email. Please excuse me for a while while I tend to my lawn.
[12:57] Bertrand Marquis: advantage of gitlab is CI part we already have i would say
[12:57] julien-f: IMHO, you should not try to port your existing email workflow to GitLab, but add a new workflow on GitLab for simple changes to help attract new contributors
[12:58] Olivier Lambert: +1
[12:58] Anthony PERARD (anthonyper): git range-diff ;-)
[12:58] Bertrand Marquis: following threads and comments is nice in gitlab i find because you can answer to a specific comment
[12:59] Demi Marie Obenour: If registering an account on GitLab automatically generated credentials for `git send-email, that would really help
[12:59] Samuel Verschelde: You can compare between successive iterations of a given commit directly in the web UI
[12:59] Bertrand Marquis: +1
[12:59] Demi Marie Obenour: Can we ask the GHC (Glasgow Haskell Compiler) team how they do code review?
[12:59] Bertrand Marquis: fully agree with Ian here
[13:00] Demi Marie Obenour: They have a massive amount of experience using GitLab for this, having previously used Phabricator
[13:00] Demi Marie Obenour: They have managed to use GitLab to review commit-by-commit.
[13:01] Jeppe: I see an issue with bar to entry outlined in this discussion. Perhaps there can be a landing page at xen project that says "hey thanks for contributing, we would love your patch... here is how to submit, gitlab has limitations for us sadly" and then provide ... X (an explanation of email submission or... a in between solution that is hopefully easier then email for a new  contributor?)
[13:01] George Dunlap (gwd): Jeppe: We have one.  the problem is i'ts super complicated
[13:01] Demi Marie Obenour: That is still a high barrier to entry, hence my suggestion of automatically sending the email.
[13:01] George Dunlap (gwd): INherently complicated; it can't be made simpler.
[13:02] Samuel Verschelde: Demi: I think Gitlab's UI has most one needs to review commit by commit, with successive interations (force pushes) for the same commit series. The main issue is that it's a different process than the current process. It would be easier if we were starting from nothing.
[13:02] Olivier Lambert: indeed, bending previous/existing workflow won't really fit
[13:02] Jeppe: @George Ah, I see. ;)
[13:04] marmarek: reviewable.io is *really* powerful for handling several revisions of the series, and tracking state of each commit or even chunk; but it's yet another web ui
[13:04] Demi Marie Obenour: Can we have a bot that automatically strips those disclaimers?
[13:05] George Dunlap (gwd): That might break DKIM
[13:05] Demi Marie Obenour: It would; we would need to rewrite From headers
[13:05] Demi Marie Obenour: But that is basically a given nowadays.
[13:05] Olivier Lambert: yeah we had the same issue here to find there was a v2 on a patch series
[13:06] Andrew Cooper: https://lore.kernel.org/xen-devel/
[13:07] Wei Liu: Try b4. It is really good.
[13:08] Olivier Lambert: Should we try to start using the full web UI workflow for "small" MRs first?
[13:08] Olivier Lambert: and see where it's going frol there
[13:08] Olivier Lambert: *from
[13:09] Doug Goldstein: Sorry. I missed the early part of the conversation.
[13:09] Olivier Lambert: hey Doug :)
[13:09] Doug Goldstein: I'm watching the text convo but I had a work meeting I couldn't get out of.
[13:10] Demi Marie Obenour: What about SourceHut?
[13:10] Jeppe: How many tools are available to manage huge code bases? It seems to me (as an outsider in this). Perhaps, one can look at several  OSS and find one that works with small contributors? Then one may vote on a solution? Or is that not possible for a technical reason.
[13:10] Olivier Lambert: I think it's a matter of leaving the email client at some point.
[13:10] Doug Goldstein: Ultimately what we do I would really like to see us adopt a process that's similar to another project. Another larger project.
[13:11] Doug Goldstein: I don't want to see us off on an island where we have to maintain a lot of extra side projects.
[13:11] George Dunlap (gwd): Doug: Did you have a large project in mind?
[13:12] Doug Goldstein: Not specifically. But projects like QEMU are good to look towards.
[13:12] Ian Jackson: I absolutely agree with what Andy is saying
[13:12] Stefano Stabellini: QEMU is still doing email reviews, but they switched to MRs for pull requests (not reviews)
[13:12] Wei Liu: Cloud Hypervisor uses GitHub, and at the same time adhere to high standard  for commit messages.
[13:13] Wei Liu: It can be done.
[13:13] julien-f: That's perfectly possible with GitLab
[13:13] Samuel Verschelde: Yes, this means no "fixup fixup" commits in merge requests, but incremental refining of commits.
[13:13] julien-f: You can rebase and merge the commits
[13:13] Jeppe: Can a solution be found where extremely verbose history is baked in?
[13:13] Stefano Stabellini: I agree with Andy
[13:13] George Dunlap (gwd): Wei: You mean the commit messages have a summary of all the "paths not taken" and why?
[13:14] Wei Liu: No. I mean commit messages are close to what Linux kernel / Xen have for archaeology purposes.
[13:14] Olivier Lambert: I think the only way to prove it is to actually try while having people around to help on using it :)
[13:14] George Dunlap (gwd): I just spent a week doing data mining on the 17-year history of xen-devel, so I definitely appreciate the open format.
[13:15] Wei Liu: It is just that people need to be vigilant about not merging things with crappy commit messages.
[13:15] Doug Goldstein: Agreed Wei.
[13:15] Bertrand Marquis: gitlab or other things like that are still generating mails which could be archived
[13:15] George Dunlap (gwd): But that's not what Andy is talking about; it's about going back 10 years ago and seeing all the "paths not taken" discussion and why they weren't taken.
[13:15] Bobby Eshleman: Rustc is a great  example of big / complicated project using MR process
[13:16] Wei Liu: Yes, Rust as well
[13:17] Samuel Verschelde: Can't you export every data from gitlab?
[13:17] julien-f: Not sure about review comments…
[13:17] Scott Davis: +1 for being able to manage code with MRs and have them automatically get sent to email list for discussion to occur
[13:17] Doug Goldstein: Require a PR to have a thread on xen-devel
[13:17] Olivier Lambert: you can export but IDK on which format exactly
[13:17] julien-f: I don't treat them as permanent data, if something is necessary, it should either be a comment code or a commit message
[13:18] Olivier Lambert: but yeah you can get the sources of GL and import on your own instance
[13:18] julien-f: @Scott that could work yes :-)
[13:18] Stefano Stabellini: right that would be easy to do: gitlab for MR and tests but mailing list for reviews. That would be OK.
[13:18] Anthony PERARD (anthonyper): I've just found out how QEMU does work around email submission: https://wiki.qemu.org/Contribute/SubmitAPatch#If_you_cannot_send_patch_emails
[13:18] George Dunlap (gwd): I mean long-term we could host our own gitlab instance, and then the project at least as a whole would have the whole history
[13:18] julien-f: @Stefano not sure it really helps new contributors
[13:19] julien-f: the main pain point appears to be review
[13:20] Bertrand Marquis: what is the interest of pushing to MR and doing review by mail after ?
[13:21] Stefano Stabellini: one of the points was tracking and that would help
[13:21] julien-f: Exactly :-p
[13:21] Scott Davis: submitter can just add the acked by to the MR
[13:21] Edwin Török: https://gerrit.googlesource.com/plugins/reviewnotes/%2B/master/src/main/resources/Documentation/refs-notes-review.md
[13:21] Edwin Török: https://gerrit.googlesource.com/plugins/reviewnotes/%2B/master/src/main/resources/Documentation/refs-notes-review.md
[13:21] Julien Grall: The main issue is not quoting but disclaimer. Technically we should not answer to such e-mail (they are meant to be private).
[13:22] Stefano Stabellini: QEMU;s solution might work: https://wiki.qemu.org/Contribute/SubmitAPatch#If_you_cannot_send_patch_emails
[13:23] Ian Jackson: Most of the disclaimers just say it "may" be private or some such.  That's not a problem.
[13:23] Bertrand Marquis: in gerrit you can modify the commit message directly in the UI
[13:23] Bertrand Marquis: this prevents v58 of patches
[13:23] Samuel Verschelde: Having used gerrit in the past I love it despite its awful UI and the way it forces you to refine until it's perfect
[13:23] Bertrand Marquis: maybe this can also be done in some other UI
[13:23] Olivier Lambert: yeah there's some bot on Gitlab that could do that
[13:23] Samuel Verschelde: (second part of my comment is a plus in fact)
[13:23] Julien Grall: Ian: Note sure we should take the chance if the patch is merged.
[13:24] Julien Grall: s/Note/not/
[13:24] Olivier Lambert: I agree with Edwin: we need to start first on something on the side.
[13:24] Olivier Lambert: consider it as a "new way"
[13:24] Bertrand Marquis: we have a gitlab
[13:24] Bertrand Marquis: we use it for Fusa or PCI right now
[13:26] Bertrand Marquis: following comments among patch versions in gitlab is definitely nice
[13:26] Julien Grall: +1 with what Ian says. The tracking would be easier.
[13:26] Bobby Eshleman: Being able to resolve comments is indeed very nice.  Combing through email to double check each comment has been addressed in the revision is a pain.
[13:27] julien-f: +1
[13:27] Olivier Lambert: +1 too :)
[13:27] julien-f: Start simple and fix things as you go
[13:28] Scott Davis: the advantage is not having to setup git-send-email
[13:28] Bertrand Marquis: also the history, you keep track of patches pending for review
[13:28] Scott Davis: I just did that and it took the better part of a workday to figure it all out for my first patch sub
[13:29] Jeppe: Oh no!
[13:29] Jeppe: lol
[13:29] julien-f: Exactly!
It's a new workflow
[13:29] Olivier Lambert: yeah indeed
[13:29] Scott Davis: though part of that was just finding all the riught documentation
[13:29] Ian Jackson: Right.  We do need a small git-send-email robot then
[13:29] Ian Jackson: Is anyone volunteering to set one up?
[13:29] Jeppe: (I thought only I test the most difficult first) lol
[13:29] Ian Jackson: Could be a bit shoddy and ad-hoc for now
[13:30] Deb Giles - LF Events Team: This is a 1 minute warning to wrap up this design session before closing remarks begin in Budapest
[13:30] Deb Giles - LF Events Team: https://xensummit.linuxfoundation.org/b/budapest
[13:30] Samuel Verschelde: :)
[13:30] Olivier Lambert: Thanks Ian :)
[13:31] Doug Goldstein: Ian, search if someone has already made one.
[13:31] Bertrand Marquis: thanks for the discussion
[13:31] julien-f: Thanks all!
[13:31] Jeppe: TY
[13:31] Olivier Lambert: Thanks!
[13:31] Doug Goldstein: You might be able to reuse some code.
[13:31] Bobby Eshleman: Great convo!  Thanks all
[13:32] Edwin Török: btw it'd be useful if this public chat got saved together with the shared notes
[13:32] Edwin Török: they both contain useful information, would that be possible Deb?
[13:33] Deb Giles - LF Events Team: Yes, I can save the public chat too

[-- Attachment #3: Design Session - Using Gitlab MR for Xen.docx --]
[-- Type: application/vnd.openxmlformats-officedocument.wordprocessingml.document, Size: 13219 bytes --]