[parted-devel] [PATCH 0/3 v2] Support for partitions on loop devices
Jim Meyering
jim at meyering.net
Thu Sep 29 14:42:56 UTC 2011
Petr Uzel wrote:
> This is the second version of patch series to improve support
> for partitionable loop devices in parted.
>
> Changes since v1:
>
> o Added test to exercise the feature.
> o Style fixes suggested by Jim.
>
> Petr Uzel (3):
> libparted: differentiate between plain files and loop devices
> libparted: improve support for partitions on loopback devices
> tests: add test for partitionable loop devices
>
> NEWS | 4 ++
> include/parted/device.h | 3 +-
> libparted/arch/linux.c | 84 +++++++++++++++++++++++++++++++++++++-------
> parted/parted.c | 2 +-
> tests/Makefile.am | 1 +
> tests/t8001-loop-blkpg.sh | 65 ++++++++++++++++++++++++++++++++++
> 6 files changed, 143 insertions(+), 16 deletions(-)
> create mode 100755 tests/t8001-loop-blkpg.sh
Thanks.
I've made some small changes, first to logs, then to the code.
Here are the diffs, followed by a complete patch.
Note that two of your three patches did not apply cleanly,
and for one, I had to omit a space-changing hunk because
the line it modified must have been added in your other series,
which I have not yet applied.
The changes are mostly stylistic.
For comments something called "the active voice" is generally preferred.
>From HACKING,
When writing prose (documentation, comments, log entries), use an
active voice, not a passive one. I.e., say "print the frobnozzle",
not "the frobnozzle will be printed".
Thanks a lot for the new test.
I've adjusted it not to discard the results of grep, and
especially not stderr. If things go wrong, stderr will often
provide details.
I removed the "rm -f backing_file" statement,
since everything in the containing directory is
removed as part of normal clean-up at the end of each test.
I noticed that you use the "udevadm" command.
It would be good to add a function in init.cfg that
makes the test skip when it's not available or when
it does not support the --timeout option.
In fact, this entire test fails on pre-linux-3.0 systems.
(it's failing on my Fedora 15 2.6.40.4-5.fc15.x86_64 desktop)
It'd be nice to make it skip rather than fail in that case, but I
don't mind if these proposed test-related changes are in a separate
patch. That might be better regardless,...
If you ACK what I've included below, I'll push these three commits.
---------------------------------------------------------------
--- k 2011-09-29 16:22:38.841650281 +0200
+++ k-new 2011-09-29 16:22:22.882216118 +0200
@@ -1,11 +1,11 @@
-From b98d3dab38095738abed76986547f0f4403f7caf Mon Sep 17 00:00:00 2001
+From 3e35b6559ad1ebfbad47e8edb2b002d7c07c984c Mon Sep 17 00:00:00 2001
From: Petr Uzel <petr.uzel at suse.cz>
Date: Thu, 29 Sep 2011 15:45:23 +0200
Subject: [PATCH 1/3] libparted: differentiate between plain files and loop
devices
-Stop using PED_DEVICE_FILE for loopback devices since loopback
-is something other than plain files.
+Stop using PED_DEVICE_FILE for loopback devices;
+loopback are significantly different from plain files.
* include/parted/device.h (PedDeviceType): Add PED_DEVICE_LOOP.
* libparted/arch/linux.c (_device_probe_type): Detect loopback device.
* parted/parted.c (do_print): Add "loopback" to list of transports.
@@ -71,15 +71,14 @@ index 32c2fcc..51ecdaf 100644
1.7.7.rc0.362.g5a14
-From 6f80711605f36bddaf07266601ba33ddd655ada3 Mon Sep 17 00:00:00 2001
+From 1eb0cc30d9a2f6a2a6c1f7ec0e4003b1081c5738 Mon Sep 17 00:00:00 2001
From: Petr Uzel <petr.uzel at suse.cz>
Date: Thu, 29 Sep 2011 15:14:23 +0200
Subject: [PATCH 2/3] libparted: improve support for partitions on loopback
devices
-Since kernel 2.6.26, kernel allows partitions on loopback devices.
+Since linux-2.6.26, the kernel allows partitions on loopback devices.
Implement support for this feature in parted.
-
* libparted/arch/linux.c (_sysfs_int_entry_from_dev): New function.
(_loop_get_partition_range): New function.
(_device_get_partition_range): Add special handling for loop devices.
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 50b3426..b1c12b5 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2428,47 +2428,39 @@ _blkpg_remove_partition (PedDisk* disk, int n)
BLKPG_DEL_PARTITION);
}
-/*
- * Read the integer from /sys/block/DEV_BASE/ENTRY and set *val
- * to that value, where DEV_BASE is the last component of
- * DEV->path. Upon success, return true. Otherwise, return false.
- */
+/* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL
+ to that value, where DEV_BASE is the last component of DEV->path.
+ Upon success, return true. Otherwise, return false. */
static bool
_sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val)
{
- int r;
char path[128];
- FILE* fp;
- bool ok;
-
- r = snprintf(path, sizeof(path), "/sys/block/%s/%s",
- last_component(dev->path), entry);
+ int r = snprintf(path, sizeof(path), "/sys/block/%s/%s",
+ last_component(dev->path), entry);
if (r < 0 || r >= sizeof(path))
return false;
- fp = fopen(path, "r");
+ FILE *fp = fopen(path, "r");
if (!fp)
return false;
- ok = fscanf(fp, "%d", val) == 1;
+ bool ok = fscanf(fp, "%d", val) == 1;
fclose(fp);
return ok;
}
-/*
- * Returns maximum number of partitions that the loopback device can hold.
- * First checks if the loop module exports max_part parameter (since kernel
- * 3.0), if it does not succeed, it falls back to checking ext_range, which
- * seems to have (for some reason) different semantics compared to other
- * devices; specifically, ext_range <= 1 means that the loopback device does
- * not support partitions.
- */
+/* Return the maximum number of partitions that the loopback device can hold.
+ First, check the loop-module-exported max_part parameter (since linux-3.0).
+ If that is not available, fall back to checking ext_range, which seems to
+ have (for some reason) different semantics compared to other devices;
+ specifically, ext_range <= 1 means that the loopback device does
+ not support partitions. */
static unsigned int
_loop_get_partition_range(PedDevice const* dev)
{
int max_part;
- bool ok = 0;
+ bool ok = false;
/* max_part module param is exported since kernel 3.0 */
FILE *fp = fopen("/sys/module/loop/parameters/max_part", "r");
diff --git a/tests/t8001-loop-blkpg.sh b/tests/t8001-loop-blkpg.sh
index 4373d7c..a521309 100755
--- a/tests/t8001-loop-blkpg.sh
+++ b/tests/t8001-loop-blkpg.sh
@@ -23,12 +23,11 @@ require_root_
cleanup_fn_()
{
test -n "$loopdev" && losetup -d "$loopdev"
- rm -f backing_file
}
# If the loop module is loaded, unload it first
-if lsmod | grep '^loop[[:space:]]' >/dev/null 2>&1; then
- rmmod loop || fail=1
+if lsmod | grep '^loop[[:space:]]'; then
+ rmmod loop || fail=1
fi
# Insert loop module with max_part > 1
@@ -47,19 +46,19 @@ compare err /dev/null || fail=1 # expect no output
# Create a partition
parted -s "$loopdev" mkpart primary 1M 2M > err 2>&1 || fail=1
-compare err /dev/null || fail=1 # expect no output
+compare /dev/null err || fail=1 # expect no output
udevadm settle --timeout=5 || fail=1
# Verify that the partition appeared in /proc/partitions
entry=`basename "$loopdev"p1`
-grep "$entry" /proc/partitions >/dev/null 2>&1 || fail=1
+grep "$entry" /proc/partitions || fail=1
# Remove the partition
parted -s "$loopdev" rm 1 > err 2>&1 || fail=1
-compare err /dev/null || fail=1 # expect no output
+compare /dev/null err || fail=1 # expect no output
udevadm settle --timeout=5 || fail=1
# Verify that the partition got removed from /proc/partitions
-grep "$entry" /proc/partitions >/dev/null 2>&1 && fail=1
+grep "$entry" /proc/partitions && fail=1
Exit $fail
Here is the complete series.
Please review it. I'll push this if/when you ACK.
--------------------------------------------
>From 3e35b6559ad1ebfbad47e8edb2b002d7c07c984c Mon Sep 17 00:00:00 2001
From: Petr Uzel <petr.uzel at suse.cz>
Date: Thu, 29 Sep 2011 15:45:23 +0200
Subject: [PATCH 1/3] libparted: differentiate between plain files and loop
devices
Stop using PED_DEVICE_FILE for loopback devices;
loopback are significantly different from plain files.
* include/parted/device.h (PedDeviceType): Add PED_DEVICE_LOOP.
* libparted/arch/linux.c (_device_probe_type): Detect loopback device.
* parted/parted.c (do_print): Add "loopback" to list of transports.
---
include/parted/device.h | 3 ++-
libparted/arch/linux.c | 7 ++++++-
parted/parted.c | 2 +-
3 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/parted/device.h b/include/parted/device.h
index 0634465..b94765c 100644
--- a/include/parted/device.h
+++ b/include/parted/device.h
@@ -48,7 +48,8 @@ typedef enum {
PED_DEVICE_SDMMC = 14,
PED_DEVICE_VIRTBLK = 15,
PED_DEVICE_AOE = 16,
- PED_DEVICE_MD = 17
+ PED_DEVICE_MD = 17,
+ PED_DEVICE_LOOP = 18
} PedDeviceType;
typedef struct _PedDevice PedDevice;
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 3792b19..5452f30 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -594,7 +594,7 @@ _device_probe_type (PedDevice* dev)
} else if (_is_virtblk_major(dev_major)) {
dev->type = PED_DEVICE_VIRTBLK;
} else if (dev_major == LOOP_MAJOR) {
- dev->type = PED_DEVICE_FILE;
+ dev->type = PED_DEVICE_LOOP;
} else if (dev_major == MD_MAJOR) {
dev->type = PED_DEVICE_MD;
} else {
@@ -1385,6 +1385,11 @@ linux_new (const char* path)
goto error_free_arch_specific;
break;
+ case PED_DEVICE_LOOP:
+ if (!init_generic (dev, _("Loopback device")))
+ goto error_free_arch_specific;
+ break;
+
case PED_DEVICE_DM:
{
char* type;
diff --git a/parted/parted.c b/parted/parted.c
index 32c2fcc..51ecdaf 100644
--- a/parted/parted.c
+++ b/parted/parted.c
@@ -853,7 +853,7 @@ _print_disk_info (const PedDevice *dev, const PedDisk *disk)
"cpqarray", "file", "ataraid", "i2o",
"ubd", "dasd", "viodasd", "sx8", "dm",
"xvd", "sd/mmc", "virtblk", "aoe",
- "md"};
+ "md", "loopback"};
char* start = ped_unit_format (dev, 0);
PedUnit default_unit = ped_unit_get_default ();
--
1.7.7.rc0.362.g5a14
>From 1eb0cc30d9a2f6a2a6c1f7ec0e4003b1081c5738 Mon Sep 17 00:00:00 2001
From: Petr Uzel <petr.uzel at suse.cz>
Date: Thu, 29 Sep 2011 15:14:23 +0200
Subject: [PATCH 2/3] libparted: improve support for partitions on loopback
devices
Since linux-2.6.26, the kernel allows partitions on loopback devices.
Implement support for this feature in parted.
* libparted/arch/linux.c (_sysfs_int_entry_from_dev): New function.
(_loop_get_partition_range): New function.
(_device_get_partition_range): Add special handling for loop devices.
* NEWS: Mention this change.
---
NEWS | 4 ++
libparted/arch/linux.c | 77 +++++++++++++++++++++++++++++++++++++----------
2 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/NEWS b/NEWS
index 6c55ec9..293dad8 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,10 @@ GNU parted NEWS -*- outline -*-
* Noteworthy changes in release ?.? (????-??-??) [?]
+** New features
+
+ parted has improved support for partitionable loopback devices
+
** Bug fixes
libparted: no longer aborts (failed assertion) due to a nilfs2_probe bug
diff --git a/libparted/arch/linux.c b/libparted/arch/linux.c
index 5452f30..b1c12b5 100644
--- a/libparted/arch/linux.c
+++ b/libparted/arch/linux.c
@@ -2428,32 +2428,75 @@ _blkpg_remove_partition (PedDisk* disk, int n)
BLKPG_DEL_PARTITION);
}
+/* Read the integer from /sys/block/DEV_BASE/ENTRY and set *VAL
+ to that value, where DEV_BASE is the last component of DEV->path.
+ Upon success, return true. Otherwise, return false. */
+static bool
+_sysfs_int_entry_from_dev(PedDevice const* dev, const char *entry, int *val)
+{
+ char path[128];
+ int r = snprintf(path, sizeof(path), "/sys/block/%s/%s",
+ last_component(dev->path), entry);
+ if (r < 0 || r >= sizeof(path))
+ return false;
+
+ FILE *fp = fopen(path, "r");
+ if (!fp)
+ return false;
+
+ bool ok = fscanf(fp, "%d", val) == 1;
+ fclose(fp);
+
+ return ok;
+}
+
+/* Return the maximum number of partitions that the loopback device can hold.
+ First, check the loop-module-exported max_part parameter (since linux-3.0).
+ If that is not available, fall back to checking ext_range, which seems to
+ have (for some reason) different semantics compared to other devices;
+ specifically, ext_range <= 1 means that the loopback device does
+ not support partitions. */
+static unsigned int
+_loop_get_partition_range(PedDevice const* dev)
+{
+ int max_part;
+ bool ok = false;
+
+ /* max_part module param is exported since kernel 3.0 */
+ FILE *fp = fopen("/sys/module/loop/parameters/max_part", "r");
+ if (fp) {
+ ok = fscanf(fp, "%d", &max_part) == 1;
+ fclose(fp);
+ }
+
+ if (ok)
+ return max_part > 0 ? max_part : 0;
+
+ /*
+ * max_part is not exported - check ext_range;
+ * device supports partitions if ext_range > 1
+ */
+ int range;
+ ok = _sysfs_int_entry_from_dev(dev, "range", &range);
+
+ return ok && range > 1 ? range : 0;
+}
+
/*
* The number of partitions that a device can have depends on the kernel.
* If we don't find this value in /sys/block/DEV/range, we will use our own
* value.
*/
static unsigned int
-_device_get_partition_range(PedDevice* dev)
+_device_get_partition_range(PedDevice const* dev)
{
- int range, r;
- char path[128];
- FILE* fp;
- bool ok;
-
- r = snprintf(path, sizeof(path), "/sys/block/%s/range",
- last_component(dev->path));
- if (r < 0 || r >= sizeof(path))
- return MAX_NUM_PARTS;
+ /* loop handling is special */
+ if (dev->type == PED_DEVICE_LOOP)
+ return _loop_get_partition_range(dev);
- fp = fopen(path, "r");
- if (!fp)
- return MAX_NUM_PARTS;
-
- ok = fscanf(fp, "%d", &range) == 1;
- fclose(fp);
+ int range;
+ bool ok = _sysfs_int_entry_from_dev(dev, "range", &range);
- /* (range <= 0) is none sense.*/
return ok && range > 0 ? range : MAX_NUM_PARTS;
}
--
1.7.7.rc0.362.g5a14
>From d079fe7d4a70cc9b30651a560f3dfe749eeb3cff Mon Sep 17 00:00:00 2001
From: Petr Uzel <petr.uzel at suse.cz>
Date: Thu, 29 Sep 2011 15:14:24 +0200
Subject: [PATCH 3/3] tests: add test for partitionable loop devices
* tests/t8001-loop-blkpg.sh: New file.
* tests/Makefile.am: Add test.
---
tests/Makefile.am | 1 +
tests/t8001-loop-blkpg.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 65 insertions(+), 0 deletions(-)
create mode 100755 tests/t8001-loop-blkpg.sh
diff --git a/tests/Makefile.am b/tests/Makefile.am
index e721f88..903ca64 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -43,6 +43,7 @@ TESTS = \
t6000-dm.sh \
t7000-scripting.sh \
t8000-loop.sh \
+ t8001-loop-blkpg.sh \
t9010-big-sector.sh \
t9020-alignment.sh \
t9021-maxima.sh \
diff --git a/tests/t8001-loop-blkpg.sh b/tests/t8001-loop-blkpg.sh
new file mode 100755
index 0000000..a521309
--- /dev/null
+++ b/tests/t8001-loop-blkpg.sh
@@ -0,0 +1,64 @@
+#!/bin/sh
+# Test support for partitions on loop devices
+
+# Copyright (C) 2008-2011 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_
+
+cleanup_fn_()
+{
+ test -n "$loopdev" && losetup -d "$loopdev"
+}
+
+# If the loop module is loaded, unload it first
+if lsmod | grep '^loop[[:space:]]'; then
+ rmmod loop || fail=1
+fi
+
+# Insert loop module with max_part > 1
+modprobe loop max_part=7 || fail=1
+
+# Create backing file
+dd if=/dev/zero of=backing_file bs=1M count=4 >/dev/null 2>&1 || fail=1
+
+# Set up loop device on top of backing file
+loopdev=`losetup -f --show backing_file`
+test -z "$loopdev" && fail=1
+
+# Expect this to succeed
+parted -s "$loopdev" mklabel msdos > err 2>&1 || fail=1
+compare err /dev/null || fail=1 # expect no output
+
+# Create a partition
+parted -s "$loopdev" mkpart primary 1M 2M > err 2>&1 || fail=1
+compare /dev/null err || fail=1 # expect no output
+udevadm settle --timeout=5 || fail=1
+
+# Verify that the partition appeared in /proc/partitions
+entry=`basename "$loopdev"p1`
+grep "$entry" /proc/partitions || fail=1
+
+# Remove the partition
+parted -s "$loopdev" rm 1 > err 2>&1 || fail=1
+compare /dev/null err || fail=1 # expect no output
+udevadm settle --timeout=5 || fail=1
+
+# Verify that the partition got removed from /proc/partitions
+grep "$entry" /proc/partitions && fail=1
+
+Exit $fail
--
1.7.7.rc0.362.g5a14
More information about the parted-devel
mailing list