[systemd-devel] Fix for - udev/ata_id failing on SATA drives behind JMicron USB bridge

[Harendra Kumar]

Reviving this thread again. Copying a few folks based on the history of the
impacted code. I have the fix implemented which is working fine on my
machine for the last few months. I can open an issue and send a pull
request if the suggested fix looks ok.

-harendra

On Wed, Oct 14, 2015 at 9:32 PM, Lennart Poettering <lennart at poettering.net>
wrote:

> On Wed, 30.09.15 20:40, Harendra Kumar (harendra.kumar at gmail.com) wrote:
>
> > I have a few drives in an enclosure (mediasonic pro box) connected via
> USB
> > using a Jmicron USB bridge (ID 152d:0567 JMicron Technology Corp.). I am
> > not able to create the desired names ("ata-<model>-<serial>") for these
> > devices in /dev/disk/by-id because ata_id is unable to retrieve the ATA
> > device identity information for these drives.
> >
> > I did some root causing and found where the problem lies. The ATA Pass
> > Through command (
> >
> https://github.com/systemd/systemd/blob/master/src/udev/ata_id/ata_id.c#L130
> )
> > sets the check condition flag (CK_COND=1). In the response it expects and
> > checks for sense data -
> >
> https://github.com/systemd/systemd/blob/master/src/udev/ata_id/ata_id.c#L178
> > .
> >
> > It seems the JMicron USB bridge successfully responds to the ATA IDENTIFY
> > DEVICE command but it does not return any sense data along with it and
> > therefore the disk_identify_command() function returns EIO even though
> the
> > command was successful and we got all that we needed. I checked by
> removing
> > this condition and the ata_id output was found to be accurate.
> >
> > The behavior implemented by ata_id seems to be correct strictly speaking
> as
> > per the SAT draft. It seems the Jmicron USB hardware does not comply with
> > the standard. Here is an excerpt from page 123 of the SAT draft I found
> by
> > googling (
> >
> http://citeseerx.ist.psu.edu/viewdoc/download?doi=10.1.1.394.3605&rep=rep1&type=pdf
> > ).
> >
> > -----snip-----
> > The CK_COND (Check Condition) bit may be used to request the SATL to
> return
> > a copy of ATA register information in the sense data upon command
> > completion. If the CK_COND bit is set to one the SATL shall return a
> status
> > of CHECK CONDITION when the ATA command completes, even if the command
> > completes successfully, and return the ATA Normal Output fields (see
> > ATA8-ACS) in the sense data using the ATA Return descriptor (see 12.2.6).
> > If the CK_COND bit is set to zero, then the SATL shall terminate the
> > command with CHECK CONDITION status only if an error occurs in processing
> > the command. See clause 11 for a description of ATA error conditions.
> > -----snip-----
> >
> > Can anyone suggest what the right fix for this would be? One way I can
> > think of is to not set the CK_COND flag in the command and check for
> sense
> > error only if check condition status/sense data was returned by the
> device.
> > This seems to be the way smartctl works for SAT devices and that's why it
> > works for my JMicron as well. Will this cause any problems? Why was the
> > code written this way in the first place? Is there a better fix for this?
>
> I figure you should put the folks who wrote that code specifically in
> CC, as they might not be on this ML. You may find that information in
> the git history data.
>
> Lennart
>
> --
> Lennart Poettering, Red Hat
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/systemd-devel/attachments/20151214/28356165/attachment.html>