From patchwork Sun Jan 24 21:05:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Kai_M=C3=A4kisara_=28Kolumbus=29?= X-Patchwork-Id: 8101231 Return-Path: X-Original-To: patchwork-linux-scsi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id B79FF9F859 for ; Sun, 24 Jan 2016 21:05:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8C4DD20373 for ; Sun, 24 Jan 2016 21:05:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 91DA120121 for ; Sun, 24 Jan 2016 21:05:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752113AbcAXVFe (ORCPT ); Sun, 24 Jan 2016 16:05:34 -0500 Received: from vs16.mail.saunalahti.fi ([62.142.117.197]:41830 "EHLO vs16.mail.saunalahti.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751996AbcAXVFc (ORCPT ); Sun, 24 Jan 2016 16:05:32 -0500 Received: from vams (localhost [127.0.0.1]) by vs16.mail.saunalahti.fi (Postfix) with SMTP id CCBDB200CC; Sun, 24 Jan 2016 23:05:29 +0200 (EET) Received: from gw03.mail.saunalahti.fi (gw03.mail.saunalahti.fi [195.197.172.111]) by vs16.mail.saunalahti.fi (Postfix) with ESMTP id A5D8E200CC; Sun, 24 Jan 2016 23:05:29 +0200 (EET) Received: from kai.makisara.private (a88-115-178-128.elisa-laajakaista.fi [88.115.178.128]) (using TLSv1 with cipher ECDHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by gw03.mail.saunalahti.fi (Postfix) with ESMTPSA id 42FCC20006; Sun, 24 Jan 2016 23:05:23 +0200 (EET) Date: Sun, 24 Jan 2016 23:05:17 +0200 (EET) From: Kai Makisara To: "Seymour, Shane M" cc: Laurence Oberman , Emmanuel Florac , Laurence Oberman , "linux-scsi@vger.kernel.org" Subject: RE: What partition should the MTMKPART argument specify? Was: Re: st driver doesn't seem to grok LTO partitioning In-Reply-To: Message-ID: References: <20151218170644.24167419@harpe.intellique.com> <20160104124626.3112e8c0@harpe.intellique.com> <688812570.3359420.1452030904683.JavaMail.zimbra@redhat.com> <20160106161049.34f29973@harpe.intellique.com> <1993563395.3556606.1452093814452.JavaMail.zimbra@redhat.com> <20160106170720.61db9e86@harpe.intellique.com> <1371374871.6203837.1452802373256.JavaMail.zimbra@redhat.com> <0840949A-970D-43FB-A691-E2F8AC2A0804@kolumbus.fi> User-Agent: Alpine 2.11 (LSU 23 2013-08-11) MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_TVD_MIME_EPI, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Friday 2016-01-22 04:10, Seymour, Shane M wrote: >> -----Original Message----- >> From: linux-scsi-owner@vger.kernel.org [mailto:linux-scsi- >> owner@vger.kernel.org] On Behalf Of "Kai Mäkisara (Kolumbus)" >> Sent: Friday, January 22, 2016 7:59 AM >> To: Seymour, Shane M >> Cc: Laurence Oberman; Emmanuel Florac; Laurence Oberman; linux- >> scsi@vger.kernel.org >> Subject: What partition should the MTMKPART argument specify? Was: Re: >> st driver doesn't seem to grok LTO partitioning >> ... >> >> There seem to be lot of arguments supporting both possible choices. Should >> we use the existing definition (1) or change it for the drives supporting SCSI >> level >= 3 (or supporting FORMAT MEDIUM)? The definition can’t be >> changed later. This is why we should make a good decision. >> >> Opinions? > >How about using the fact the size is signed to indicate one slightly different >thing? I'm not sure if you'd call this using or abusing the fact that it's signed. > >Make the default behavior for a positive size the same as the current >behavior for SCSI-2 (size applies to partition 1). If the size is negative then >the absolute value of the size applies to partition 0. That provides some >flexibility in choosing which partition the size applies to if it worked that >way for all devices. > >With that you also get consistent behavior between tape drives without >having to know if the size will apply to partition 0 or partition 1 based on >the tape technology and you get the benefit of being able to set an >explicit size for partition 0 or partition 1. > I like this suggestion, because of the reasons you point out. I think this is using the fact that the argument is signed. >You could overload the value of 0 as well to use FDP to choose the sizes >for the partitions (assuming 0 doesn't already have a side effect in >the code). Then you get whatever the tape drive wants to do. > The value zero is used to specify only one partition. The previous patches overloaded the size 1. However, I think supporting FDP is useless nowadays: the drives that support FDP=1 seem to end up with the same partitioning (two wraps to partition 1) with any small number as argument. Below is a test patch that implements the current behaviour with non-negative argument (but works with LTOs etc.) and makes partition zero size absolute value of argument (MB) if argument is negative. If you want to test the patch, note that the current mt-st does not accept negative numbers. A minimal patch is needed. Thanks, Kai -------------------------------8<---------------------------------------- --- ref/drivers/scsi/st.c 2015-12-21 18:54:05.068882001 +0200 +++ new/drivers/scsi/st.c 2016-01-24 22:36:13.250928500 +0200 @@ -9,7 +9,7 @@ Steve Hirsch, Andreas Koppenh"ofer, Michael Leodolter, Eyal Lebedinsky, Michael Schaefer, J"org Weule, and Eric Youngdale. - Copyright 1992 - 2010 Kai Makisara + Copyright 1992 - 2016 Kai Makisara email Kai.Makisara@kolumbus.fi Some small formal changes - aeb, 950809 @@ -17,7 +17,7 @@ Last modified: 18-JAN-1998 Richard Gooch Devfs support */ -static const char *verstr = "20101219"; +static const char *verstr = "20160124"; #include @@ -3296,7 +3296,10 @@ #define PP_OFF_RESERVED 7 #define PP_BIT_IDP 0x20 +#define PP_BIT_FDP 0x80 #define PP_MSK_PSUM_MB 0x10 +#define PP_MSK_PSUM_UNITS 0x18 +#define PP_MSK_POFM 0x04 /* Get the number of partitions on the tape. As a side effect reads the mode page into the tape buffer. */ @@ -3322,6 +3325,29 @@ } +static int format_medium(struct scsi_tape *STp, int format) +{ + int result = 0; + int timeout = STp->long_timeout; + unsigned char scmd[MAX_COMMAND_SIZE]; + struct st_request *SRpnt; + + memset(scmd, 0, MAX_COMMAND_SIZE); + scmd[0] = FORMAT_UNIT; + scmd[2] = format; + if (STp->immediate) { + scmd[1] |= 1; /* Don't wait for completion */ + timeout = STp->device->request_queue->rq_timeout; + } + DEBC_printk(STp, "Sending FORMAT MEDIUM\n"); + SRpnt = st_do_scsi(NULL, STp, scmd, 0, DMA_NONE, + timeout, MAX_RETRIES, 1); + if (!SRpnt) + result = STp->buffer->syscall_result; + return result; +} + + /* Partition the tape into two partitions if size > 0 or one partition if size == 0. @@ -3340,11 +3366,16 @@ and 10 when 1 partition is defined (information from Eric Lee Green). This is is acceptable also to some other old drives and enforced if the first partition size field is used for the first additional partition size. + + For drives that advertize SCSI-3 or newer, use the SSC-3 methods. */ static int partition_tape(struct scsi_tape *STp, int size) { int result; + int target_partition; + bool scsi3 = STp->device->scsi_level >= SCSI_3, needs_format = false; int pgo, psd_cnt, psdo; + int psum = PP_MSK_PSUM_MB, units = 0; unsigned char *bp; result = read_mode_page(STp, PART_PAGE, 0); @@ -3352,16 +3383,72 @@ DEBC_printk(STp, "Can't read partition mode page.\n"); return result; } + target_partition = 1; + if (size < 0) { + target_partition = 0; + size = -size; + } + /* The mode page is in the buffer. Let's modify it and write it. */ bp = (STp->buffer)->b_data; pgo = MODE_HEADER_LENGTH + bp[MH_OFF_BDESCS_LENGTH]; DEBC_printk(STp, "Partition page length is %d bytes.\n", bp[pgo + MP_OFF_PAGE_LENGTH] + 2); + DEBC_printk(STp, "PP: max %u, add %u, xdp %u, psum %02x, pofmetc %u, " + "rec %02x, units %02x, sizes: %u %u\n", + bp[pgo + PP_OFF_MAX_ADD_PARTS], + bp[pgo + PP_OFF_NBR_ADD_PARTS], + (bp[pgo + PP_OFF_FLAGS] & 0xe0) >> 5, + (bp[pgo + PP_OFF_FLAGS] & 0x18) >> 3, + bp[pgo + PP_OFF_FLAGS] & 0x07, + bp[pgo + 5], + bp[pgo + PP_OFF_PART_UNITS], + bp[pgo + 8] * 256 + bp[pgo + 9], + bp[pgo + 10] * 256 + bp[pgo + 11]); + DEBC_printk(STp, "MP: %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", + bp[pgo], bp[pgo+1], bp[pgo+2], bp[pgo+3], bp[pgo+4], bp[pgo+5], + bp[pgo+6], bp[pgo+7], bp[pgo+8], bp[pgo+9], bp[pgo+10], bp[pgo+11]); + + if (scsi3) { + needs_format = (bp[pgo + PP_OFF_FLAGS] & PP_MSK_POFM) != 0; + if (needs_format && size == 0) { + /* No need to write the mode page when clearing partitioning */ + result = format_medium(STp, 0); + goto out; + } + + psd_cnt = size > 0 ? 2 : 1; + if ((bp[pgo + PP_OFF_FLAGS] & PP_MSK_PSUM_UNITS) == PP_MSK_PSUM_UNITS) { + /* Use units scaling for large partitions if the device suggests + it and no precision lost. Required for IBM TS1140/50 drives + that don't support MB units. */ + if (size >= 1000 && (size % 1000) == 0) { + size /= 1000; + psum = PP_MSK_PSUM_UNITS; + units = 9; /* GB */ + } + } + /* Try it anyway if too large to specify in MB */ + if (psum == PP_MSK_PSUM_MB && size >= 65534) { + size /= 1000; + psum = PP_MSK_PSUM_UNITS; + units = 9; /* GB */ + } + } else + psd_cnt = (bp[pgo + MP_OFF_PAGE_LENGTH] + 2 - + PART_PAGE_FIXED_LENGTH) / 2; + + if (size >= 65535 || /* Does not fit into two bytes */ + (target_partition == 0 && psd_cnt < 2)) { + result = -EINVAL; + goto out; + } - psd_cnt = (bp[pgo + MP_OFF_PAGE_LENGTH] + 2 - PART_PAGE_FIXED_LENGTH) / 2; psdo = pgo + PART_PAGE_FIXED_LENGTH; - if (psd_cnt > bp[pgo + PP_OFF_MAX_ADD_PARTS]) { - bp[psdo] = bp[psdo + 1] = 0xff; /* Rest of the tape */ + /* The second condition if for HP DDS which use only one partition size + descriptor */ + if (target_partition > 0 && psd_cnt > bp[pgo + PP_OFF_MAX_ADD_PARTS]) { + bp[psdo] = bp[psdo + 1] = 0xff; /* Rest of the tape to partition 0 */ psdo += 2; } memset(bp + psdo, 0, bp[pgo + PP_OFF_NBR_ADD_PARTS] * 2); @@ -3370,7 +3457,7 @@ psd_cnt, bp[pgo + PP_OFF_MAX_ADD_PARTS], bp[pgo + PP_OFF_NBR_ADD_PARTS]); - if (size <= 0) { + if (size == 0) { bp[pgo + PP_OFF_NBR_ADD_PARTS] = 0; if (psd_cnt <= bp[pgo + PP_OFF_MAX_ADD_PARTS]) bp[pgo + MP_OFF_PAGE_LENGTH] = 6; @@ -3378,22 +3465,54 @@ } else { bp[psdo] = (size >> 8) & 0xff; bp[psdo + 1] = size & 0xff; + if (target_partition == 0) + bp[psdo + 2] = bp[psdo + 3] = 0xff; bp[pgo + 3] = 1; if (bp[pgo + MP_OFF_PAGE_LENGTH] < 8) bp[pgo + MP_OFF_PAGE_LENGTH] = 8; DEBC_printk(STp, "Formatting tape with two partitions " - "(1 = %d MB).\n", size); + "(1 = %d MB).\n", + units > 0 ? size * 1000 : size); } bp[pgo + PP_OFF_PART_UNITS] = 0; bp[pgo + PP_OFF_RESERVED] = 0; - bp[pgo + PP_OFF_FLAGS] = PP_BIT_IDP | PP_MSK_PSUM_MB; + if (size != 1 || units != 0) { + bp[pgo + PP_OFF_FLAGS] = PP_BIT_IDP | psum | + (bp[pgo + PP_OFF_FLAGS] & 0x07); + bp[pgo + PP_OFF_PART_UNITS] = units; + } else + bp[pgo + PP_OFF_FLAGS] = PP_BIT_FDP | + (bp[pgo + PP_OFF_FLAGS] & 0x1f); + bp[pgo + MP_OFF_PAGE_LENGTH] = 6 + psd_cnt * 2; + + DEBC_printk(STp, "Sent partition page length is %d bytes. needs_format: %d\n", + bp[pgo + MP_OFF_PAGE_LENGTH] + 2, needs_format); + DEBC_printk(STp, "PP: max %u, add %u, xdp %u, psum %02x, pofmetc %u, " + "rec %02x, units %02x, sizes: %u %u\n", + bp[pgo + PP_OFF_MAX_ADD_PARTS], + bp[pgo + PP_OFF_NBR_ADD_PARTS], + (bp[pgo + PP_OFF_FLAGS] & 0xe0) >> 5, + (bp[pgo + PP_OFF_FLAGS] & 0x18) >> 3, + bp[pgo + PP_OFF_FLAGS] & 0x07, + bp[pgo + 5], + bp[pgo + PP_OFF_PART_UNITS], + bp[pgo + 8] * 256 + bp[pgo + 9], + bp[pgo + 10] * 256 + bp[pgo + 11]); + DEBC_printk(STp, "MP: %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x %02x\n", + bp[pgo], bp[pgo+1], bp[pgo+2], bp[pgo+3], bp[pgo+4], bp[pgo+5], + bp[pgo+6], bp[pgo+7], bp[pgo+8], bp[pgo+9], bp[pgo+10], bp[pgo+11]); result = write_mode_page(STp, PART_PAGE, 1); + + if (!result && needs_format) + result = format_medium(STp, 1); + if (result) { st_printk(KERN_INFO, STp, "Partitioning of tape failed.\n"); result = (-EIO); } +out: return result; } @@ -3570,7 +3689,7 @@ retval = (-EINVAL); goto out; } - if ((i = st_int_ioctl(STp, MTREW, 0)) < 0 || + if ((i = do_load_unload(STp, file, 1)) < 0 || (i = partition_tape(STp, mtc.mt_count)) < 0) { retval = i; goto out; @@ -3581,7 +3700,7 @@ STp->ps[i].last_block_valid = 0; } STp->partition = STp->new_partition = 0; - STp->nbr_partitions = 1; /* Bad guess ?-) */ + STp->nbr_partitions = mtc.mt_count > 0 ? 2 : 1; STps->drv_block = STps->drv_file = 0; retval = 0; goto out;