From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jes Sorensen Subject: Re: [PATCH] IMSM: Add warning message when x8-type device is used Date: Fri, 21 Oct 2016 10:22:03 -0400 Message-ID: References: <1477042671-12934-1-git-send-email-pawel.baldysiak@intel.com> <1477059381.15890.9.camel@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Return-path: In-Reply-To: <1477059381.15890.9.camel@intel.com> (Pawel Baldysiak's message of "Fri, 21 Oct 2016 14:17:20 +0000") Sender: linux-raid-owner@vger.kernel.org To: "Baldysiak, Pawel" Cc: "linux-raid@vger.kernel.org" List-Id: linux-raid.ids "Baldysiak, Pawel" writes: > On Fri, 2016-10-21 at 08:47 -0400, Jes Sorensen wrote: >> Pawel Baldysiak writes: >> > This patch adds the warning message when x8-type device >> > is used with IMSM metadata. x8 device is a special >> > NVMe drive - two of them on a single PCIe card. >> > This card could be a single point of failure for >> > RAID levels different than RAID0. x8 devices have >> > serial number ending with "-A/-B" or "-1/-2". >> > >> > Signed-off-by: Pawel Baldysiak >> > Reviewed-by: Artur Paszkiewicz >> > --- >> >  super-intel.c | 42 ++++++++++++++++++++++++++++++++++++++++++ >> >  1 file changed, 42 insertions(+) >> Hi Pawel, >> >> When posting a multi-patch series, it would be helpful to have it sent >> with appropriate headers staging the set is X/Y patches and a cover >> letter. >> >> I normally use -n --cover-letter > Hi Jes, > > Sorry about that. Those patches supposed to be sent separately, but I use > one git send-mail command to sent them all. Now I know that this was not > the most clever thing to do :) Please treat them as separate patches. Gotcha! No worries, I can do that! Would you mind fixing up the first one as per the comment below? Thanks, Jes >> >> > diff --git a/super-intel.c b/super-intel.c >> > index 5c6ab05..e860e75 100644 >> > --- a/super-intel.c >> > +++ b/super-intel.c >> > @@ -5139,9 +5139,51 @@ static int add_to_super_imsm(struct >> > supertype *st, mdu_disk_info_t *dk, >> >   rv = imsm_read_serial(fd, devname, dd->serial); >> >   if (rv) { >> >   pr_err("failed to retrieve scsi serial, aborting\n"); >> > + if (dd->devname) >> > + free(dd->devname); >> >   free(dd); >> >   abort(); >> >   } >> > + if (super->hba && ((super->hba->type == SYS_DEV_NVME) || >> > +    (super->hba->type == SYS_DEV_VMD))) { >> > + int i; >> > + char *devpath = diskfd_to_devpath(fd); >> > + char controller_path[PATH_MAX]; >> > + >> > + if (!devpath) { >> > + pr_err("failed to get devpath, aborting\n"); >> > + if (dd->devname) >> > + free(dd->devname); >> > + free(dd); >> > + return 1; >> > + } >> > + >> > + snprintf(controller_path, PATH_MAX-1, "%s/device", devpath); >> > + free(devpath); >> > + >> > + if (devpath_to_vendor(controller_path) == 0x8086) { >> > + /* >> > +  * If Intel's NVMe drive has serial ended with >> > +  * "-A","-B","-1" or "-2" it means that this is "x8" >> > +  * device (double drive on single PCIe card). >> > +  * User should be warned about potential data loss. >> > +  */ >> > + for (i = MAX_RAID_SERIAL_LEN-1; i > 0; i--) { >> > + /* Skip empty character at the end */ >> > + if (dd->serial[i] == 0) >> > + continue; >> > + >> > + if (((dd->serial[i] == 'A') || (dd->serial[i] == 'B') || >> > +    (dd->serial[i] == '1') || (dd->serial[i] == '2')) && >> > +    (dd->serial[i-1] == '-')) >> >> Minor nit here - please keep this within 80 characters. > > Would you like me to resend corrected version? > I saw that those lines are over 80 chars, but I kept it this way. > I think that its more readable right now, but if you prefer I can > break those lines. > > Thanks > Pawel >> >> Thanks, >> Jes