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

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

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