Re: [patch 21/21] Add ioctl support for EMC Symmetrix Subsystem Control I/O

Previous thread: [patch 08/21] more bus_id -> dev_name conversions by Martin Schwidefsky on Wednesday, October 1, 2008 - 1:33 am. (1 message)

Next thread: [patch 07/21] bus_id -> dev_set_name() changes by Martin Schwidefsky on Wednesday, October 1, 2008 - 1:33 am. (1 message)
From: Martin Schwidefsky
Date: Wednesday, October 1, 2008 - 1:33 am

From: Nigel Hislop <hislop_nigel@emc.com>

EMC Symmetrix Subsystem Control I/O through CKD dasd requires a
specific parameter list sent to the array via a Perform Subsystem
Function CCW. The Symmetrix response is retrieved from the array
via a Read Subsystem Data CCW.
    
Signed-off-by: Nigel Hislop <hislop_nigel@emc.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/s390/include/asm/dasd.h   |   13 +++++
 drivers/s390/block/dasd_eckd.c |   98 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

Index: quilt-2.6/arch/s390/include/asm/dasd.h
===================================================================
--- quilt-2.6.orig/arch/s390/include/asm/dasd.h
+++ quilt-2.6/arch/s390/include/asm/dasd.h
@@ -3,6 +3,8 @@
  * Author(s)......: Holger Smolinski <Holger.Smolinski@de.ibm.com>
  * Bugreports.to..: <Linux390@de.ibm.com>
  * (C) IBM Corporation, IBM Deutschland Entwicklung GmbH, 1999,2000
+ * EMC Symmetrix ioctl Copyright EMC Corporation, 2008
+ * Author.........: Nigel Hislop <hislop_nigel@emc.com>
  *
  * This file is the interface of the DASD device driver, which is exported to user space
  * any future changes wrt the API will result in a change of the APIVERSION reported
@@ -202,6 +204,16 @@ typedef struct attrib_data_t {
 #define DASD_SEQ_PRESTAGE  0x4
 #define DASD_REC_ACCESS    0x5
 
+/*
+ * Perform EMC Symmetrix I/O
+ */
+typedef struct dasd_symmio_parms {
+	unsigned char reserved[8];	/* compat with older releases */
+	unsigned long long psf_data;	/* char * cast to u64 */
+	unsigned long long rssd_result; /* char * cast to u64 */
+	int psf_data_len;
+	int rssd_result_len;
+} __attribute__ ((packed)) dasd_symmio_parms_t;
 
 /********************************************************************************
  * SECTION: Definition of IOCTLs
@@ -247,6 +259,7 @@ typedef struct attrib_data_t {
 /* Set Attributes (cache operations) */
 #define BIODASDSATTR   ...
From: Christoph Hellwig
Date: Wednesday, October 1, 2008 - 4:03 am

That style is quite odd,  what about the more normal

	rc = -EFAULT;
	if (copy_from_user(&usrparm, argp, sizeof(usrparm)))
		goto out;


just check each on individually and do individual unwinding.  Makes the


And here again.

--

From: Martin Schwidefsky
Date: Wednesday, October 1, 2008 - 4:17 am

Well, matter of taste I would say. Two kzallocs, one check for the
results and just two kfrees if one of the kzallocs failed. I find this
version to be a little more compact and therefore easier to read. If you


Ok.

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.


--

Previous thread: [patch 08/21] more bus_id -> dev_name conversions by Martin Schwidefsky on Wednesday, October 1, 2008 - 1:33 am. (1 message)

Next thread: [patch 07/21] bus_id -> dev_set_name() changes by Martin Schwidefsky on Wednesday, October 1, 2008 - 1:33 am. (1 message)