[PATCH] Add Connor Kuehl as reviewer for AMD SEV

[PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

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(+)

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

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

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

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Dr. David Alan Gilbert]

* 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
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

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

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Daniel P. Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

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

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Daniel P. Berrangé]

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
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH] Add Connor Kuehl as reviewer for AMD SEV

[Connor Kuehl]

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