I am volunteering to be sponsor of this case.

Larry

Juergen Keil wrote:
> OK, I've filed CR 6505720 "wrong default disk label on x86, for an audio cd",
> and need a sponsor for the fix.
>
> My suggested fix is at http://www.tools.de/solaris/opensolaris/sd_label
>
> Contributor agreement # : OS0003
>
>
> Larry, would you sponsor the fix for me?
>
>
>
>   
>> The changes look good to me. I can help to integrated if needed.
>>
>> Larry
>>
>>
>>     
>>> From: "J??rgen Keil" <jk at tools.de>
>>> To: ufs-discuss at opensolaris.org
>>> Cc:
>>> Subject: [ufs-discuss] Re: sd(7d) uses broken default disk labels on 
>>> x86, for an audio cd
>>> Date: Fri, 15 Dec 2006 11:54:12 +0100
>>>
>>> I wrote:
>>>
>>>       
>>>> First issue, where sd(7d) uses a bogus disk label for (multisession / 
>>>> mixed audio+data)
>>>> cds on x86.
>>>>         
>>> I think there are two issues here:
>>>
>>> 1. when sd_read_fdisk() cannot read sector 0, it returns an
>>>    SD_CMD_FAILURE error, but doesn't set "un->un_solaris_offset"
>>>    and "un->un_solaris_size".
>>>    And it doesn't clear the VTOC info.
>>>
>>>
>>>       
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/scsi/targets/
> sd.c#4915 
>   
>>>    4915     bufp = kmem_zalloc(blocksize, KM_SLEEP);
>>>    4916     rval = sd_send_scsi_READ(un, bufp, blocksize, 0, path_flag);
>>>    ....
>>>    4919     if (rval != 0) {
>>>    4920         SD_ERROR(SD_LOG_ATTACH_DETACH, un,
>>>    4921             "sd_read_fdisk: fdisk read err\n");
>>>    4922         kmem_free(bufp, blocksize);
>>>    4923         return (SD_CMD_FAILURE);
>>>    4924     }
>>>
>>>
>>> instead of returning immediatelly, shouldn't this jump to the "done" 
>>> label,
>>> just like the case when sector 0 is readable but doesn't contain a valid
>>> MBR signature?
>>>
>>>
>>>    4979     /*
>>>    4980      * Endian-independent signature check
>>>    4981      */
>>>    4982     if (((sigbuf[1] & 0xFF) != ((MBB_MAGIC >> 8) & 0xFF)) ||
>>>    4983         (sigbuf[0] != (MBB_MAGIC & 0xFF))) {
>>>    4984         SD_ERROR(SD_LOG_ATTACH_DETACH, un,
>>>    4985             "sd_read_fdisk: no fdisk\n");
>>>    4986         bzero(un->un_fmap, sizeof (struct fmap) * FD_NUMPART);
>>>    4987         rval = SD_CMD_SUCCESS;
>>>    4988         goto done;
>>>    4989     }
>>>
>>>
>>> 2. The second issue is that no default label is constructed by 
>>> sd_validate_geometry()
>>> when sd_read_fdisk() returns with SD_CMD_FAILURE, which happens when 
>>> sector 0
>>> wasn't readable.  The code returns with an ENOMEM error.
>>>
>>>
>>>       
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/scsi/targets/
> sd.c#4478 
>   
>>>    4478         /*
>>>    4479          * Note: This will set up un->un_solaris_size and
>>>    4480          * un->un_solaris_offset.
>>>    4481          */
>>>    4482         switch (sd_read_fdisk(un, capacity, lbasize, 
>>> path_flag)) {
>>>    4483         case SD_CMD_RESERVATION_CONFLICT:
>>>    4484             ASSERT(mutex_owned(SD_MUTEX(un)));
>>>    4485             return (EACCES);
>>>    4486         case SD_CMD_FAILURE:
>>>    4487             ASSERT(mutex_owned(SD_MUTEX(un)));
>>>    4488             return (ENOMEM);
>>>    4489         }
>>>
>>>
>>> On SPARC, sd_read_fdisk() cannot fail, so we keep going and a default
>>> label is constructed further down, at line 4595:
>>>
>>>
>>>       
> http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/uts/common/io/scsi/targets/
> sd.c#4595 
>   
>>>    4595     /*
>>>    4596      * If a valid label was not found, AND if no reservation 
>>> conflict
>>>    4597      * was detected, then go ahead and create a default label 
>>> (4069506).
>>>    4598      */
>>>    4599     if (un->un_f_default_vtoc_supported && (label_error != 
>>> EACCES)) {
>>>    4600         if (un->un_f_geometry_is_valid == FALSE) {
>>>    4601             sd_build_default_label(un);
>>>    4602         }
>>>    4603         label_error = 0;
>>>    4604     }
>>>
>>>
>>>
>>> I think the x86 platform should behave just like sparc in this case:  
>>> although
>>> the MBR sector 0 wasn't readable, we do have obtained valid blocksize and
>>> capacity from the optical drive, and we can use that capacity to 
>>> construct a
>>> default label.
>>>
>>>
>>> I'm currently using the following fix:
>>>
>>> diff -r c1119b5a9ffa usr/src/uts/common/io/scsi/targets/sd.c
>>> --- a/usr/src/uts/common/io/scsi/targets/sd.c   Wed Nov 29 16:09:14 
>>> 2006 -0800
>>> +++ b/usr/src/uts/common/io/scsi/targets/sd.c   Sun Dec 10 21:03:32 
>>> 2006 +0100
>>> @@ -4916,8 +4958,9 @@ sd_read_fdisk(struct sd_lun *un, uint_t
>>>         if (rval != 0) {
>>>                 SD_ERROR(SD_LOG_ATTACH_DETACH, un,
>>>                     "sd_read_fdisk: fdisk read err\n");
>>> -               kmem_free(bufp, blocksize);
>>> -               return (SD_CMD_FAILURE);
>>> +               bzero(un->un_fmap, sizeof (struct fmap) * FD_NUMPART);
>>> +               rval = SD_CMD_FAILURE;
>>> +               goto done;
>>>         }
>>>
>>>         mbp = (struct mboot *)bufp;
>>>
>>>
>>>
>>>
>>>
>>> diff -r c1119b5a9ffa usr/src/uts/common/io/scsi/targets/sd.c
>>> --- a/usr/src/uts/common/io/scsi/targets/sd.c   Wed Nov 29 16:09:14 
>>> 2006 -0800
>>> +++ b/usr/src/uts/common/io/scsi/targets/sd.c   Sun Dec 10 23:15:26 
>>> 2006 +0100
>>> @@ -4482,6 +4524,14 @@ sd_validate_geometry(struct sd_lun *un,
>>>                         return (EACCES);
>>>                 case SD_CMD_FAILURE:
>>>                         ASSERT(mutex_owned(SD_MUTEX(un)));
>>> +                       /*
>>> +                        * A multisession audio cd can have an unreadable
>>> +                        * fdisk sector, but there could be readable data
>>> +                        * in a separate session.  Accept this and let
>>> +                        * the code build a default disk label later on.
>>> +                        */
>>> +                       if (ISCD(un))
>>> +                               break;
>>>                         return (ENOMEM);
>>>                 }
>>>
>>>
>>>
>>>
>>> Comments?
>>>
>>>
>>> This message posted from opensolaris.org
>>> _______________________________________________
>>> ufs-discuss mailing list
>>> ufs-discuss at opensolaris.org
>>>
>>>
>>>
>>>       
>
> Juergen Keil                          jk at tools.de
> Tools GmbH                    +49 (228) 9858011
>
>   

Reply via email to