Skip to contentSkip to conversation

8a3e33cf92c7 "ata: ahci: find eSATA ports and flag them as removable" changes userspace behavior

REGRESSION Laura Abbott 3 participants 8 messages Feb 25, 2016 – Feb 27, 2016 libata

lore
Share & tools
lore

Laura Abbott

author
#0 lore raw

Hi,

We received a bugzilla report https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible. This seems to be classified as 'breaking' userspace given that this behavior results in partitions that were previously unmounted now being being mounted unwanted automatically which is unwanted behavior for the user. Can we revert this patch or give a fix to change this behavior?

Thanks,
Laura

#1 lore raw
Quoted 14 lines · Laura Abbott

Hi,

We received a bugzilla report
https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

This patch doesn't cause esata disks to be automounted on my distro (gentoo), it just let's them appear in KDE's removable device widget for me to mount it at a klick's notice.
I'm not convinced this is a "breaking userspace" problem.

Manuel

#2 lore raw
In reply to Laura Abbott
Quoted 14 lines · Laura Abbott

Hi,

We received a bugzilla report
https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

Do the affected users have sata ports set to hotplug capable? Because that's what the patch does: it flags ports which have the hotplug capability enabled as "removable", and I guess redhat just automounts anything that is flagged as a removable disk (like an usb drive).

Manuel

author
#3 lore raw
Quoted 16 lines · Manuel Lauss
Quoted 14 lines · Laura Abbott

Hi,

We received a bugzilla report
https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

Quoted 7 lines

Do the affected users have sata ports set to hotplug capable? Because that's what the patch does: it flags ports which have the hotplug capability enabled as "removable", and I guess redhat just automounts anything that is flagged as a removable disk (like an usb drive).

Manuel

The mounting comes from udisks. udisks will automount anything marked as removable unless explicitly forbidden. You can argue whether or not this is good design but it's still a change in behavior from what udisks was relying on to decide whether or not to automount. lspci from the users hardware:

00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port SATA AHCI Controller (rev 06)

I'll make a build with the patch you gave for testing.

Thanks,
Laura

#4 lore raw

Hello, Laura.

Quoted 7 lines · Laura Abbott

The mounting comes from udisks. udisks will automount anything marked as removable unless explicitly forbidden. You can argue whether or not this is good design but it's still a change in behavior from what udisks was relying on to decide whether or not to automount. lspci from the users hardware:

00:1f.2 SATA controller: Intel Corporation C600/X79 series chipset 6-Port SATA AHCI Controller (rev 06)

It is a behavior change but difficult to characterize as a breakage, given that we do want userland to treat ports explicitly marked as external to be treated as such. That said, eSATA being mostly a fringe thing, we can go "whatever" and revert it.

Let's hope it's a false positive that the patch can fix.

Thanks.

Signature · 1 line
tejun
#5 lore raw
In reply to Laura Abbott
Quoted 14 lines · Laura Abbott

Hi,

We received a bugzilla report
https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

Can you test this? I think the old HPCP bit should just be ignored, although windows does use it, i.e. when I enable hotplug capability in the bios on my desktop it shows the "eject device" icon for a device attached to the port on the motherboard, we probably can ignore it on linux to not break all automounting distros. On my laptop, where I care about this, it still works as intended.

drivers/ata/libahci.c +0 -0
index 4029679..e029e3c 100644
@@ -1164,8 +1164,7 @@ static void ahci_port_init(struct device *dev,

struct ata_port *ap,

        /* mark esata ports */
        tmp = readl(port_mmio + PORT_CMD);
-       if ((tmp & PORT_CMD_HPCP) ||
-           ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)))
+       if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
                ap->pflags |= ATA_PFLAG_EXTERNAL;

}

author Tested-by Bob Schultz
#6 lore raw
Quoted 29 lines · Manuel Lauss
Quoted 14 lines · Laura Abbott

Hi,

We received a bugzilla report
https://bugzilla.redhat.com/show_bug.cgi?id=1310682
of partitions being automounted unexpectedly. Testing showed that 8a3e33cf92c7 ("ata: ahci: find eSATA ports and flag them as removable") was responsible.
This seems to be classified as 'breaking' userspace given that this behavior results
in partitions that were previously unmounted now being being mounted unwanted
automatically which is unwanted behavior for the user. Can we revert this patch or
give a fix to change this behavior?

Can you test this? I think the old HPCP bit should just be ignored, although windows does use it, i.e. when I enable hotplug capability in the bios on my desktop it shows the "eject device" icon for a device attached to the port on the motherboard, we probably can ignore it on linux to not break all automounting distros. On my laptop, where I care about this, it still works as intended.

drivers/ata/libahci.c +0 -0
index 4029679..e029e3c 100644

drivers/ata/libahci.c static void ahci_port_init(struct device *dev, · line 1170

Quoted 9 lines
drivers/ata/libahci.c +1 -2
@@ -1164,8 +1164,7 @@ static void ahci_port_init(struct device *dev,
 struct ata_port *ap,
 
         /* mark esata ports */
         tmp = readl(port_mmio + PORT_CMD);
-       if ((tmp & PORT_CMD_HPCP) ||
-           ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS)))
+       if ((tmp & PORT_CMD_ESP) && (hpriv->cap & HOST_CAP_SXS))
                 ap->pflags |= ATA_PFLAG_EXTERNAL;
  }

At least one user reported that this fixed the issue and he said you could add

Tested-by Bob Schultz <[email protected]>

Thanks,
Laura

#7 lore raw

Hello,

Quoted 4 lines · Laura Abbott

At least one user reported that this fixed the issue and he said you could add

Tested-by Bob Schultz <[email protected]>

Great, Manuel, can you please send a proper patch?

Thanks.

Signature · 1 line
tejun