Date: Thu, 19 Jun 2014 09:57:25 -0700 From: Adrian Chadd <adrian@freebsd.org> To: Alexander Motin <mav@freebsd.org> Cc: "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "src-committers@freebsd.org" <src-committers@freebsd.org> Subject: Re: svn commit: r267639 - head/sys/cam/ctl Message-ID: <CAJ-VmokGiADCorNSz87c%2Bw9qr7Z_hZTLpLAMU5NKxHcqeu0SXg@mail.gmail.com> In-Reply-To: <201406190946.s5J9khBe031610@svn.freebsd.org> References: <201406190946.s5J9khBe031610@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
FreeBSD_version? -a On 19 June 2014 02:46, Alexander Motin <mav@freebsd.org> wrote: > Author: mav > Date: Thu Jun 19 09:46:43 2014 > New Revision: 267639 > URL: http://svnweb.freebsd.org/changeset/base/267639 > > Log: > Increase CTL_DEVID_LEN from 16 to 64 bytes. > > SPC-4 recommends T10 vendor ID based LUN ID was created by concatenating > product name and serial number (and istgt follows that). But product name > is 16 bytes long by itself, so 16 bytes total length is clearly not enough > to fit both. > > To keep compatibility with existing configurations, pad short device IDs > to old length of 16, same as before. > > This change probably breaks CTL user-level ABI, so control tools should > be rebuilt after this change. > > MFC after: 2 weeks > > Modified: > head/sys/cam/ctl/ctl.c > head/sys/cam/ctl/ctl.h > head/sys/cam/ctl/ctl_frontend_iscsi.c > > Modified: head/sys/cam/ctl/ctl.c > ============================================================================== > --- head/sys/cam/ctl/ctl.c Thu Jun 19 09:08:20 2014 (r267638) > +++ head/sys/cam/ctl/ctl.c Thu Jun 19 09:46:43 2014 (r267639) > @@ -91,12 +91,6 @@ struct ctl_softc *control_softc = NULL; > #define CTL_DONE_THREAD > > /* > - * Use the serial number and device ID provided by the backend, rather than > - * making up our own. > - */ > -#define CTL_USE_BACKEND_SN > - > -/* > * Size and alignment macros needed for Copan-specific HA hardware. These > * can go away when the HA code is re-written, and uses busdma for any > * hardware. > @@ -9477,9 +9471,6 @@ ctl_inquiry_evpd_serial(struct ctl_scsii > { > struct scsi_vpd_unit_serial_number *sn_ptr; > struct ctl_lun *lun; > -#ifndef CTL_USE_BACKEND_SN > - char tmpstr[32]; > -#endif > > lun = (struct ctl_lun *)ctsio->io_hdr.ctl_private[CTL_PRIV_LUN].ptr; > > @@ -9513,7 +9504,6 @@ ctl_inquiry_evpd_serial(struct ctl_scsii > > sn_ptr->page_code = SVPD_UNIT_SERIAL_NUMBER; > sn_ptr->length = ctl_min(sizeof(*sn_ptr) - 4, CTL_SN_LEN); > -#ifdef CTL_USE_BACKEND_SN > /* > * If we don't have a LUN, we just leave the serial number as > * all spaces. > @@ -9523,15 +9513,6 @@ ctl_inquiry_evpd_serial(struct ctl_scsii > strncpy((char *)sn_ptr->serial_num, > (char *)lun->be_lun->serial_num, CTL_SN_LEN); > } > -#else > - /* > - * Note that we're using a non-unique serial number here, > - */ > - snprintf(tmpstr, sizeof(tmpstr), "MYSERIALNUMIS000"); > - memset(sn_ptr->serial_num, 0x20, sizeof(sn_ptr->serial_num)); > - strncpy(sn_ptr->serial_num, tmpstr, ctl_min(CTL_SN_LEN, > - ctl_min(sizeof(tmpstr), sizeof(*sn_ptr) - 4))); > -#endif > ctsio->scsi_status = SCSI_STATUS_OK; > > ctsio->be_move_done = ctl_config_move_done; > @@ -9552,10 +9533,7 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > struct ctl_lun *lun; > struct ctl_frontend *fe; > char *val; > -#ifndef CTL_USE_BACKEND_SN > - char tmpstr[32]; > -#endif /* CTL_USE_BACKEND_SN */ > - int devid_len; > + int data_len, devid_len; > > ctl_softc = control_softc; > > @@ -9568,23 +9546,30 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > > lun = (struct ctl_lun *)ctsio->io_hdr.ctl_private[CTL_PRIV_LUN].ptr; > > - devid_len = sizeof(struct scsi_vpd_device_id) + > + if (lun == NULL) { > + devid_len = CTL_DEVID_MIN_LEN; > + } else { > + devid_len = max(CTL_DEVID_MIN_LEN, > + strnlen(lun->be_lun->device_id, CTL_DEVID_LEN)); > + } > + > + data_len = sizeof(struct scsi_vpd_device_id) + > sizeof(struct scsi_vpd_id_descriptor) + > - sizeof(struct scsi_vpd_id_t10) + CTL_DEVID_LEN + > + sizeof(struct scsi_vpd_id_t10) + devid_len + > sizeof(struct scsi_vpd_id_descriptor) + CTL_WWPN_LEN + > sizeof(struct scsi_vpd_id_descriptor) + > sizeof(struct scsi_vpd_id_rel_trgt_port_id) + > sizeof(struct scsi_vpd_id_descriptor) + > sizeof(struct scsi_vpd_id_trgt_port_grp_id); > > - ctsio->kern_data_ptr = malloc(devid_len, M_CTL, M_WAITOK | M_ZERO); > + ctsio->kern_data_ptr = malloc(data_len, M_CTL, M_WAITOK | M_ZERO); > devid_ptr = (struct scsi_vpd_device_id *)ctsio->kern_data_ptr; > ctsio->kern_sg_entries = 0; > > - if (devid_len < alloc_len) { > - ctsio->residual = alloc_len - devid_len; > - ctsio->kern_data_len = devid_len; > - ctsio->kern_total_len = devid_len; > + if (data_len < alloc_len) { > + ctsio->residual = alloc_len - data_len; > + ctsio->kern_data_len = data_len; > + ctsio->kern_total_len = data_len; > } else { > ctsio->residual = 0; > ctsio->kern_data_len = alloc_len; > @@ -9597,7 +9582,7 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > desc = (struct scsi_vpd_id_descriptor *)devid_ptr->desc_list; > t10id = (struct scsi_vpd_id_t10 *)&desc->identifier[0]; > desc1 = (struct scsi_vpd_id_descriptor *)(&desc->identifier[0] + > - sizeof(struct scsi_vpd_id_t10) + CTL_DEVID_LEN); > + sizeof(struct scsi_vpd_id_t10) + devid_len); > desc2 = (struct scsi_vpd_id_descriptor *)(&desc1->identifier[0] + > CTL_WWPN_LEN); > desc3 = (struct scsi_vpd_id_descriptor *)(&desc2->identifier[0] + > @@ -9615,7 +9600,7 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > > devid_ptr->page_code = SVPD_DEVICE_ID; > > - scsi_ulto2b(devid_len - 4, devid_ptr->length); > + scsi_ulto2b(data_len - 4, devid_ptr->length); > > mtx_lock(&ctl_softc->ctl_lock); > > @@ -9644,7 +9629,7 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > * per-LUN identifier. > */ > desc->id_type = SVPD_ID_PIV | SVPD_ID_ASSOC_LUN | SVPD_ID_TYPE_T10; > - desc->length = sizeof(*t10id) + CTL_DEVID_LEN; > + desc->length = sizeof(*t10id) + devid_len; > if (lun == NULL || (val = ctl_get_opt(lun->be_lun, "vendor")) == NULL) { > strncpy((char *)t10id->vendor, CTL_VENDOR, sizeof(t10id->vendor)); > } else { > @@ -9701,7 +9686,6 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > else > desc3->identifier[3] = 2; > > -#ifdef CTL_USE_BACKEND_SN > /* > * If we've actually got a backend, copy the device id from the > * per-LUN data. Otherwise, set it to all spaces. > @@ -9711,19 +9695,13 @@ ctl_inquiry_evpd_devid(struct ctl_scsiio > * Copy the backend's LUN ID. > */ > strncpy((char *)t10id->vendor_spec_id, > - (char *)lun->be_lun->device_id, CTL_DEVID_LEN); > + (char *)lun->be_lun->device_id, devid_len); > } else { > /* > * No backend, set this to spaces. > */ > - memset(t10id->vendor_spec_id, 0x20, CTL_DEVID_LEN); > + memset(t10id->vendor_spec_id, 0x20, devid_len); > } > -#else > - snprintf(tmpstr, sizeof(tmpstr), "MYDEVICEIDIS%4d", > - (lun != NULL) ? (int)lun->lun : 0); > - strncpy(t10id->vendor_spec_id, tmpstr, ctl_min(CTL_DEVID_LEN, > - sizeof(tmpstr))); > -#endif > > ctsio->scsi_status = SCSI_STATUS_OK; > > > Modified: head/sys/cam/ctl/ctl.h > ============================================================================== > --- head/sys/cam/ctl/ctl.h Thu Jun 19 09:08:20 2014 (r267638) > +++ head/sys/cam/ctl/ctl.h Thu Jun 19 09:46:43 2014 (r267639) > @@ -96,7 +96,8 @@ union ctl_modepage_info { > /* > * Device ID length, for VPD page 0x83. > */ > -#define CTL_DEVID_LEN 16 > +#define CTL_DEVID_LEN 64 > +#define CTL_DEVID_MIN_LEN 16 > /* > * WWPN length, for VPD page 0x83. > */ > > Modified: head/sys/cam/ctl/ctl_frontend_iscsi.c > ============================================================================== > --- head/sys/cam/ctl/ctl_frontend_iscsi.c Thu Jun 19 09:08:20 2014 (r267638) > +++ head/sys/cam/ctl/ctl_frontend_iscsi.c Thu Jun 19 09:46:43 2014 (r267639) > @@ -2060,7 +2060,7 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > const struct icl_pdu *request; > int i, ret; > char *val; > - size_t devid_len, wwnn_len, wwpn_len, lun_name_len; > + size_t data_len, devid_len, wwnn_len, wwpn_len, lun_name_len; > > lun = (struct ctl_lun *)ctsio->io_hdr.ctl_private[CTL_PRIV_LUN].ptr; > request = ctsio->io_hdr.ctl_private[CTL_PRIV_FRONTEND].ptr; > @@ -2078,8 +2078,11 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > wwnn_len += (4 - (wwnn_len % 4)); > > if (lun == NULL) { > + devid_len = CTL_DEVID_MIN_LEN; > lun_name_len = 0; > } else { > + devid_len = max(CTL_DEVID_MIN_LEN, > + strnlen(lun->be_lun->device_id, CTL_DEVID_LEN)); > lun_name_len = strlen(cs->cs_target->ct_name); > lun_name_len += strlen(",lun,XXXXXXXX"); > lun_name_len += 1; /* '\0' */ > @@ -2087,9 +2090,9 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > lun_name_len += (4 - (lun_name_len % 4)); > } > > - devid_len = sizeof(struct scsi_vpd_device_id) + > + data_len = sizeof(struct scsi_vpd_device_id) + > sizeof(struct scsi_vpd_id_descriptor) + > - sizeof(struct scsi_vpd_id_t10) + CTL_DEVID_LEN + > + sizeof(struct scsi_vpd_id_t10) + devid_len + > sizeof(struct scsi_vpd_id_descriptor) + lun_name_len + > sizeof(struct scsi_vpd_id_descriptor) + wwnn_len + > sizeof(struct scsi_vpd_id_descriptor) + wwpn_len + > @@ -2098,14 +2101,14 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > sizeof(struct scsi_vpd_id_descriptor) + > sizeof(struct scsi_vpd_id_trgt_port_grp_id); > > - ctsio->kern_data_ptr = malloc(devid_len, M_CTL, M_WAITOK | M_ZERO); > + ctsio->kern_data_ptr = malloc(data_len, M_CTL, M_WAITOK | M_ZERO); > devid_ptr = (struct scsi_vpd_device_id *)ctsio->kern_data_ptr; > ctsio->kern_sg_entries = 0; > > - if (devid_len < alloc_len) { > - ctsio->residual = alloc_len - devid_len; > - ctsio->kern_data_len = devid_len; > - ctsio->kern_total_len = devid_len; > + if (data_len < alloc_len) { > + ctsio->residual = alloc_len - data_len; > + ctsio->kern_data_len = data_len; > + ctsio->kern_total_len = data_len; > } else { > ctsio->residual = 0; > ctsio->kern_data_len = alloc_len; > @@ -2118,7 +2121,7 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > desc = (struct scsi_vpd_id_descriptor *)devid_ptr->desc_list; > t10id = (struct scsi_vpd_id_t10 *)&desc->identifier[0]; > desc1 = (struct scsi_vpd_id_descriptor *)(&desc->identifier[0] + > - sizeof(struct scsi_vpd_id_t10) + CTL_DEVID_LEN); > + sizeof(struct scsi_vpd_id_t10) + devid_len); > desc2 = (struct scsi_vpd_id_descriptor *)(&desc1->identifier[0] + > lun_name_len); > desc3 = (struct scsi_vpd_id_descriptor *)(&desc2->identifier[0] + > @@ -2136,7 +2139,7 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > > devid_ptr->page_code = SVPD_DEVICE_ID; > > - scsi_ulto2b(devid_len - 4, devid_ptr->length); > + scsi_ulto2b(data_len - 4, devid_ptr->length); > > /* > * We're using a LUN association here. i.e., this device ID is a > @@ -2144,7 +2147,7 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > */ > desc->proto_codeset = (SCSI_PROTO_ISCSI << 4) | SVPD_ID_CODESET_ASCII; > desc->id_type = SVPD_ID_PIV | SVPD_ID_ASSOC_LUN | SVPD_ID_TYPE_T10; > - desc->length = sizeof(*t10id) + CTL_DEVID_LEN; > + desc->length = sizeof(*t10id) + devid_len; > if (lun == NULL || (val = ctl_get_opt(lun->be_lun, "vendor")) == NULL) { > strncpy((char *)t10id->vendor, CTL_VENDOR, sizeof(t10id->vendor)); > } else { > @@ -2162,12 +2165,12 @@ cfiscsi_devid(struct ctl_scsiio *ctsio, > * Copy the backend's LUN ID. > */ > strncpy((char *)t10id->vendor_spec_id, > - (char *)lun->be_lun->device_id, CTL_DEVID_LEN); > + (char *)lun->be_lun->device_id, devid_len); > } else { > /* > * No backend, set this to spaces. > */ > - memset(t10id->vendor_spec_id, 0x20, CTL_DEVID_LEN); > + memset(t10id->vendor_spec_id, 0x20, devid_len); > } > > /* >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ-VmokGiADCorNSz87c%2Bw9qr7Z_hZTLpLAMU5NKxHcqeu0SXg>