Re: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.

["Wang, Fan" <fan.wang@intel.com>]

Thanks, Laszlo and Jiaxin, I will have a check about the author name "edk2-devel-bounces@lists.01.org".

Best Regards
Fan

-----Original Message-----
From: Wu, Jiaxin 
Sent: Wednesday, September 19, 2018 9:44 AM
To: Wu, Jiaxin <jiaxin.wu@intel.com>; Laszlo Ersek <lersek@redhat.com>; Subramanian, Sriram <sriram-s@hpe.com>; Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Subramanian@mx0b-002e3701.pphosted.com
Subject: RE: [edk2] [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE attribute when opening SNP protocol installed by PXE.

Correct one of my comments.

The new version patch should *NOT* include the reviewed-by tag from previous version.

Thanks,
Jiaxin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of 
> Wu, Jiaxin
> Sent: Wednesday, September 19, 2018 9:32 AM
> To: Laszlo Ersek <lersek@redhat.com>; Subramanian, Sriram <sriram- 
> s@hpe.com>; Wang, Fan <fan.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; 
> Subramanian@mx0b-002e3701.pphosted.com
> Subject: Re: [edk2] [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE 
> attribute when opening SNP protocol installed by PXE.
> 
> > > Subject: [Patch v2] NetworkPkg: UefiPxeBcDxe: Add EXCLUSIVE 
> > > attribute
> > when opening SNP protocol installed by PXE.
> > >
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1152
> > >
> > > v2: Sync the same logic to Ipv6 and update code comments.
> > >
> > > The PXE driver installs a SNP and open this SNP with attribute 
> > > BY_DRIVER to avoid it being opened by MNP driver, this SNP is also 
> > > expected not to be opened by other drivers with EXCLUSIVE 
> > > attribute. In some cases,
> other
> > > drivers may happen to do this by error, and thus cause a system crash.
> > > This patch adds EXCLUSIVE attribute when opening SNP in PXE 
> > > driver, and will reject all OpenProtocol requests by EXCLUSIVE.
> > >
> > > Cc: Subramanian, Sriram <sriram-s@hpe.com>
> > > Cc: Ye Ting <ting.ye@intel.com>
> > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > Cc: Wu Jiaxin <jiaxin.wu@intel.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Wang Fan <fan.wang@intel.com>
> > > ---
> > >  NetworkPkg/UefiPxeBcDxe/PxeBcDriver.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > Either my edk2-devel subscription is breaking down, or our 
> > discipline regarding commit pushing is down the drain, recently.
> >
> > This patch has been pushed as commit cde5a72d365e. Problems with 
> > that
> > commit:
> >
> > (1) The git authorship is marked as "edk2-devel-bounces@lists.01.org".
> > That's *wrong*. This patch was authored by Wang Fan 
> > <fan.wang@intel.com>.
> >
> 
> Sorry Laszlo, I didn't check the "Author" field before pushing the 
> patch as I received the commit request form Fan. I don't know whether 
> it's the patch reason or TortoiseGit failed to fetch the "Signed-off-by" tag as "Author" field.
> Anyway, I will double check that.
> 
> > (2) The commit lists Siyuan and Jiaxin as having given their 
> > reviews. I don't see their *v2* reviews anywhere on the list. I see 
> > their *v1* reviews, but the patch was changed in v2. Wang Fan was 
> > correct not to carry forward the v1 reviews into the v2 posting. If 
> > the v2 posting was fine, then Siyuan and Jiaxin should have 
> > confirmed that (with R-b's) on the list, before pushing the commit.
> >
> 
> Yes, you are right, Fan should include the v1 reviewer into the v2 
> posting, and the reviewer should confirm the new version patch even 
> he/she is the final version committer.
> 


The new version patch should *NOT* include the reviewed-by tag from previous version. 
 


> > Another process failure that I'm witnessing is that people forget to 
> > close BZs once the corresponding fixes are upstream. Sorry but that 
> > just makes a joke out of bug tracking.
> >
> 
> Thanks the reminder, we will pay attention next time.
> 
> > Come on, guys. We can do better than this. Show some discipline and 
> > dedication to the process, please. The process is not self-serving; 
> > it's in place to promote mutual understanding.
> >
> > Laszlo
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel