Re: [PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 5)

[Vitaliy Margolen]

On 05/09/2011 07:42 AM, Charles Davis wrote:
> Try 5: Get the value of "DeviceName" from mountmgr instead of making one up.
> Try 4: Fix unused variable warning.
> Try 3: Add support for IDE drives.
> Try 2: Don't depend on the SCSI generic driver on Linux.
> - RtlCreateUnicodeStringFromAsciiz( &nameW, "FirstBusTimeScanInMs" );
> - static const WCHAR bus_scan_timeW[] = {'F','i','r','s','t','B','u','s','S','c','a','n','T','i','m','e','I','n','M','s',0};

These do not match.
> - if (devtype)
> - {
> -
>    if (!(type = p_libhal_device_get_property_string( ctx, udi, "storage.drive_type",&error )))
> -
>        *devtype = ~0;
> -
>    else if (!strcmp( type, "disk" ) || !strcmp( type, "floppy" ))
> -
>        *devtype = 0x00;
> -
>    else if (!strcmp( type, "tape" ))
> -
>        *devtype = 0x01;
> -
>    else if (!strcmp( type, "cdrom" ))
> -
>        *devtype = 0x05;
> -
>    else if (!strcmp( type, "raid" ))
> -
>        *devtype = 0x0C;
> -
>    else
> -
>        *devtype = ~0;
> - }

Please don't use magic numbers here. Also these numbers are wrong. From 
winbase.h:
#define DRIVE_UNKNOWN              0
#define DRIVE_NO_ROOT_DIR          1
#define DRIVE_REMOVABLE            2
#define DRIVE_FIXED                3
#define DRIVE_REMOTE               4
/* Win32 additions */
#define DRIVE_CDROM                5
#define DRIVE_RAMDISK              6

Missing "DeviceName"="Cdrom0" for cdroms.

Also names of devices do not match. This can potentially break some software:
-"Identifier"="ATA     Hitachi HDS72202JKAO"
+"Identifier"="ATA Hitachi HDS72202"
-"Identifier"="ATA     WDC WD7500AAKS-030.0"
+"Identifier"="ATA WDC WD7500AAKS-0"
-"Identifier"="SAMSUNG DVDWBD SH-B083L SB01"
+"Identifier"="SAMSUNG DVDWBD SH-B083L"

Vitaliy.

[PATCH 1/2] mountmgr: Populate HKLM\HARDWARE\DEVICEMAP\Scsi here instead of in kernel32. (try 5)

[Charles Davis]

On 5/9/11 8:36 AM, Vitaliy Margolen wrote:
> On 05/09/2011 07:42 AM, Charles Davis wrote:
> > Try 5: Get the value of "DeviceName" from mountmgr instead of making
> > one up.
> > Try 4: Fix unused variable warning.
> > Try 3: Add support for IDE drives.
> > Try 2: Don't depend on the SCSI generic driver on Linux.
> > - RtlCreateUnicodeStringFromAsciiz( &nameW, "FirstBusTimeScanInMs" );
> > - static const WCHAR bus_scan_timeW[] =
> >
> > {'F','i','r','s','t','B','u','s','S','c','a','n','T','i','m','e','I','n','M','s',0};
>
> These do not match.

You're right, will fix.
> > - if (devtype)
> > - {
> > -
> >    if (!(type = p_libhal_device_get_property_string( ctx, udi,
> >
> > "storage.drive_type",&error )))
> > -
> >        *devtype = ~0;
> > -
> >    else if (!strcmp( type, "disk" ) || !strcmp( type, "floppy" ))
> > -
> >        *devtype = 0x00;
> > -
> >    else if (!strcmp( type, "tape" ))
> > -
> >        *devtype = 0x01;
> > -
> >    else if (!strcmp( type, "cdrom" ))
> > -
> >        *devtype = 0x05;
> > -
> >    else if (!strcmp( type, "raid" ))
> > -
> >        *devtype = 0x0C;
> > -
> >    else
> > -
> >        *devtype = ~0;
> > - }
>
> Please don't use magic numbers here.

OK.
> Also these numbers are wrong. From
> winbase.h:
> #define DRIVE_UNKNOWN              0
> #define DRIVE_NO_ROOT_DIR          1
> #define DRIVE_REMOVABLE            2
> #define DRIVE_FIXED                3
> #define DRIVE_REMOTE               4
> /* Win32 additions */
> #define DRIVE_CDROM                5
> #define DRIVE_RAMDISK              6

The numbers I used are actually the SCSI peripheral device type numbers.
I use them in create_scsi_entry() to figure out which one of the
"*Peripheral" strings to put in the registry. But yeah, I probably
shouldn't use magic numbers like that.
> Missing "DeviceName"="Cdrom0" for cdroms.

You only get a DeviceName if there's a disk in the drive (a limitation
of the way mountmgr works).
> Also names of devices do not match. This can potentially break some
> software:
> -"Identifier"="ATA     Hitachi HDS72202JKAO"
> +"Identifier"="ATA Hitachi HDS72202"
> -"Identifier"="ATA     WDC WD7500AAKS-030.0"
> +"Identifier"="ATA WDC WD7500AAKS-0"

OK, I can fix these.
> -"Identifier"="SAMSUNG DVDWBD SH-B083L SB01"
> +"Identifier"="SAMSUNG DVDWBD SH-B083L"

Can't fix that. HAL provides no way to query the SCSI revision of a SCSI
device. Short of actually sending an INQUIRY SCSI command to the device,
I don't know how to fix that.

Thanks.

Chip