Re: [Bug 121531] Adaptec 7805H SAS HBA (pm80xx): hangs when writing >80MB at once

2016-07-07 Thread Viswas G
Patch set for pm80xx is pending for the last 3 quarters.
We will submit those soon with all the buf fixes and  performance
tuning changes.

Regards,
Viswas G

On Thu, Jul 7, 2016 at 10:04 PM,   wrote:
> https://bugzilla.kernel.org/show_bug.cgi?id=121531
>
> --- Comment #14 from Jack Wang  ---
> The changes are huge, hard to do it, without help from MicroSemi/PMCs side.
> And I don't have hardware to test.
>
> I've asked developer from MicroSemi to upsteam their changes, but
> sadly no reply.
>
> 2016-07-07 17:36 GMT+02:00  :
>> https://bugzilla.kernel.org/show_bug.cgi?id=121531
>>
>> --- Comment #13 from Martin von Wittich  ---
>> Yup, the Debian installation worked and seems to work fine so far. I manually
>> installed the pm80xx-1.4.0-11068-debian64.deb package in the target system
>> while I was still in the installer by chrooting to /target; then I added
>> "pm80xx" to /etc/initramfs/modules (without that update-initramfs wouldn't 
>> add
>> the pm80xx module, I'm not really sure why).
>>
>> After booting the system, I've succcessfully written ~500 GB of /dev/zero 
>> data
>> into a file on an MD raid consisting of both of the disks. No error messages 
>> in
>> the dmesg either.
>>
>> Can you include those missing changes into the official kernel, or how can we
>> resolve this bug? We'll ask to the customer if we can keep the server for an
>> additional two weeks for testing, so if you need me to test builds, let me
>> know.
>>
>> --
>> You are receiving this mail because:
>> You are on the CC list for the bug.
>
> --
> You are receiving this mail because:
> You are the assignee for the bug.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

2016-07-07 Thread Rasmus Villemoes
On Tue, Jul 05 2016, Markus Mayer  wrote:

> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
>
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
>
> +/**
> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be 0 to set no limit.
> + *
> + * Returns pointer to terminating '\0' in @dst.
> + */
> +char *strncpytoupper(char *dst, const char *src, size_t len)
> +{
> + size_t i;
> +
> + for (i = 0; src[i] != '\0' && (i < len || !len); i++)
> + dst[i] = toupper(src[i]);
> + if (i < len || !len)
> + dst[i] = '\0';
> +
> + return dst + i;
> +}

Hm, this seems to copy the insane semantics from strncpy of not
guaranteeing '\0'-termination.

Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
well and require a little less code (no || !len)?

I regret suggesting this return semantics and now agree that void would
be better, especially since there doesn't seem to be anyone who can
use this (or any other) return value. How about

if (!len)
   return;

for (i = 0; i < len && src[i]; ++i)
  dst[i] = toupper(src[i]);
dst[i < len ? i : i-1] = '\0';

(I think you must do i < len before testing src[i], since the len
parameter should be an upper bound on the number of bytes to access in
both src and dst).

Rasmus
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: Double-Fetch bug in Linux-4.5/drivers/scsi/aacraid/commctrl.c

2016-07-07 Thread David Carroll
> -Original Message-
> From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi-
> ow...@vger.kernel.org] On Behalf Of Pengfei Wang
> Sent: Thursday, July 07, 2016 7:00 AM
> To: linux-scsi@vger.kernel.org
> Subject: Re: Double-Fetch bug in Linux-4.5/drivers/scsi/aacraid/commctrl.c
> Hi,
> 
> Will anyone bother to confirm and fix this problem I reported last time? From
> the point of view of security, I think it should be fixed.
> I have discovered several cases of the same kind and all have been fixed by
> the maintainers. Thanks!
> 
> Pengfei
> 

Hi Pengfei,

I'm currently working on a patch set for the HBA-1000 card, and I agree with 
your assessment. We will check the sizes and return an error if the size is 
larger than when first checked;
 i.e.

if (copy_from_user(kfib, arg, size)) {
retval = -EFAULT;
goto cleanup;
}

if (unlikely((le16_to_cpu(kfib->header.Size) + sizeof(struct 
aac_fibhdr)) > size)) {
retval = -EINVAL;
goto cleanup;
}

Thanks, -Dave



Re: [RFC] libata-scsi: introducing SANITIZE translation

2016-07-07 Thread James Bottomley
On Fri, 2016-07-08 at 00:32 +0800, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> With this patch, users can make use of the SANITIZE DEVICE feature
> set through utility like sg_sanitize.
> 
> Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE
> has been implemented. Support for OVERWRITE that involves a
> parameter list has been left out for now.
> 
> Further support for command with IMMED bit set to zero, REQUEST
> SENSE translation for user-space status polling, and support
> checking in IDENTIFY DEVICE data log (return proper sense data
> when designated method is not supported) should be implemented
> in the future as well.
> 
> `sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.

Why on earth would you want to do this?  If your intent is to sanitise
the disk using a cryptographic erase you presumably have a real
security need for doing it and, knowing what goes into most SAT layers,
I'd not really trust any SAT for this operation, so for an underlying
SATA device I'd use ATA_16 to send a real ACS-2 SANITIZE command.

Just as a general note about our SAT layer: Adding little used features
is an invitation to bloat it with buggy implementations which makes it
harder to understand and bug prone for odd and unlikely use cases,
which then take ages to diagnose and track down.  The only things which
should be in the SAT is what the Linux SCSI subsystem would actually
use.  For everything else, if the user cares enough, they'll send down
an encapsulated ATA command.

James


--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Bug 121531] Adaptec 7805H SAS HBA (pm80xx): hangs when writing >80MB at once

2016-07-07 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=121531

--- Comment #14 from Jack Wang  ---
The changes are huge, hard to do it, without help from MicroSemi/PMCs side.
And I don't have hardware to test.

I've asked developer from MicroSemi to upsteam their changes, but
sadly no reply.

2016-07-07 17:36 GMT+02:00  :
> https://bugzilla.kernel.org/show_bug.cgi?id=121531
>
> --- Comment #13 from Martin von Wittich  ---
> Yup, the Debian installation worked and seems to work fine so far. I manually
> installed the pm80xx-1.4.0-11068-debian64.deb package in the target system
> while I was still in the installer by chrooting to /target; then I added
> "pm80xx" to /etc/initramfs/modules (without that update-initramfs wouldn't add
> the pm80xx module, I'm not really sure why).
>
> After booting the system, I've succcessfully written ~500 GB of /dev/zero data
> into a file on an MD raid consisting of both of the disks. No error messages 
> in
> the dmesg either.
>
> Can you include those missing changes into the official kernel, or how can we
> resolve this bug? We'll ask to the customer if we can keep the server for an
> additional two weeks for testing, so if you need me to test builds, let me
> know.
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.

-- 
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[RFC] libata-scsi: introducing SANITIZE translation

2016-07-07 Thread tom . ty89
From: Tom Yan 

With this patch, users can make use of the SANITIZE DEVICE feature
set through utility like sg_sanitize.

Support for BLOCK ERASE, CRYPTOGRAPHIC ERASE and EXIT FAILURE MODE
has been implemented. Support for OVERWRITE that involves a
parameter list has been left out for now.

Further support for command with IMMED bit set to zero, REQUEST
SENSE translation for user-space status polling, and support
checking in IDENTIFY DEVICE data log (return proper sense data
when designated method is not supported) should be implemented
in the future as well.

`sg_sanitize -e -B|-C|-F /dev/sdX` should work fine with this.

Signed-off-by: Tom Yan 

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfec66f..a64991b 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3346,6 +3346,63 @@ invalid_opcode:
return 1;
 }
 
+static unsigned int ata_scsi_sanitize_xlat(struct ata_queued_cmd *qc) {
+   struct ata_taskfile *tf = >tf;
+   struct scsi_cmnd *scmd = qc->scsicmd;
+   struct ata_device *dev = qc->dev;
+   const u8 *cdb = scmd->cmnd;
+   u16 fp;
+   u8 bp = 0xff;
+
+   /* for now we only support SANITIZE with IMMED bit set */
+   if (unlikely(!(cdb[1] & 0x80))) {
+   fp = 1;
+   bp = 7;
+   goto invalid_fld;
+   }
+
+   tf->protocol = ATA_PROT_NODATA;
+   tf->command = ATA_CMD_SANITIZE_DEVICE;
+   tf->hob_nsect |= (cdb[1] & 0x40) << 1;
+   tf->nsect |= (cdb[1] & 0x20) >> 1;
+   tf->flags |= ATA_TFLAG_LBA48 | ATA_TFLAG_ISADDR;
+
+   switch (cdb[1] & 0x1f) {
+   /* TODO: add support for OVERWRITE */
+   case 0x2: /* BLOCK ERASE */
+   tf->hob_feature = 0x0;
+   tf->feature = 0x12;
+   tf->hob_lbal = 0x42;
+   tf->lbah = 0x6b;
+   tf->lbam = 0x45;
+   tf->lbal = 0x72;
+   break;
+   case 0x3: /* CRYPTOGRAPHIC ERASE */
+   tf->hob_feature = 0x0;
+   tf->feature = 0x11;
+   tf->hob_lbal = 0x43;
+   tf->lbah = 0x72;
+   tf->lbam = 0x79;
+   tf->lbal = 0x70;
+   break;
+   case 0x1f: /* EXIT FAILURE MODE */
+   tf->hob_feature = 0x0;
+   tf->feature = 0x0;
+   tf->nsect |= 0x1;
+   break;
+   default:
+   fp = 1;
+   bp = 4;
+   goto invalid_fld;
+   }
+
+   return 0;
+
+invalid_fld:
+   ata_scsi_set_invalid_field(dev, scmd, fp, bp);
+   return 1;
+}
+
 /**
  * ata_scsi_report_zones_complete - convert ATA output
  * @qc: command structure returning the data
@@ -3869,6 +3926,9 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct 
ata_device *dev, u8 cmd)
case WRITE_SAME_16:
return ata_scsi_write_same_xlat;
 
+   case SANITIZE:
+   return ata_scsi_sanitize_xlat;
+
case SYNCHRONIZE_CACHE:
if (ata_try_flush_cache(dev))
return ata_scsi_flush_xlat;
diff --git a/include/scsi/scsi_proto.h b/include/scsi/scsi_proto.h
index d1defd1..20d6085 100644
--- a/include/scsi/scsi_proto.h
+++ b/include/scsi/scsi_proto.h
@@ -72,6 +72,7 @@
 #define UNMAP0x42
 #define READ_TOC  0x43
 #define READ_HEADER   0x44
+#define SANITIZE  0x48
 #define GET_EVENT_STATUS_NOTIFICATION 0x4a
 #define LOG_SELECT0x4c
 #define LOG_SENSE 0x4d
-- 
2.9.0

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Bug 121531] Adaptec 7805H SAS HBA (pm80xx): hangs when writing >80MB at once

2016-07-07 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=121531

--- Comment #13 from Martin von Wittich  ---
Yup, the Debian installation worked and seems to work fine so far. I manually
installed the pm80xx-1.4.0-11068-debian64.deb package in the target system
while I was still in the installer by chrooting to /target; then I added
"pm80xx" to /etc/initramfs/modules (without that update-initramfs wouldn't add
the pm80xx module, I'm not really sure why).

After booting the system, I've succcessfully written ~500 GB of /dev/zero data
into a file on an MD raid consisting of both of the disks. No error messages in
the dmesg either.

Can you include those missing changes into the official kernel, or how can we
resolve this bug? We'll ask to the customer if we can keep the server for an
additional two weeks for testing, so if you need me to test builds, let me
know.

-- 
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH trivial] fcoe: use defines from ethtool for 20Gbit and 40Gbit speeds

2016-07-07 Thread Laurence Oberman


- Original Message -
> From: "Johannes Thumshirn" 
> To: "Martin K . Petersen" , "James Bottomley" 
> , "Jiri Kosina"
> 
> Cc: "Linux SCSI Mailinglist" , 
> fcoe-de...@open-fcoe.org, "Johannes Thumshirn"
> 
> Sent: Thursday, July 7, 2016 9:41:58 AM
> Subject: [PATCH trivial] fcoe: use defines from ethtool for 20Gbit and 40Gbit 
> speeds
> 
> Use defines from ethtool for 20Gbit and 40Gbit speeds instead of magic
> numbers.
> 
> Signed-off-by: Johannes Thumshirn 
> ---
>  drivers/scsi/fcoe/fcoe_transport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/fcoe/fcoe_transport.c
> b/drivers/scsi/fcoe/fcoe_transport.c
> index 641c60e..7028dd3 100644
> --- a/drivers/scsi/fcoe/fcoe_transport.c
> +++ b/drivers/scsi/fcoe/fcoe_transport.c
> @@ -133,10 +133,10 @@ int fcoe_link_speed_update(struct fc_lport *lport)
>   case SPEED_1:
>   lport->link_speed = FC_PORTSPEED_10GBIT;
>   break;
> - case 2:
> + case SPEED_2:
>   lport->link_speed = FC_PORTSPEED_20GBIT;
>   break;
> - case 4:
> + case SPEED_4:
>   lport->link_speed = FC_PORTSPEED_40GBIT;
>   break;
>   default:
> --
> 1.8.5.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Looks fine, simple change
Reviewed-by: Laurence Oberman 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH trivial] fcoe: use defines from ethtool for 20Gbit and 40Gbit speeds

2016-07-07 Thread Johannes Thumshirn
Use defines from ethtool for 20Gbit and 40Gbit speeds instead of magic
numbers.

Signed-off-by: Johannes Thumshirn 
---
 drivers/scsi/fcoe/fcoe_transport.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_transport.c 
b/drivers/scsi/fcoe/fcoe_transport.c
index 641c60e..7028dd3 100644
--- a/drivers/scsi/fcoe/fcoe_transport.c
+++ b/drivers/scsi/fcoe/fcoe_transport.c
@@ -133,10 +133,10 @@ int fcoe_link_speed_update(struct fc_lport *lport)
case SPEED_1:
lport->link_speed = FC_PORTSPEED_10GBIT;
break;
-   case 2:
+   case SPEED_2:
lport->link_speed = FC_PORTSPEED_20GBIT;
break;
-   case 4:
+   case SPEED_4:
lport->link_speed = FC_PORTSPEED_40GBIT;
break;
default:
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

2016-07-07 Thread Tom Yan
Yeah it said "One identification descriptor..." but not "Only one
identification descriptor...". So I suppose it is *alright* to also
return t10 designator in addition to the WWN. It's just redundunt and
unnecessary (and that's why I sent the patch), since we only need one
LU name (either derive from the WWN, or the t10 identification when
WWN is not available):

If the ATA IDENTIFY DEVICE data word 87 bit 8 is set to one indicating
that the ATA device supports the
WORLD WIDE NAME field (i.e., ATA IDENTIFY DEVICE data words 108 to
111), then the SATL shall include a
designation descriptor containing a logical unit name as defined in 10.5.4.2.2.

If the ATA IDENTIFY DEVICE data word 87 bit 8 is set to zero,
indicating that the ATA device does not support
the WORLD WIDE NAME field (i.e., ATA IDENTIFY DEVICE data words 108 to
111), then the SATL shall include
an identification descriptor containing a logical unit name as defined
in 10.5.4.2.3.

(sat4r05f.pdf)

On 7 July 2016 at 20:44, Hannes Reinecke  wrote:
> On 07/07/2016 02:40 PM, Tom Yan wrote:
>> Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
>> ATA PASS-THROUGH) though.
>>
>> Anyway I expected the reasoning you gave and I can't really argue with
>> you. It's just personally I still prefer a cleaner SATL implementation
>> (considering Linux is open source and can be deemed as some sort of
>> reference), so I gave it a go.
>>
>> Not that SAT requires the DI VPD return only one desingator / LU name though.
>>
> Really?
>
> sat-r08 has:
>
> One identification descriptor for a logical unit (i.e., a logical unit
> name) shall be included (see clause 10.3.4.2).
> In some environments, one or more additional identification descriptors
> may be included (see clause 10.3.4.3).
>
> Am I misreading something?
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes ReineckeTeamlead Storage & Networking
> h...@suse.de   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Double-Fetch bug in Linux-4.5/drivers/scsi/aacraid/commctrl.c

2016-07-07 Thread Pengfei Wang
Hi,

Will anyone bother to confirm and fix this problem I reported last
time? From the point of view of security, I think it should be fixed.
I have discovered several cases of the same kind and all have been
fixed by the maintainers. Thanks!

Pengfei


在 2016年4月26日,下午5:00,Pengfei Wang  写道:

Hi,


在 2016年4月26日,下午3:46,James Bottomley  写道:

On Tue, 2016-04-26 at 13:35 +0100, Pengfei Wang wrote:

Hello,

I found this Double-Fetch bug in
Linux-4.5/drivers/scsi/aacraid/commctrl.c when I was examining the
source code.

In function ioctl_send_fib(), the driver fetches user space data by
pointer arg via copy_from_user(), and this happens twice at line 81
and line 116 respectively. The first fetched value (stored in kfib)
is used to get the header and calculate the size at line 90 so as to
copy the whole message later at line 116, which means the copy size
of the whole message is based on the old value that came from the
first fetch. Besides, the whole message copied in the  second fetch
also contains the header.

However, when the function processes the message after the second
fetch at line 130, it uses kfib->header.Size that came from the
second fetch, which might be different from the one came from the
first fetch as well as calculated the size to copy the message from
user space to driver.


I don't actually see where there's a bug in this.  If the user chooses
to alter data in-flight (quite hard to do because one thread of
execution is already tied up in the ioctl) then the consequences are
their own fault.


A Double-Fetch happens when the kernel reads the same user data multiple
times, whilst the data is likely to be modified by a concurrently running user
thread under race condition between the kernel reads, which results in data
inconsistency for the kernel use. Since neither the kernel nor the user is aware
of the change of the data under race condition, this data inconsistency might
cause serious consequences.

I call this Double-Fetch situation as a bug is because I think it lacks a
sanity checking after the second fetch of the same data, which should be
a guarantee for the data consistency for kernel use, and I have seen
this checking
mechanism in some other driver files. Moreover, I don’t think it is the user’s
fault if the data is altered, for the reason that the data might be
altered by an
intended malicious process launched by an attacker or rewritten by
some unintended user
behaviors that are not supposed to happen. As long as the data resides in user
space, there is a chance that it is modified under race condition, even though
the chance might be very small. Besides, it should be the kernel’s
duty to handle
potential data inconsistency in Double-Fetch.


If the kfib->header.Size is modified by a user thread under race
condition between the fetch operations, for example changing to a
very large value, this will lead to over-boundary access or other
serious consequences in function aac_fib_send().


Our only real concern would be could an unprivileged user crash the
kernel this way and the user must have CAP_SYS_RAWIO anyway (which is
quite a high privilege capability) plus the only problem with
shortening the data would be EFAULT.

I’m not quite sure what consequence will actually be caused if this Double-Fetch
really happens, maybe not as serious as a kernel crash, but I think it
would actually
results in something wrong in the driver functionality, which should
be eliminated now.



Thank you

Pengfei
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: better style in ata_msense_caching()

2016-07-07 Thread Sergei Shtylyov

On 07/07/2016 03:44 PM, Tom Yan wrote:


Sorry I am a bit new in this. Does that mean I should also use {} for
the if clause even it has only one line of statement, because I
(needed to) use that for the else clause? So it should be like:




if ... {
...
} else {
...
...
}


   Exactly, see Documentation/CodingStyle, chapter 3.

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

2016-07-07 Thread Hannes Reinecke
On 07/07/2016 02:40 PM, Tom Yan wrote:
> Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
> ATA PASS-THROUGH) though.
> 
> Anyway I expected the reasoning you gave and I can't really argue with
> you. It's just personally I still prefer a cleaner SATL implementation
> (considering Linux is open source and can be deemed as some sort of
> reference), so I gave it a go.
> 
> Not that SAT requires the DI VPD return only one desingator / LU name though.
> 
Really?

sat-r08 has:

One identification descriptor for a logical unit (i.e., a logical unit
name) shall be included (see clause 10.3.4.2).
In some environments, one or more additional identification descriptors
may be included (see clause 10.3.4.3).

Am I misreading something?

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: better style in ata_msense_caching()

2016-07-07 Thread Tom Yan
Sorry I am a bit new in this. Does that mean I should also use {} for
the if clause even it has only one line of statement, because I
(needed to) use that for the else clause? So it should be like:

if ... {
...
} else {
...
...
}

On 7 July 2016 at 18:55, Sergei Shtylyov
 wrote:
>
>CodingStyle: {} should be used in all branches if used in at least one.
>
> MBR, Sergei
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

2016-07-07 Thread Tom Yan
Well, udev uses its own `ata_id` (which issues IDENTIFY DEVICE through
ATA PASS-THROUGH) though.

Anyway I expected the reasoning you gave and I can't really argue with
you. It's just personally I still prefer a cleaner SATL implementation
(considering Linux is open source and can be deemed as some sort of
reference), so I gave it a go.

Not that SAT requires the DI VPD return only one desingator / LU name though.

On 7 July 2016 at 18:56, Hannes Reinecke  wrote:
> On 07/07/2016 12:12 AM, tom.t...@gmail.com wrote:
>> From: Tom Yan 
>>
>> SAT (as of sat4r05f.pdf) only requires the t10 designator if the
>> drive does not support/have WWN. Besides, we already have the ATA
>> information VPD.
>>
>> Signed-off-by: Tom Yan 
>>
>> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
>> index 9f478ad..84b3d42 100644
>> --- a/drivers/ata/libata-scsi.c
>> +++ b/drivers/ata/libata-scsi.c
>> @@ -2210,21 +2210,6 @@ static unsigned int ata_scsiop_inq_83(struct 
>> ata_scsi_args *args, u8 *rbuf)
>>   rbuf[1] = 0x83; /* this page code */
>>   num = 4;
>>
>> - /* SAT defined lu model and serial numbers descriptor */
>> - /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
>> - rbuf[num + 0] = 2;
>> - rbuf[num + 1] = 1;
>> - rbuf[num + 3] = sat_model_serial_desc_len;
>> - num += 4;
>> - memcpy(rbuf + num, "ATA ", 8);
>> - num += 8;
>> - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD,
>> -   ATA_ID_PROD_LEN);
>> - num += ATA_ID_PROD_LEN;
>> - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO,
>> -   ATA_ID_SERNO_LEN);
>> - num += ATA_ID_SERNO_LEN;
>> -
>>   if (ata_id_has_wwn(args->id)) {
>>   /* SAT defined lu world wide name */
>>   /* piv=0, assoc=lu, code_set=binary, designator=NAA */
>> @@ -2236,6 +2221,23 @@ static unsigned int ata_scsiop_inq_83(struct 
>> ata_scsi_args *args, u8 *rbuf)
>> ATA_ID_WWN, ATA_ID_WWN_LEN);
>>   num += ATA_ID_WWN_LEN;
>>   }
>> + else {
>> + /* SAT defined lu model and serial numbers descriptor */
>> + /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
>> + rbuf[num + 0] = 2;
>> + rbuf[num + 1] = 1;
>> + rbuf[num + 3] = sat_model_serial_desc_len;
>> + num += 4;
>> + memcpy(rbuf + num, "ATA ", 8);
>> + num += 8;
>> + ata_id_string(args->id, (unsigned char *) rbuf + num, 
>> ATA_ID_PROD,
>> +   ATA_ID_PROD_LEN);
>> + num += ATA_ID_PROD_LEN;
>> + ata_id_string(args->id, (unsigned char *) rbuf + num, 
>> ATA_ID_SERNO,
>> +   ATA_ID_SERNO_LEN);
>> + num += ATA_ID_SERNO_LEN;
>> + }
>> +
>>   rbuf[3] = num - 4;/* page len (assume less than 256 bytes) */
>>   return 0;
>>  }
>>
> Nope.
> We cannot go about and remove VPD descriptors.
> Existing systems might rely on the VPD attribute to be present (think of
> udev persistent symlinks), and the system will refuse to boot.
> NACK.
>
> Cheers,
>
> Hannes
> --
> Dr. Hannes ReineckeTeamlead Storage & Networking
> h...@suse.de   +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] xen-scsiback: correct return value checks on xenbus_scanf()

2016-07-07 Thread Juergen Gross
On 07/07/16 10:01, Jan Beulich wrote:
> Only a positive return value indicates success.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Juergen Gross 

> ---
>  drivers/xen/xen-scsiback.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- 4.7-rc6-xenbus_scanf.orig/drivers/xen/xen-scsiback.c
> +++ 4.7-rc6-xenbus_scanf/drivers/xen/xen-scsiback.c
> @@ -1071,7 +1071,7 @@ static void scsiback_do_1lun_hotplug(str
>   /* read status */
>   snprintf(state, sizeof(state), "vscsi-devs/%s/state", ent);
>   err = xenbus_scanf(XBT_NIL, dev->nodename, state, "%u", _state);
> - if (XENBUS_EXIST_ERR(err))
> + if (err <= 0)
>   return;
>  
>   /* physical SCSI device */
> @@ -1089,7 +1089,7 @@ static void scsiback_do_1lun_hotplug(str
>   snprintf(str, sizeof(str), "vscsi-devs/%s/v-dev", ent);
>   err = xenbus_scanf(XBT_NIL, dev->nodename, str, "%u:%u:%u:%u",
>  , , , );
> - if (XENBUS_EXIST_ERR(err)) {
> + if (err != 4) {
>   xenbus_printf(XBT_NIL, dev->nodename, state,
> "%d", XenbusStateClosed);
>   return;
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] xen-scsifront: correct return value checks on xenbus_scanf()

2016-07-07 Thread Juergen Gross
On 07/07/16 10:01, Jan Beulich wrote:
> Only a positive return value indicates success.
> 
> Signed-off-by: Jan Beulich 

Acked-by: Juergen Gross 

> ---
>  drivers/scsi/xen-scsifront.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- 4.7-rc6-xenbus_scanf.orig/drivers/scsi/xen-scsifront.c
> +++ 4.7-rc6-xenbus_scanf/drivers/scsi/xen-scsifront.c
> @@ -1000,14 +1000,14 @@ static void scsifront_do_lun_hotplug(str
>   snprintf(str, sizeof(str), "vscsi-devs/%s/state", dir[i]);
>   err = xenbus_scanf(XBT_NIL, dev->otherend, str, "%u",
>  _state);
> - if (XENBUS_EXIST_ERR(err))
> + if (err <= 0)
>   continue;
>  
>   /* virtual SCSI device */
>   snprintf(str, sizeof(str), "vscsi-devs/%s/v-dev", dir[i]);
>   err = xenbus_scanf(XBT_NIL, dev->otherend, str,
>  "%u:%u:%u:%u", , , , );
> - if (XENBUS_EXIST_ERR(err))
> + if (err != 4)
>   continue;
>  
>   /*
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Bug 121531] Adaptec 7805H SAS HBA (pm80xx): hangs when writing >80MB at once

2016-07-07 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=121531

--- Comment #12 from Martin von Wittich  ---
@Jack: No, I hadn't tried that yet, but I will now:

I downloaded the driver from the homepage and installed the
adaptec/debian_7.4/x64/pm80xx-1.4.0-11068-debian64.deb package on our dev
server. After DKMS had compiled the pm80xx.ko module, I unloaded the pm80xx
module on the installer live system and replaced
/lib/modules/3.16.0-4-amd64/kernel/drivers/scsi/pm8001/pm80xx.ko with the newly
compiled module and loaded it again. It seems to work better... I've been able
to write 1024 MB successfully:

root@(none):~# dd if=/dev/zero of=/dev/sdc bs=1M count=1024
1024+0 records in
1024+0 records out
1073741824 bytes (1.1 GB) copied, 6.51408 s, 165 MB/s

I'm now installing Debian onto the machine, I'll report back how that turns
out.

-- 
You are receiving this mail because:
You are the assignee for the bug.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

2016-07-07 Thread Eric Engestrom
On Tue, Jul 05, 2016 at 01:47:05PM -0700, Markus Mayer wrote:
> All functions return a pointer to the terminating '\0' character in the
> modified string ("dst" or "s", respectively).

I think this is going to be confusing. From the man:

The strcpy() and strncpy() functions return a pointer to the
destination string dest.

I think it would be better to keep the same behaviour, especially since
you used the same name for your functions (which I think is sensible),
not to mention you don't use this return value in any of your calls.
(IMHO strcpy() shouldn't have had a return value and neither should your
functions, but since it does, yours should match.)

Cheers,
  Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

2016-07-07 Thread Hannes Reinecke
On 07/07/2016 12:12 AM, tom.t...@gmail.com wrote:
> From: Tom Yan 
> 
> SAT (as of sat4r05f.pdf) only requires the t10 designator if the
> drive does not support/have WWN. Besides, we already have the ATA
> information VPD.
> 
> Signed-off-by: Tom Yan 
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 9f478ad..84b3d42 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -2210,21 +2210,6 @@ static unsigned int ata_scsiop_inq_83(struct 
> ata_scsi_args *args, u8 *rbuf)
>   rbuf[1] = 0x83; /* this page code */
>   num = 4;
>  
> - /* SAT defined lu model and serial numbers descriptor */
> - /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
> - rbuf[num + 0] = 2;
> - rbuf[num + 1] = 1;
> - rbuf[num + 3] = sat_model_serial_desc_len;
> - num += 4;
> - memcpy(rbuf + num, "ATA ", 8);
> - num += 8;
> - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_PROD,
> -   ATA_ID_PROD_LEN);
> - num += ATA_ID_PROD_LEN;
> - ata_id_string(args->id, (unsigned char *) rbuf + num, ATA_ID_SERNO,
> -   ATA_ID_SERNO_LEN);
> - num += ATA_ID_SERNO_LEN;
> -
>   if (ata_id_has_wwn(args->id)) {
>   /* SAT defined lu world wide name */
>   /* piv=0, assoc=lu, code_set=binary, designator=NAA */
> @@ -2236,6 +2221,23 @@ static unsigned int ata_scsiop_inq_83(struct 
> ata_scsi_args *args, u8 *rbuf)
> ATA_ID_WWN, ATA_ID_WWN_LEN);
>   num += ATA_ID_WWN_LEN;
>   }
> + else {
> + /* SAT defined lu model and serial numbers descriptor */
> + /* piv=0, assoc=lu, code_set=ACSII, designator=t10 vendor id */
> + rbuf[num + 0] = 2;
> + rbuf[num + 1] = 1;
> + rbuf[num + 3] = sat_model_serial_desc_len;
> + num += 4;
> + memcpy(rbuf + num, "ATA ", 8);
> + num += 8;
> + ata_id_string(args->id, (unsigned char *) rbuf + num, 
> ATA_ID_PROD,
> +   ATA_ID_PROD_LEN);
> + num += ATA_ID_PROD_LEN;
> + ata_id_string(args->id, (unsigned char *) rbuf + num, 
> ATA_ID_SERNO,
> +   ATA_ID_SERNO_LEN);
> + num += ATA_ID_SERNO_LEN;
> + }
> +
>   rbuf[3] = num - 4;/* page len (assume less than 256 bytes) */
>   return 0;
>  }
> 
Nope.
We cannot go about and remove VPD descriptors.
Existing systems might rely on the VPD attribute to be present (think of
udev persistent symlinks), and the system will refuse to boot.
NACK.

Cheers,

Hannes
-- 
Dr. Hannes ReineckeTeamlead Storage & Networking
h...@suse.de   +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: better style in ata_msense_caching()

2016-07-07 Thread Sergei Shtylyov

On 7/7/2016 4:43 AM, tom.t...@gmail.com wrote:


From: Tom Yan 

Signed-off-by: Tom Yan 

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bfec66f..e3f5751 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2424,10 +2424,12 @@ static void modecpy(u8 *dest, const u8 *src, int n, 
bool changeable)
 static unsigned int ata_msense_caching(u16 *id, u8 *buf, bool changeable)
 {
modecpy(buf, def_cache_mpage, sizeof(def_cache_mpage), changeable);
-   if (changeable || ata_id_wcache_enabled(id))
-   buf[2] |= (1 << 2);   /* write cache enable */
-   if (!changeable && !ata_id_rahead_enabled(id))
-   buf[12] |= (1 << 5);  /* disable read ahead */
+   if (changeable)
+   buf[2] |= (1 << 2); /* ata_mselect_caching() */


Parens not really needed.


+   else {


   CodingStyle: {} should be used in all branches if used in at least one.


+   buf[2] |= (ata_id_wcache_enabled(id) << 2);   /* write 
cache enable */
+   buf[12] |= (!ata_id_rahead_enabled(id) << 5); /* disable 
read ahead */


   Outer parens not needed.


+   }
return sizeof(def_cache_mpage);
 }



MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] libata-scsi: do not return t10 designator if drive has WWN

2016-07-07 Thread Sergei Shtylyov

Hello.

On 7/7/2016 1:12 AM, tom.t...@gmail.com wrote:


From: Tom Yan 

SAT (as of sat4r05f.pdf) only requires the t10 designator if the
drive does not support/have WWN. Besides, we already have the ATA
information VPD.

Signed-off-by: Tom Yan 

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 9f478ad..84b3d42 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c

[...]

@@ -2236,6 +2221,23 @@ static unsigned int ata_scsiop_inq_83(struct 
ata_scsi_args *args, u8 *rbuf)
  ATA_ID_WWN, ATA_ID_WWN_LEN);
num += ATA_ID_WWN_LEN;
}
+   else {


   CodingStyle, should be:

} else {

[...]

MBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: how to test pscsi with vhost

2016-07-07 Thread Zhangfei Gao
Hi, Lingshan

On Mon, Jul 4, 2016 at 10:02 PM, Zhu Lingshan  wrote:
> Hi Zhangfei,
>
> I am also interested in pscsi, you can try kvm, seems you can create a
> virtualized pscsi device in kvm / virt-manager. I haven't tried that yet,
> hope this can help.
>


Somehow I failed to setup virt-manager

error like:
Failed to connect to Mir: Failed to connect to server socket: No such
file or directory
Unable to init server: Could not connect: Connection refused

(virt-manager:6661): Gtk-CRITICAL **: gtk_settings_get_for_screen:
assertion 'GDK_IS_SCREEN (screen)' failed


Still in check with qemu.
The issue is scsi_probe_lun can not get correct lun.
Have hacked with scsi_execute_req INQUIRY sereral times but same result.

sc->result=0x800
sc->sense_buffer[0]=0x0

after targetcli setup, we can get:
[  104.238740] PSCSI[0]: Referencing SCSI Host ID: 1
[  104.243471] PSCSI[0]: Referencing SCSI Channel ID: 0
[  104.248463] PSCSI[0]: Referencing SCSI Target ID: 1
[  104.253368] PSCSI[0]: Referencing SCSI LUN ID: 0

pscsi_show_configfs_dev_params SCSI Device Bus Location: Channel ID: 0
Target ID: 1 LUN: 0 Host ID: 1
Vendor: HGST Model: HUSMM1640ASS204  Rev: C2D0

Suppose drivers/scsi/virtio_scsi.c &
drivers/target/target_core_pscsi.c have been verified.
Suspect sas driver itself need do something to support pscsi.

Still not find the root cause.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[patch] fnic: pci_dma_mapping_error() doesn't return an error code

2016-07-07 Thread Dan Carpenter
pci_dma_mapping_error() returns true on error and false on success.

Fixes: fd6ddfa4c1dd ('fnic: check pci_map_single() return value')
Signed-off-by: Dan Carpenter 

diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index 67669a9..f3a3331 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -954,8 +954,8 @@ int fnic_alloc_rq_frame(struct vnic_rq *rq)
skb_put(skb, len);
pa = pci_map_single(fnic->pdev, skb->data, len, PCI_DMA_FROMDEVICE);
 
-   r = pci_dma_mapping_error(fnic->pdev, pa);
-   if (r) {
+   if (pci_dma_mapping_error(fnic->pdev, pa)) {
+   r = -ENOMEM;
printk(KERN_ERR "PCI mapping failed with error %d\n", r);
goto free_skb;
}
@@ -1093,8 +1093,8 @@ static int fnic_send_frame(struct fnic *fnic, struct 
fc_frame *fp)
 
pa = pci_map_single(fnic->pdev, eth_hdr, tot_len, PCI_DMA_TODEVICE);
 
-   ret = pci_dma_mapping_error(fnic->pdev, pa);
-   if (ret) {
+   if (pci_dma_mapping_error(fnic->pdev, pa)) {
+   ret = -ENOMEM;
printk(KERN_ERR "DMA map failed with error %d\n", ret);
goto free_skb_on_err;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] xen-scsiback: prefer xenbus_write() over xenbus_printf() where possible

2016-07-07 Thread Jan Beulich
... as being the simpler variant.

Signed-off-by: Jan Beulich 
---
 drivers/xen/xen-scsiback.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- 4.7-rc6-prefer-xenbus_write.orig/drivers/xen/xen-scsiback.c
+++ 4.7-rc6-prefer-xenbus_write/drivers/xen/xen-scsiback.c
@@ -1260,8 +1260,8 @@ static int scsiback_probe(struct xenbus_
INIT_LIST_HEAD(>v2p_entry_lists);
spin_lock_init(>v2p_lock);
 
-   err = xenbus_printf(XBT_NIL, dev->nodename, "feature-sg-grant", "%u",
-   SG_ALL);
+   err = xenbus_write(XBT_NIL, dev->nodename, "feature-sg-grant",
+  __stringify(SG_ALL));
if (err)
xenbus_dev_error(dev, err, "writing feature-sg-grant");
 



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] xen-scsifront: correct return value checks on xenbus_scanf()

2016-07-07 Thread Jan Beulich
Only a positive return value indicates success.

Signed-off-by: Jan Beulich 
---
 drivers/scsi/xen-scsifront.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- 4.7-rc6-xenbus_scanf.orig/drivers/scsi/xen-scsifront.c
+++ 4.7-rc6-xenbus_scanf/drivers/scsi/xen-scsifront.c
@@ -1000,14 +1000,14 @@ static void scsifront_do_lun_hotplug(str
snprintf(str, sizeof(str), "vscsi-devs/%s/state", dir[i]);
err = xenbus_scanf(XBT_NIL, dev->otherend, str, "%u",
   _state);
-   if (XENBUS_EXIST_ERR(err))
+   if (err <= 0)
continue;
 
/* virtual SCSI device */
snprintf(str, sizeof(str), "vscsi-devs/%s/v-dev", dir[i]);
err = xenbus_scanf(XBT_NIL, dev->otherend, str,
   "%u:%u:%u:%u", , , , );
-   if (XENBUS_EXIST_ERR(err))
+   if (err != 4)
continue;
 
/*



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] xen-scsiback: correct return value checks on xenbus_scanf()

2016-07-07 Thread Jan Beulich
Only a positive return value indicates success.

Signed-off-by: Jan Beulich 
---
 drivers/xen/xen-scsiback.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- 4.7-rc6-xenbus_scanf.orig/drivers/xen/xen-scsiback.c
+++ 4.7-rc6-xenbus_scanf/drivers/xen/xen-scsiback.c
@@ -1071,7 +1071,7 @@ static void scsiback_do_1lun_hotplug(str
/* read status */
snprintf(state, sizeof(state), "vscsi-devs/%s/state", ent);
err = xenbus_scanf(XBT_NIL, dev->nodename, state, "%u", _state);
-   if (XENBUS_EXIST_ERR(err))
+   if (err <= 0)
return;
 
/* physical SCSI device */
@@ -1089,7 +1089,7 @@ static void scsiback_do_1lun_hotplug(str
snprintf(str, sizeof(str), "vscsi-devs/%s/v-dev", ent);
err = xenbus_scanf(XBT_NIL, dev->nodename, str, "%u:%u:%u:%u",
   , , , );
-   if (XENBUS_EXIST_ERR(err)) {
+   if (err != 4) {
xenbus_printf(XBT_NIL, dev->nodename, state,
  "%d", XenbusStateClosed);
return;



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v3 13/13] cxgbit: add files for cxgbit.ko

2016-07-07 Thread Or Gerlitz
On Wed, Jul 6, 2016 at 8:17 PM, Varun Prakash  wrote:
> On Wed, Jul 06, 2016 at 12:24:43AM +0300, Or Gerlitz wrote:
>> On Thu, May 26, 2016 at 9:58 PM, Varun Prakash  wrote:
>>
>> > Hi Or, Nicholas and Steve
>> > Thank you for the feedback and apologies for the delay in my response.
>>
>> > I agree that we can refactor initiator, target and iwarp drivers to
>> > reduce code duplication as Steve has mentioned. We will work on this for
>> > 4.8 merge window.
>>
>> Nic, Varun, was anything done on this matter? we're on rc6
>
> Yes, I am working on this, I will post first series
> within two days, it will add common library module libcxgb.ko.

thanks for the update, the plan was to get that info 4.8-rc1,
and as we're after rc6 there's not much time left for submitting code
for 4.8, so doing that sooner rather than later would be good here.

> In first series libcxgb.ko will have common
> iSCSI DDP Page Pod Manager that will be shared
> by three Chelsio iSCSI drivers
> cxgb3i, cxgb4i, cxgbit.

cool

> In subsequent series I will add common connection
> management and other hardware specific common code
> in this module.

any chance to get that ready for 4.8 too?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html