| Message ID | 20210608192537.103584-1-ckuehl@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Connor Kuehl as reviewer for AMD SEV | expand |
On 6/8/21 2:25 PM, Connor Kuehl wrote: > It may not be appropriate for me to take over as a maintainer at this time, > but I would consider myself familiar with AMD SEV and what this code is > meant to be doing as part of a VMM for launching SEV-protected guests. > > To that end, I would be happy to volunteer as a reviewer for SEV-related > changes so that I am CC'd on them and can help share the review burden with > whoever does maintain this code. > > Signed-off-by: Connor Kuehl <ckuehl@redhat.com> > --- > Note: because there's no maintainer entry, when running > ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu > mailing list is the only thing that shows up... it doesn't even show > previous committers (as it would before applying this patch). Which is > probably not great considering I do not make pull requests to QEMU. > > Is the way forward to get someone to sign up as a maintainer before > applying a patch like this? I need to resend this patch since I realized I forgot to add target/i386/sev_i386.h, and target/i386/sev-stub.c, but I still am wondering about the answer to the question above. Connor
* Connor Kuehl (ckuehl@redhat.com) wrote: > It may not be appropriate for me to take over as a maintainer at this time, > but I would consider myself familiar with AMD SEV and what this code is > meant to be doing as part of a VMM for launching SEV-protected guests. > > To that end, I would be happy to volunteer as a reviewer for SEV-related > changes so that I am CC'd on them and can help share the review burden with > whoever does maintain this code. > > Signed-off-by: Connor Kuehl <ckuehl@redhat.com> Ooh yes please, we could do with someone to be a reviewer; > --- > Note: because there's no maintainer entry, when running > ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu > mailing list is the only thing that shows up... it doesn't even show > previous committers (as it would before applying this patch). Which is > probably not great considering I do not make pull requests to QEMU. > > Is the way forward to get someone to sign up as a maintainer before > applying a patch like this? If you wanted to do a submaintainer for it and send it to one of the x86 maintainers rather than having to do full pulls? Dave > MAINTAINERS | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d9cd29042..3eb7ce8fc6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2938,6 +2938,10 @@ F: hw/core/clock-vmstate.c > F: hw/core/qdev-clock.c > F: docs/devel/clocks.rst > > +AMD Secure Encrypted Virtualization > +R: Connor Kuehl <ckuehl@redhat.com> > +F: target/i386/sev.c > + > Usermode Emulation > ------------------ > Overall usermode emulation > -- > 2.31.1 >
On Tue, Jun 08, 2021 at 02:25:37PM -0500, Connor Kuehl wrote: > It may not be appropriate for me to take over as a maintainer at this time, > but I would consider myself familiar with AMD SEV and what this code is > meant to be doing as part of a VMM for launching SEV-protected guests. > > To that end, I would be happy to volunteer as a reviewer for SEV-related > changes so that I am CC'd on them and can help share the review burden with > whoever does maintain this code. > > Signed-off-by: Connor Kuehl <ckuehl@redhat.com> > --- > Note: because there's no maintainer entry, when running > ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu > mailing list is the only thing that shows up... it doesn't even show > previous committers (as it would before applying this patch). Which is > probably not great considering I do not make pull requests to QEMU. > > Is the way forward to get someone to sign up as a maintainer before > applying a patch like this? There's no requirement to have a maintainer before having a reviewer. If any of the existing committers shown do send pull requests, it is probably co-incidental since they're not listed as official maintainers, and being listed as Reviewer doesn't commit you to doing pull requests. That said if you're the only nominated reviewer and actually do useful reviews, you will probably quickly find yourself the defacto maintainer in 12 months time and end up doing pull requests... > MAINTAINERS | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7d9cd29042..3eb7ce8fc6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2938,6 +2938,10 @@ F: hw/core/clock-vmstate.c > F: hw/core/qdev-clock.c > F: docs/devel/clocks.rst > > +AMD Secure Encrypted Virtualization > +R: Connor Kuehl <ckuehl@redhat.com> > +F: target/i386/sev.c Regards, Daniel
On 6/8/21 3:10 PM, Daniel P. Berrangé wrote: > On Tue, Jun 08, 2021 at 02:25:37PM -0500, Connor Kuehl wrote: >> It may not be appropriate for me to take over as a maintainer at this time, >> but I would consider myself familiar with AMD SEV and what this code is >> meant to be doing as part of a VMM for launching SEV-protected guests. >> >> To that end, I would be happy to volunteer as a reviewer for SEV-related >> changes so that I am CC'd on them and can help share the review burden with >> whoever does maintain this code. >> >> Signed-off-by: Connor Kuehl <ckuehl@redhat.com> >> --- >> Note: because there's no maintainer entry, when running >> ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu >> mailing list is the only thing that shows up... it doesn't even show >> previous committers (as it would before applying this patch). Which is >> probably not great considering I do not make pull requests to QEMU. >> >> Is the way forward to get someone to sign up as a maintainer before >> applying a patch like this? > > There's no requirement to have a maintainer before having a reviewer. > If any of the existing committers shown do send pull requests, it is > probably co-incidental since they're not listed as official maintainers, > and being listed as Reviewer doesn't commit you to doing pull requests. > > That said if you're the only nominated reviewer and actually do useful > reviews, you will probably quickly find yourself the defacto maintainer > in 12 months time and end up doing pull requests... Right, I am just worried that if I am the only person that shows up in the get_maintainer.pl output, the submitter will have to know some other way who a relevant maintainer is that can take the patches otherwise they won't be CC'd. Or we'll have to hope a relevant maintainer sees them on the list. Or I'll have to chase down a maintainer myself assuming the reviews all check out. :-) Connor
On Tue, Jun 08, 2021 at 03:32:54PM -0500, Connor Kuehl wrote: > On 6/8/21 3:10 PM, Daniel P. Berrangé wrote: > > On Tue, Jun 08, 2021 at 02:25:37PM -0500, Connor Kuehl wrote: > >> It may not be appropriate for me to take over as a maintainer at this time, > >> but I would consider myself familiar with AMD SEV and what this code is > >> meant to be doing as part of a VMM for launching SEV-protected guests. > >> > >> To that end, I would be happy to volunteer as a reviewer for SEV-related > >> changes so that I am CC'd on them and can help share the review burden with > >> whoever does maintain this code. > >> > >> Signed-off-by: Connor Kuehl <ckuehl@redhat.com> > >> --- > >> Note: because there's no maintainer entry, when running > >> ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu > >> mailing list is the only thing that shows up... it doesn't even show > >> previous committers (as it would before applying this patch). Which is > >> probably not great considering I do not make pull requests to QEMU. > >> > >> Is the way forward to get someone to sign up as a maintainer before > >> applying a patch like this? > > > > There's no requirement to have a maintainer before having a reviewer. > > If any of the existing committers shown do send pull requests, it is > > probably co-incidental since they're not listed as official maintainers, > > and being listed as Reviewer doesn't commit you to doing pull requests. > > > > That said if you're the only nominated reviewer and actually do useful > > reviews, you will probably quickly find yourself the defacto maintainer > > in 12 months time and end up doing pull requests... > > Right, I am just worried that if I am the only person that shows up in > the get_maintainer.pl output, the submitter will have to know some other > way who a relevant maintainer is that can take the patches otherwise > they won't be CC'd. Or we'll have to hope a relevant maintainer sees > them on the list. Or I'll have to chase down a maintainer myself > assuming the reviews all check out. :-) Well there's no real guarantee that any of the previous committers will take the patch even if they are listed by get_maintainer. This is typical with anything lacking a maintainer assigned. We typically hope that whoever runs the "misc" queue sees the patch and picks it up, but often it requires pings to remind someone to pick it up. The only real right answer here is to actually get someone as the nominated maintainer. Every other scenario is a just a band aid and is not a good experiance for contributors. A nominated reviewer is usually hoped to be a stepping stone to someone becoming maintainer in future, so in that sense the fact that only you will be cc'd is sort of intentional :-) Regards, Daniel
On 6/8/21 2:34 PM, Dr. David Alan Gilbert wrote: >> Note: because there's no maintainer entry, when running >> ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu >> mailing list is the only thing that shows up... it doesn't even show >> previous committers (as it would before applying this patch). Which is >> probably not great considering I do not make pull requests to QEMU. >> >> Is the way forward to get someone to sign up as a maintainer before >> applying a patch like this? > > If you wanted to do a submaintainer for it and send it to one of the x86 > maintainers rather than having to do full pulls? I'm not opposed to this. I think I have a few of the right people on CC, so let's see if they weigh in on this. Unless it means I have to manage a GPG key again... (just kidding, kind of...) Connor
On 6/8/21 3:45 PM, Daniel P. Berrangé wrote: >> Right, I am just worried that if I am the only person that shows up in >> the get_maintainer.pl output, the submitter will have to know some other >> way who a relevant maintainer is that can take the patches otherwise >> they won't be CC'd. Or we'll have to hope a relevant maintainer sees >> them on the list. Or I'll have to chase down a maintainer myself >> assuming the reviews all check out. :-) > > Well there's no real guarantee that any of the previous committers will > take the patch even if they are listed by get_maintainer. This is typical > with anything lacking a maintainer assigned. We typically hope that > whoever runs the "misc" queue sees the patch and picks it up, but often > it requires pings to remind someone to pick it up. > > The only real right answer here is to actually get someone as the > nominated maintainer. Every other scenario is a just a band aid and > is not a good experiance for contributors. A nominated reviewer is > usually hoped to be a stepping stone to someone becoming maintainer > in future, so in that sense the fact that only you will be cc'd is > sort of intentional :-) That makes perfect sense. I'll forge on ahead, then :-) Thanks! Connor
diff --git a/MAINTAINERS b/MAINTAINERS index 7d9cd29042..3eb7ce8fc6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2938,6 +2938,10 @@ F: hw/core/clock-vmstate.c F: hw/core/qdev-clock.c F: docs/devel/clocks.rst +AMD Secure Encrypted Virtualization +R: Connor Kuehl <ckuehl@redhat.com> +F: target/i386/sev.c + Usermode Emulation ------------------ Overall usermode emulation
It may not be appropriate for me to take over as a maintainer at this time, but I would consider myself familiar with AMD SEV and what this code is meant to be doing as part of a VMM for launching SEV-protected guests. To that end, I would be happy to volunteer as a reviewer for SEV-related changes so that I am CC'd on them and can help share the review burden with whoever does maintain this code. Signed-off-by: Connor Kuehl <ckuehl@redhat.com> --- Note: because there's no maintainer entry, when running ./scripts/get_maintainers.pl on target/i386/sev.c, my name and the qemu mailing list is the only thing that shows up... it doesn't even show previous committers (as it would before applying this patch). Which is probably not great considering I do not make pull requests to QEMU. Is the way forward to get someone to sign up as a maintainer before applying a patch like this? MAINTAINERS | 4 ++++ 1 file changed, 4 insertions(+)