[parted-devel] [PATCH 1/6] libparted: refactor device-mapper partition sync code
Phillip Susi
psusi at ubuntu.com
Mon Oct 15 03:59:58 UTC 2012
The device-mapper partition sync code was still using the remove all
partitions, then add new partitions method. Refactor to use the same
algorithm as regular disks: try to remove all, and ignore any that could
not be removed but have not changed.
---
NEWS | 2 +
libparted/arch/linux.c | 403 +++++++++++++++++++++---------------------------
tests/Makefile.am | 1 +
tests/t6002-dm-busy.sh | 94 +++++++++++
4 files changed, 275 insertions(+), 225 deletions(-)
create mode 100644 tests/t6002-dm-busy.sh
diff --git a/NEWS b/NEWS
index 4c4716d..f182cce 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ GNU parted NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
** Bug Fixes
+ libparted: Don't fail to manipulate partitions on dmraid disks that
+ have other partitions in use.
libparted: mac: a MAC partition table could have a block_size larger
than the one the kernel told us about. Upon reading that partition
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index e2c4139..496e1c5 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2225,28 +2225,57 @@ zasprintf (const char *format, ...)
return r < 0 ? NULL : resultp;
}
+static char *dm_canonical_path (PedDevice *dev)
+{
+ char* dev_name = NULL;
+ LinuxSpecific* arch_specific = LINUX_SPECIFIC (dev);
+
+ /* Get map name from devicemapper */
+ struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
+ if (!task)
+ goto err;
+ if (!dm_task_set_major_minor (task, arch_specific->major,
+ arch_specific->minor, 0))
+ goto err;
+ if (!dm_task_run(task))
+ goto err;
+ dev_name = ped_malloc (strlen (dm_task_get_name (task)) +
+ strlen ("/dev/mapper/") + 1);
+ strcpy (dev_name, "/dev/mapper/");
+ strcat (dev_name, dm_task_get_name (task));
+ /* Caution: dm_task_destroy frees dev_name. */
+ dm_task_destroy (task);
+ return dev_name;
+err:
+ return NULL;
+}
+
static char*
_device_get_part_path (PedDevice *dev, int num)
{
- size_t path_len = strlen (dev->path);
-
+ char *devpath;
+ if (dev->type == PED_DEVICE_DM)
+ devpath = dm_canonical_path (dev);
+ else devpath = dev->path;
+ int path_len = strlen (devpath);
char *result;
/* Check for devfs-style /disc => /partN transformation
unconditionally; the system might be using udev with devfs rules,
and if not the test is harmless. */
- if (5 < path_len && !strcmp (dev->path + path_len - 5, "/disc")) {
+ if (5 < path_len && !strcmp (devpath + path_len - 5, "/disc")) {
/* replace /disc with /part%d */
result = zasprintf ("%.*s/part%d",
- (int) (path_len - 5), dev->path, num);
+ (int) (path_len - 5), devpath, num);
} else {
char const *p = (dev->type == PED_DEVICE_DAC960
|| dev->type == PED_DEVICE_CPQARRAY
|| dev->type == PED_DEVICE_ATARAID
- || isdigit (dev->path[path_len - 1])
+ || isdigit (devpath[path_len - 1])
? "p" : "");
- result = zasprintf ("%s%s%d", dev->path, p, num);
+ result = zasprintf ("%s%s%d", devpath, p, num);
}
-
+ if (dev->type == PED_DEVICE_DM)
+ free (devpath);
return result;
}
@@ -2530,6 +2559,8 @@ static unsigned int
_device_get_partition_range(PedDevice const* dev)
{
int range;
+ if (dev->type == PED_DEVICE_DM)
+ return MAX_NUM_PARTS;
bool ok = _sysfs_int_entry_from_dev(dev, "ext_range", &range);
if (!ok)
@@ -2538,6 +2569,126 @@ _device_get_partition_range(PedDevice const* dev)
return range > 1 ? range : 0;
}
+#ifdef ENABLE_DEVICE_MAPPER
+static int
+_dm_remove_partition(PedDisk* disk, int partno)
+{
+ int rc;
+ char *part_name = _device_get_part_path (disk->dev, partno);
+
+ int fd = open (part_name, O_RDONLY | O_EXCL);
+ if (fd == -1) {
+ if (errno == ENOENT)
+ errno = ENXIO; /* nothing to remove, device already doesn't exist */
+ free (part_name);
+ return 0;
+ }
+ close (fd);
+ struct dm_task *task = dm_task_create(DM_DEVICE_REMOVE);
+ if (!task) {
+ free (part_name);
+ return 0;
+ }
+ dm_task_set_name (task, part_name);
+ rc = dm_task_run(task);
+ dm_task_update_nodes();
+ dm_task_destroy(task);
+ free (part_name);
+ if (!rc)
+ return 0;
+
+ return 1;
+}
+
+static bool
+_dm_get_partition_start_and_length(PedPartition const *part,
+ unsigned long long *start,
+ unsigned long long *length)
+{
+ struct dm_task* task = NULL;
+ int rc = 0;
+
+ if (!(task = dm_task_create(DM_DEVICE_TABLE)))
+ return 0;
+ char *path = _device_get_part_path (part->disk->dev, part->num);
+ PED_ASSERT(path);
+ dm_task_set_name(task, path);
+ if (!dm_task_run(task))
+ goto err;
+
+ int major, minor;
+ char *params;
+ char *target_type;
+ dm_get_next_target(task, NULL, (uint64_t *)start, (uint64_t *)length, &target_type, ¶ms);
+ if (sscanf (params, "%d:%d %Ld", &major, &minor, start) != 3)
+ goto err;
+ rc = 1;
+err:
+ free (path);
+ dm_task_destroy(task);
+ return rc;
+}
+
+
+static int
+_dm_add_partition (PedDisk* disk, const PedPartition* part)
+{
+ LinuxSpecific* arch_specific = LINUX_SPECIFIC (disk->dev);
+
+ /* Get map name from devicemapper */
+ struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
+ if (!task)
+ goto err;
+
+ if (!dm_task_set_major_minor (task, arch_specific->major,
+ arch_specific->minor, 0))
+ goto err;
+
+ if (!dm_task_run(task))
+ goto err;
+
+ const char *dev_name = dm_task_get_name (task);
+ char *vol_name;
+ if (isdigit (dev_name[strlen (dev_name) - 1])) {
+ if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
+ goto err;
+ } else if ( ! (vol_name = zasprintf ("%s%d", dev_name, part->num)))
+ goto err;
+
+ /* Caution: dm_task_destroy frees dev_name. */
+ dm_task_destroy (task);
+ task = NULL;
+ char *params;
+ if ( ! (params = zasprintf ("%d:%d %lld", arch_specific->major,
+ arch_specific->minor, part->geom.start)))
+ goto err;
+
+ task = dm_task_create (DM_DEVICE_CREATE);
+ if (!task)
+ goto err;
+
+ dm_task_set_name (task, vol_name);
+ dm_task_add_target (task, 0, part->geom.length,
+ "linear", params);
+ if (dm_task_run (task)) {
+ dm_task_update_nodes();
+ dm_task_destroy(task);
+ free(params);
+ free(vol_name);
+ return 1;
+ } else {
+ _dm_remove_partition (disk, part->num);
+ }
+err:
+ dm_task_update_nodes();
+ if (task)
+ dm_task_destroy (task);
+ free (params);
+ free (vol_name);
+ return 0;
+}
+#endif
+
/*
* Sync the partition table in two step process:
* 1. Remove all of the partitions from the kernel's tables, but do not attempt
@@ -2558,8 +2709,23 @@ _disk_sync_part_table (PedDisk* disk)
PED_ASSERT(disk != NULL);
PED_ASSERT(disk->dev != NULL);
int lpn;
-
unsigned int part_range = _device_get_partition_range(disk->dev);
+ int (*add_partition)(PedDisk* disk, const PedPartition *part);
+ int (*remove_partition)(PedDisk* disk, int partno);
+ bool (*get_partition_start_and_length)(PedPartition const *part,
+ unsigned long long *start,
+ unsigned long long *length);
+
+
+ if (disk->dev->type == PED_DEVICE_DM) {
+ add_partition = _dm_add_partition;
+ remove_partition = _dm_remove_partition;
+ get_partition_start_and_length = _dm_get_partition_start_and_length;
+ } else {
+ add_partition = _blkpg_add_partition;
+ remove_partition = _blkpg_remove_partition;
+ get_partition_start_and_length = _kernel_get_partition_start_and_length;
+ }
/* lpn = largest partition number. */
if (ped_disk_get_max_supported_partition_count(disk, &lpn))
@@ -2594,7 +2760,7 @@ _disk_sync_part_table (PedDisk* disk)
int j;
for (j = 0; j < lpn; j++) {
if (!ok[j]) {
- ok[j] = _blkpg_remove_partition (disk, j + 1);
+ ok[j] = remove_partition (disk, j + 1);
errnums[j] = errno;
if (!ok[j] && errnums[j] == EBUSY)
busy = true;
@@ -2611,8 +2777,8 @@ _disk_sync_part_table (PedDisk* disk)
unsigned long long length;
unsigned long long start;
/* get start and length of existing partition */
- if (!_kernel_get_partition_start_and_length(part,
- &start, &length))
+ if (!get_partition_start_and_length(part,
+ &start, &length))
goto cleanup;
if (start == part->geom.start
&& length == part->geom.length)
@@ -2625,7 +2791,7 @@ _disk_sync_part_table (PedDisk* disk)
}
/* add the (possibly modified or new) partition */
- if (!_blkpg_add_partition (disk, part)) {
+ if (!add_partition (disk, part)) {
ped_exception_throw (
PED_EXCEPTION_ERROR,
PED_EXCEPTION_RETRY_CANCEL,
@@ -2671,215 +2837,6 @@ _disk_sync_part_table (PedDisk* disk)
return ret;
}
-#ifdef ENABLE_DEVICE_MAPPER
-static int
-_dm_remove_map_name(char *name)
-{
- struct dm_task *task = NULL;
- int rc;
-
- task = dm_task_create(DM_DEVICE_REMOVE);
- if (!task)
- return 1;
-
- dm_task_set_name (task, name);
-
- rc = dm_task_run(task);
- dm_task_update_nodes();
- dm_task_destroy(task);
- if (!rc)
- return 1;
-
- return 0;
-}
-
-static int
-_dm_is_part (struct dm_info *this, char *name)
-{
- struct dm_task* task = NULL;
- struct dm_info* info = alloca(sizeof *info);
- struct dm_deps* deps = NULL;
- int rc = 0;
- unsigned int i;
-
- task = dm_task_create(DM_DEVICE_DEPS);
- if (!task)
- return 0;
-
- dm_task_set_name(task, name);
- if (!dm_task_run(task))
- goto err;
-
- memset(info, '\0', sizeof *info);
- dm_task_get_info(task, info);
- if (!info->exists)
- goto err;
-
- deps = dm_task_get_deps(task);
- if (!deps)
- goto err;
-
- for (i = 0; i < deps->count; i++) {
- unsigned int ma = major(deps->device[i]),
- mi = minor(deps->device[i]);
-
- if (ma == this->major && mi == this->minor)
- rc = 1;
- }
-
-err:
- dm_task_destroy(task);
- return rc;
-}
-
-static int
-_dm_remove_parts (PedDevice* dev)
-{
- struct dm_task* task = NULL;
- struct dm_info* info = alloca(sizeof *info);
- struct dm_names* names = NULL;
- unsigned int next = 0;
- int rc;
- LinuxSpecific* arch_specific = LINUX_SPECIFIC (dev);
-
- task = dm_task_create(DM_DEVICE_LIST);
- if (!task)
- goto err;
-
- if (!dm_task_set_major_minor (task, arch_specific->major,
- arch_specific->minor, 0))
- goto err;
-
- if (!dm_task_run(task))
- goto err;
-
- memset(info, '\0', sizeof *info);
- dm_task_get_info(task, info);
- if (!info->exists)
- goto err;
-
- names = dm_task_get_names(task);
- if (!names)
- goto err;
-
- rc = 0;
- do {
- names = (void *) ((char *) names + next);
-
- if (_dm_is_part(info, names->name))
- rc += _dm_remove_map_name(names->name);
-
- next = names->next;
- } while (next);
-
- dm_task_update_nodes();
- dm_task_destroy(task);
- task = NULL;
-
- if (!rc)
- return 1;
-err:
- if (task)
- dm_task_destroy(task);
- ped_exception_throw (PED_EXCEPTION_WARNING, PED_EXCEPTION_IGNORE,
- _("parted was unable to re-read the partition "
- "table on %s (%s). This means Linux won't know "
- "anything about the modifications you made. "),
- dev->path, strerror (errno));
- return 0;
-}
-
-static int
-_dm_add_partition (PedDisk* disk, PedPartition* part)
-{
- char* vol_name = NULL;
- const char* dev_name = NULL;
- char* params = NULL;
- LinuxSpecific* arch_specific = LINUX_SPECIFIC (disk->dev);
-
- /* Get map name from devicemapper */
- struct dm_task *task = dm_task_create (DM_DEVICE_INFO);
- if (!task)
- goto err;
-
- if (!dm_task_set_major_minor (task, arch_specific->major,
- arch_specific->minor, 0))
- goto err;
-
- if (!dm_task_run(task))
- goto err;
-
- dev_name = dm_task_get_name (task);
-
- if (isdigit (dev_name[strlen (dev_name) - 1])) {
- if ( ! (vol_name = zasprintf ("%sp%d", dev_name, part->num)))
- goto err;
- } else if ( ! (vol_name = zasprintf ("%s%d", dev_name, part->num)))
- goto err;
-
- /* Caution: dm_task_destroy frees dev_name. */
- dm_task_destroy (task);
- task = NULL;
-
- if ( ! (params = zasprintf ("%d:%d %lld", arch_specific->major,
- arch_specific->minor, part->geom.start)))
- goto err;
-
- task = dm_task_create (DM_DEVICE_CREATE);
- if (!task)
- goto err;
-
- dm_task_set_name (task, vol_name);
- dm_task_add_target (task, 0, part->geom.length,
- "linear", params);
- if (dm_task_run (task)) {
- //printf("0 %ld linear %s\n", part->geom.length, params);
- dm_task_update_nodes();
- dm_task_destroy(task);
- free(params);
- free(vol_name);
- return 1;
- } else {
- _dm_remove_map_name(vol_name);
- }
-err:
- dm_task_update_nodes();
- if (task)
- dm_task_destroy (task);
- free (params);
- free (vol_name);
- return 0;
-}
-
-static int
-_dm_reread_part_table (PedDisk* disk)
-{
- int largest_partnum = ped_disk_get_last_partition_num (disk);
- if (largest_partnum <= 0)
- return 1;
-
- int rc = 1;
- int last = PED_MIN (largest_partnum, 16);
- int i;
-
- sync();
- if (!_dm_remove_parts(disk->dev))
- rc = 0;
-
- for (i = 1; i <= last; i++) {
- PedPartition* part;
-
- part = ped_disk_get_partition (disk, i);
- if (!part)
- continue;
-
- if (!_dm_add_partition (disk, part))
- rc = 0;
- }
- return rc;
-}
-#endif
-
static int
_have_blkpg ()
{
@@ -2897,10 +2854,6 @@ _have_blkpg ()
static int
linux_disk_commit (PedDisk* disk)
{
-#ifdef ENABLE_DEVICE_MAPPER
- if (disk->dev->type == PED_DEVICE_DM)
- return _dm_reread_part_table (disk);
-#endif
if (disk->dev->type != PED_DEVICE_FILE) {
/* We now require BLKPG support. If this assertion fails,
diff --git a/tests/Makefile.am b/tests/Makefile.am
index cdc1c4b..4649c0a 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -58,6 +58,7 @@ TESTS = \
t5000-tags.sh \
t6000-dm.sh \
t6001-psep.sh \
+ t6002-dm-busy.sh \
t6100-mdraid-partitions.sh \
t7000-scripting.sh \
t8000-loop.sh \
diff --git a/tests/t6002-dm-busy.sh b/tests/t6002-dm-busy.sh
new file mode 100644
index 0000000..040f18f
--- /dev/null
+++ b/tests/t6002-dm-busy.sh
@@ -0,0 +1,94 @@
+#!/bin/sh
+# ensure that parted can alter a partition on a dmraid disk
+# while another one is mounted
+
+# Copyright (C) 2008-2012 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../parted
+
+require_root_
+lvm_init_root_dir_
+
+test "x$ENABLE_DEVICE_MAPPER" = xyes \
+ || skip_ "no device-mapper support"
+
+# Device maps names - should be random to not conflict with existing ones on
+# the system
+linear_=plinear-$$
+
+d1=
+f1=
+dev=
+cleanup_fn_() {
+ umount "${dev}p2" > /dev/null 2>&1
+ dmsetup remove ${linear_}p1
+ dmsetup remove ${linear_}p2
+ dmsetup remove $linear_
+ test -n "$d1" && losetup -d "$d1"
+ rm -f "$f1"
+}
+
+f1=$(pwd)/1; d1=$(loop_setup_ "$f1") \
+ || fail=1
+
+# setup: create a mapping
+echo "0 2048 linear $d1 0" | dmsetup create $linear_ || fail=1
+dev="$DM_DEV_DIR/mapper/$linear_"
+
+# Create msdos partition table
+parted -s $dev mklabel msdos > out 2>&1 || fail=1
+compare /dev/null out || fail=1
+
+parted -s $dev -a none mkpart primary fat32 1s 1000s > out 2>&1 || fail=1
+compare /dev/null out || fail=1
+
+parted -s $dev -a none mkpart primary fat32 1001s 2000s > out 2>&1 || fail=1
+compare /dev/null out || fail=1
+
+# wait for new partition device to appear
+wait_for_dev_to_appear_ ${dev}p2 || fail_ ${dev}p2 did not appear
+
+mkfs.vfat -F 32 ${dev}p2 || fail_ mkfs.vfat failed
+
+mount_point=$(pwd)/mnt
+
+mkdir $mount_point || fail=1
+mount "${dev}p2" "$mount_point" || fail=1
+
+# Removal of unmounted partition must succeed.
+parted -s "$dev" rm 1 > /dev/null 2>&1 || fail=1
+
+# Removal of mounted partition must fail.
+parted -s "$dev" rm 2 > /dev/null 2>&1 && fail=1
+
+parted -s "$dev" print > out 2>&1 || fail=1
+sed 's/^Disk .*: /Disk DEV: /' out > k; mv k out
+
+# Create expected output file.
+cat <<EOF >> exp || fail=1
+Model: Linux device-mapper (linear) (dm)
+Disk DEV: 1049kB
+Sector size (logical/physical): 512B/512B
+Partition Table: msdos
+
+Number Start End Size Type File system Flags
+ 2 513kB 1025kB 512kB primary fat32 lba
+
+EOF
+
+compare exp out || fail=1
+
+Exit $fail
--
1.7.10.4
More information about the parted-devel
mailing list