From 5af1835d678031d4a6615edc96ba58c82304b31d Mon Sep 17 00:00:00 2001 From: Jeff Sharkey Date: Tue, 7 Jul 2015 17:26:59 -0700 Subject: [PATCH] Generate stable MTP storage IDs. It ends up that MediaProvider is persisting MTP storage IDs in its database, so we need to make sure we generate stable IDs over time, otherwise we can end up looking into a black hole. Bug: 22256092 Change-Id: I6a75c239aac1b71fd5f6df0df69b24971079a086 --- .../android/os/storage/StorageVolume.java | 3 ++ core/java/android/os/storage/VolumeInfo.java | 42 +++++++++++++++---- media/java/android/mtp/MtpStorage.java | 12 ------ .../java/com/android/server/MountService.java | 17 ++------ 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/core/java/android/os/storage/StorageVolume.java b/core/java/android/os/storage/StorageVolume.java index d66e228de41da..1408202a35ea4 100644 --- a/core/java/android/os/storage/StorageVolume.java +++ b/core/java/android/os/storage/StorageVolume.java @@ -58,6 +58,9 @@ public class StorageVolume implements Parcelable { // ACTION_MEDIA_BAD_REMOVAL, ACTION_MEDIA_UNMOUNTABLE and ACTION_MEDIA_EJECT broadcasts. public static final String EXTRA_STORAGE_VOLUME = "storage_volume"; + public static final int STORAGE_ID_INVALID = 0x00000000; + public static final int STORAGE_ID_PRIMARY = 0x00010001; + public StorageVolume(String id, int storageId, File path, String description, boolean primary, boolean removable, boolean emulated, long mtpReserveSize, boolean allowMassStorage, long maxFileSize, UserHandle owner, String fsUuid, String state) { diff --git a/core/java/android/os/storage/VolumeInfo.java b/core/java/android/os/storage/VolumeInfo.java index 91cb9443c0fc3..a4ee8b774a9e6 100644 --- a/core/java/android/os/storage/VolumeInfo.java +++ b/core/java/android/os/storage/VolumeInfo.java @@ -147,15 +147,11 @@ public class VolumeInfo implements Parcelable { public String path; public String internalPath; - /** Framework state */ - public final int mtpIndex; - - public VolumeInfo(String id, int type, DiskInfo disk, String partGuid, int mtpIndex) { + public VolumeInfo(String id, int type, DiskInfo disk, String partGuid) { this.id = Preconditions.checkNotNull(id); this.type = type; this.disk = disk; this.partGuid = partGuid; - this.mtpIndex = mtpIndex; } public VolumeInfo(Parcel parcel) { @@ -175,7 +171,6 @@ public class VolumeInfo implements Parcelable { fsLabel = parcel.readString(); path = parcel.readString(); internalPath = parcel.readString(); - mtpIndex = parcel.readInt(); } public static @NonNull String getEnvironmentForState(int state) { @@ -308,7 +303,6 @@ public class VolumeInfo implements Parcelable { final boolean removable; final boolean emulated; final boolean allowMassStorage = false; - final int mtpStorageId = MtpStorage.getStorageIdForIndex(mtpIndex); final String envState = getEnvironmentForState(state); File userPath = getPathForUser(userId); @@ -326,9 +320,15 @@ public class VolumeInfo implements Parcelable { long mtpReserveSize = 0; long maxFileSize = 0; + int mtpStorageId = StorageVolume.STORAGE_ID_INVALID; if (type == TYPE_EMULATED) { emulated = true; + + if (isPrimary()) { + mtpStorageId = StorageVolume.STORAGE_ID_PRIMARY; + } + mtpReserveSize = StorageManager.from(context).getStorageLowBytes(userPath); if (ID_EMULATED_INTERNAL.equals(id)) { @@ -341,6 +341,14 @@ public class VolumeInfo implements Parcelable { emulated = false; removable = true; + if (isPrimary()) { + mtpStorageId = StorageVolume.STORAGE_ID_PRIMARY; + } else { + // Since MediaProvider currently persists this value, we need a + // value that is stable over time. + mtpStorageId = buildStableMtpStorageId(fsUuid); + } + if ("vfat".equals(fsType)) { maxFileSize = 4294967295L; } @@ -354,6 +362,24 @@ public class VolumeInfo implements Parcelable { fsUuid, envState); } + public static int buildStableMtpStorageId(String fsUuid) { + if (TextUtils.isEmpty(fsUuid)) { + return StorageVolume.STORAGE_ID_INVALID; + } else { + int hash = 0; + for (int i = 0; i < fsUuid.length(); ++i) { + hash = 31 * hash + fsUuid.charAt(i); + } + hash = (hash ^ (hash << 16)) & 0xffff0000; + // Work around values that the spec doesn't allow, or that we've + // reserved for primary + if (hash == 0x00000000) hash = 0x00020000; + if (hash == 0x00010000) hash = 0x00020000; + if (hash == 0xffff0000) hash = 0xfffe0000; + return hash | 0x0001; + } + } + // TODO: avoid this layering violation private static final String DOCUMENT_AUTHORITY = "com.android.externalstorage.documents"; private static final String DOCUMENT_ROOT_PRIMARY_EMULATED = "primary"; @@ -402,7 +428,6 @@ public class VolumeInfo implements Parcelable { pw.println(); pw.printPair("path", path); pw.printPair("internalPath", internalPath); - pw.printPair("mtpIndex", mtpIndex); pw.decreaseIndent(); pw.println(); } @@ -469,6 +494,5 @@ public class VolumeInfo implements Parcelable { parcel.writeString(fsLabel); parcel.writeString(path); parcel.writeString(internalPath); - parcel.writeInt(mtpIndex); } } diff --git a/media/java/android/mtp/MtpStorage.java b/media/java/android/mtp/MtpStorage.java index 3641ff50315c3..6ca442c7e66fb 100644 --- a/media/java/android/mtp/MtpStorage.java +++ b/media/java/android/mtp/MtpStorage.java @@ -53,18 +53,6 @@ public class MtpStorage { return mStorageId; } - /** - * Generates a storage ID for storage of given index. - * Index 0 is for primary external storage - * - * @return the storage ID - */ - public static int getStorageIdForIndex(int index) { - // storage ID is 0x00010001 for primary storage, - // then 0x00020001, 0x00030001, etc. for secondary storages - return ((index + 1) << 16) + 1; - } - /** * Returns the file path for the storage unit's storage in the file system * diff --git a/services/core/java/com/android/server/MountService.java b/services/core/java/com/android/server/MountService.java index 894f513ed1a96..c7ca1ab19d315 100644 --- a/services/core/java/com/android/server/MountService.java +++ b/services/core/java/com/android/server/MountService.java @@ -402,16 +402,6 @@ class MountService extends IMountService.Stub } } - private static int sNextMtpIndex = 1; - - private static int allocateMtpIndex(String volId) { - if (VolumeInfo.ID_EMULATED_INTERNAL.equals(volId)) { - return 0; - } else { - return sNextMtpIndex++; - } - } - /** List of crypto types. * These must match CRYPT_TYPE_XXX in cryptfs.h AND their * corresponding commands in CommandListener.cpp */ @@ -742,7 +732,7 @@ class MountService extends IMountService.Stub // Create a stub volume that represents internal storage final VolumeInfo internal = new VolumeInfo(VolumeInfo.ID_PRIVATE_INTERNAL, - VolumeInfo.TYPE_PRIVATE, null, null, 0); + VolumeInfo.TYPE_PRIVATE, null, null); internal.state = VolumeInfo.STATE_MOUNTED; internal.path = Environment.getDataDirectory().getAbsolutePath(); mVolumes.put(internal.id, internal); @@ -967,8 +957,7 @@ class MountService extends IMountService.Stub final String partGuid = TextUtils.nullIfEmpty(cooked[4]); final DiskInfo disk = mDisks.get(diskId); - final int mtpIndex = allocateMtpIndex(id); - final VolumeInfo vol = new VolumeInfo(id, type, disk, partGuid, mtpIndex); + final VolumeInfo vol = new VolumeInfo(id, type, disk, partGuid); mVolumes.put(id, vol); onVolumeCreatedLocked(vol); break; @@ -2604,7 +2593,7 @@ class MountService extends IMountService.Stub final String uuid = null; final String state = Environment.MEDIA_REMOVED; - res.add(0, new StorageVolume(id, MtpStorage.getStorageIdForIndex(0), path, + res.add(0, new StorageVolume(id, StorageVolume.STORAGE_ID_INVALID, path, description, primary, removable, emulated, mtpReserveSize, allowMassStorage, maxFileSize, owner, uuid, state)); }