Skip to contentSkip to conversation

loopdev: sync capacity after setting it

PATCH Superseded Jeff Mahoney 2 participants 7 messages Mar 15, 2013 – Apr 9, 2013

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 loopdev: sync capacity after setting it

I recently tried to mount an hfsplus file system from an image file with a partition table by using the loop offset and sizelimit options to specify the location of the file system.

hfsplus stores some metadata at a set offset from the end of the partition, so it's sensitive to the device size reported by the kernel.

It worked with this:

# losetup -r -o 32k --sizelimit=102400000 <loopdev> <imagefile>
# mount -r -t hfsplus <loopdev> <mountpoint>

But failed with this:

# mount -r -oloop,offset=32k,sizelimit=102400000 <imagefile> <mountpoint>
# losetup -a
/dev/loop0: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000
/dev/loop1: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000

/proc/partitions shows the correct number of blocks to match the sizelimit.

But if I set a breakpoint in mount before the mount syscall, I could see:

# blockdev --getsize64 /dev/loop[01]
102400000
102432768

The kernel loop driver will set the gendisk capacity of the device at LOOP_SET_STATUS64 but won't sync it to the block device until one of two conditions are met: All open file descriptors referring to the device are closed (and it will sync when re-opened) or if the LOOP_SET_CAPACITY ioctl is called to sync it. Since mount opens the device and passes it directly to the mount syscall after LOOP_SET_STATUS64 without closing and reopening it, the sizelimit argument is effectively ignroed. The capacity needs to be synced immediately for it to work as expected.

This patch adds the LOOP_SET_CAPACITY call to loopctx_setup_device since the device isn't yet released to the user, so it's safe to sync the capacity immediately.

---
 lib/loopdev.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)
lib/loopdev.c +12 -0
@@ -1105,6 +1105,18 @@ int loopcxt_setup_device(struct loopdev_
 		goto err;
 	}
 
+	/*
+	 * mount passes the file descriptor without closing it, so
+	 * the capacity isn't updated automatically. File systems
+	 * won't see the new size if it's not the full size of
+	 * the backing file.
+	 */
+	if ((lc->info.lo_offset || lc->info.lo_sizelimit) &&
+	    ioctl(dev_fd, LOOP_SET_CAPACITY, 0)) {
+		DBG(lc, loopdev_debug("LOOP_SET_CAPACITY failed: %m"));
+		goto err;
+	}
+
 	DBG(lc, loopdev_debug("setup: LOOP_SET_STATUS64: OK"));
 
 	memset(&lc->info, 0, sizeof(lc->info));
Signature · 2 lines
Jeff Mahoney
SUSE Labs
#1 lore raw
Quoted 34 lines · Jeff Mahoney

I recently tried to mount an hfsplus file system from an image file with a partition table by using the loop offset and sizelimit options to specify the location of the file system.

hfsplus stores some metadata at a set offset from the end of the partition, so it's sensitive to the device size reported by the kernel.

It worked with this:

# losetup -r -o 32k --sizelimit=102400000 <loopdev> <imagefile>
# mount -r -t hfsplus <loopdev> <mountpoint>

But failed with this:

# mount -r -oloop,offset=32k,sizelimit=102400000 <imagefile> <mountpoint>
# losetup -a
/dev/loop0: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000
/dev/loop1: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000

/proc/partitions shows the correct number of blocks to match the sizelimit.

But if I set a breakpoint in mount before the mount syscall, I could see:

# blockdev --getsize64 /dev/loop[01]
102400000
102432768

The kernel loop driver will set the gendisk capacity of the device at LOOP_SET_STATUS64 but won't sync it to the block device until one of two conditions are met: All open file descriptors referring to the device are closed (and it will sync when re-opened) or if the LOOP_SET_CAPACITY ioctl is called to sync it. Since mount opens the device and passes it directly to the mount syscall after LOOP_SET_STATUS64 without closing and reopening it, the sizelimit argument is effectively ignroed. The capacity needs to be synced immediately for it to work as expected.

Quoted 3 lines

This patch adds the LOOP_SET_CAPACITY call to loopctx_setup_device since the device isn't yet released to the user, so it's safe to sync the capacity immediately.

It turns out this ioctl wasn't introduced until 2.6.30. I'll fix that up and resend tomorrow.

-Jeff

Quoted 27 lines
---
  lib/loopdev.c |   12 ++++++++++++
  1 file changed, 12 insertions(+)
lib/loopdev.c +12 -0
@@ -1105,6 +1105,18 @@ int loopcxt_setup_device(struct loopdev_
  		goto err;
  	}
 
+	/*
+	 * mount passes the file descriptor without closing it, so
+	 * the capacity isn't updated automatically. File systems
+	 * won't see the new size if it's not the full size of
+	 * the backing file.
+	 */
+	if ((lc->info.lo_offset || lc->info.lo_sizelimit) &&
+	    ioctl(dev_fd, LOOP_SET_CAPACITY, 0)) {
+		DBG(lc, loopdev_debug("LOOP_SET_CAPACITY failed: %m"));
+		goto err;
+	}
+
  	DBG(lc, loopdev_debug("setup: LOOP_SET_STATUS64: OK"));
 
  	memset(&lc->info, 0, sizeof(lc->info));
Signature · 2 lines
Jeff Mahoney
SUSE Labs
maintainer
#2 lore raw
Quoted 35 lines · Jeff Mahoney
Quoted 34 lines · Jeff Mahoney

I recently tried to mount an hfsplus file system from an image file with a partition table by using the loop offset and sizelimit options to specify the location of the file system.

hfsplus stores some metadata at a set offset from the end of the partition, so it's sensitive to the device size reported by the kernel.

It worked with this:

# losetup -r -o 32k --sizelimit=102400000 <loopdev> <imagefile>
# mount -r -t hfsplus <loopdev> <mountpoint>

But failed with this:

# mount -r -oloop,offset=32k,sizelimit=102400000 <imagefile> <mountpoint>
# losetup -a
/dev/loop0: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000
/dev/loop1: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000

/proc/partitions shows the correct number of blocks to match the sizelimit.

But if I set a breakpoint in mount before the mount syscall, I could see:

# blockdev --getsize64 /dev/loop[01]
102400000
102432768

The kernel loop driver will set the gendisk capacity of the device at LOOP_SET_STATUS64 but won't sync it to the block device until one of two conditions are met: All open file descriptors referring to the device are closed (and it will sync when re-opened) or if the LOOP_SET_CAPACITY ioctl is called to sync it. Since mount opens the device and passes it directly to the mount syscall after LOOP_SET_STATUS64 without closing and reopening it, the sizelimit argument is effectively ignroed. The capacity needs to be synced immediately for it to work as expected.

Quoted 6 lines
Quoted 3 lines

This patch adds the LOOP_SET_CAPACITY call to loopctx_setup_device since the device isn't yet released to the user, so it's safe to sync the capacity immediately.

It turns out this ioctl wasn't introduced until 2.6.30. I'll fix that up and resend tomorrow.

Do you mean #ifdef LOOP_SET_CAPACITY? I can fix the patch manually.

(It seem that we already use this ioctl in code without any extra
care about old kernel and nobody complains :-)

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com
#3 lore raw
Quoted 40 lines · Karel Zak
Quoted 39 lines · Jeff Mahoney
Quoted 37 lines · Jeff Mahoney

I recently tried to mount an hfsplus file system from an image file with a partition table by using the loop offset and sizelimit options to specify the location of the file system.

hfsplus stores some metadata at a set offset from the end of the partition, so it's sensitive to the device size reported by the kernel.

It worked with this:

# losetup -r -o 32k --sizelimit=102400000 <loopdev> <imagefile>
# mount -r -t hfsplus <loopdev> <mountpoint>

But failed with this:

# mount -r -oloop,offset=32k,sizelimit=102400000 <imagefile> <mountpoint>
# losetup -a
/dev/loop0: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000
/dev/loop1: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000

/proc/partitions shows the correct number of blocks to match the sizelimit.

But if I set a breakpoint in mount before the mount syscall, I could see:

# blockdev --getsize64 /dev/loop[01]
102400000
102432768

The kernel loop driver will set the gendisk capacity of the device at LOOP_SET_STATUS64 but won't sync it to the block device until one of two conditions are met: All open file descriptors referring to the device are closed (and it will sync when re-opened) or if the LOOP_SET_CAPACITY ioctl is called to sync it. Since mount opens the device and passes it directly to the mount syscall after LOOP_SET_STATUS64 without closing and reopening it, the sizelimit argument is effectively ignroed. The capacity needs to be synced immediately for it to work as expected.

This patch adds the LOOP_SET_CAPACITY call to loopctx_setup_device since the device isn't yet released to the user, so it's safe to sync the capacity immediately.

Quoted 7 lines
Quoted 2 lines

It turns out this ioctl wasn't introduced until 2.6.30. I'll fix that up and resend tomorrow.

Do you mean #ifdef LOOP_SET_CAPACITY? I can fix the patch manually.

(It seem that we already use this ioctl in code without any extra
care about old kernel and nobody complains :-)

Yeah, but that's in losetup --set-capacity where it's an explicit operation. This change will add the ioctl into every loopdev_setup_device call when the offset or sizelimit options are used. If it isn't supported by the kernel, the ioctl will fail silently and *maybe* the mount will fail, but that's totally dependent on the the file system. If the mount succeeds, it will be done outside of the parameters the user requested.

So, all I really want to do is dump an error message when the ioctl fails with -ENOTTY || -EINVAL about there being a lack of kernel support. We shouldn't allow the device configuration to proceed.

The part that makes it more "fun" is that a few patches in 3.9-rc1 fixed this in the kernel, so it won't actually be needed for new kernels.

-Jeff

Signature · 2 lines
Jeff Mahoney
SUSE Labs
maintainer
#4 lore raw
Quoted 8 lines · Jeff Mahoney

Yeah, but that's in losetup --set-capacity where it's an explicit operation. This change will add the ioctl into every loopdev_setup_device call when the offset or sizelimit options are used. If it isn't supported by the kernel, the ioctl will fail silently and *maybe* the mount will fail, but that's totally dependent on the the file system. If the mount succeeds, it will be done outside of the parameters the user requested.

Quoted 6 lines

So, all I really want to do is dump an error message when the ioctl fails with -ENOTTY || -EINVAL about there being a lack of kernel support. We shouldn't allow the device configuration to proceed.

The part that makes it more "fun" is that a few patches in 3.9-rc1 fixed this in the kernel, so it won't actually be needed for new kernels.

Nasty... well, send patch.

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com
#5 lore raw

PATCH v2 1/1 loopdev: sync capacity after setting it

I recently tried to mount an hfsplus file system from an image file with a partition table by using the loop offset and sizelimit options to specify the location of the file system.

hfsplus stores some metadata at a set offset from the end of the partition, so it's sensitive to the device size reported by the kernel.

It worked with this:

# losetup -r -o 32k --sizelimit=102400000 <loopdev> <imagefile>
# mount -r -t hfsplus <loopdev> <mountpoint>

But failed with this:

# mount -r -oloop,offset=32k,sizelimit=102400000 <imagefile> <mountpoint>
# losetup -a
/dev/loop0: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000
/dev/loop1: [0089]:2 (<imagefile>), offset 32768, sizelimit 102400000

/proc/partitions shows the correct number of blocks to match the sizelimit.

But if I set a breakpoint in mount before the mount syscall, I could see:

# blockdev --getsize64 /dev/loop[01]
102400000
102432768

The kernel loop driver will set the gendisk capacity of the device at LOOP_SET_STATUS64 but won't sync it to the block device until one of two conditions are met: All open file descriptors referring to the device are closed (and it will sync when re-opened) or if the LOOP_SET_CAPACITY ioctl is called to sync it. Since mount opens the device and passes it directly to the mount syscall after LOOP_SET_STATUS64 without closing and reopening it, the sizelimit argument is effectively ignroed. The capacity needs to be synced immediately for it to work as expected.

This patch adds the LOOP_SET_CAPACITY call to loopctx_setup_device since the device isn't yet released to the user, so it's safe to sync the capacity immediately.

---
 include/loopdev.h        |    1 
 lib/Makefile.am          |    4 +-
 lib/blkdev.c             |    2 -
 lib/loopdev.c            |   89 +++++++++++++++++++++++++++++++++++++++++++++--
 libmount/src/Makefile.am |    1 
 mount/mount.8            |   16 ++++++++
 sys-utils/Makefile.am    |    3 +
 sys-utils/losetup.c      |   19 ++--------
 8 files changed, 114 insertions(+), 21 deletions(-)
include/loopdev.h +1 -0
@@ -160,6 +160,7 @@ extern int loopcxt_next(struct loopdev_c
 
 extern int loopcxt_setup_device(struct loopdev_cxt *lc);
 extern int loopcxt_delete_device(struct loopdev_cxt *lc);
+extern int loopcxt_set_capacity(struct loopdev_cxt *lc);
 
 int loopcxt_set_offset(struct loopdev_cxt *lc, uint64_t offset);
 int loopcxt_set_sizelimit(struct loopdev_cxt *lc, uint64_t sizelimit);
lib/Makefile.am +3 -1
@@ -13,6 +13,7 @@ noinst_PROGRAMS += test_sysfs test_loopd
 endif
 
 test_blkdev_SOURCES = blkdev.c
+test_blkdev_CFLAGS = -DTEST_PROGRAM_BLKDEV
 test_ismounted_SOURCES = ismounted.c
 test_wholedisk_SOURCES = wholedisk.c
 test_mangle_SOURCES = mangle.c
@@ -31,7 +32,8 @@ test_sysfs_CFLAGS = -DTEST_PROGRAM_SYSFS
 test_loopdev_SOURCES = loopdev.c \
 		       $(test_sysfs_SOURCES) \
 		       $(top_srcdir)/lib/linux_version.c \
-		       $(top_srcdir)/lib/canonicalize.c
+		       $(top_srcdir)/lib/canonicalize.c \
+		       $(top_srcdir)/lib/blkdev.c
 
 test_loopdev_CFLAGS = -DTEST_PROGRAM_LOOPDEV
 endif
lib/blkdev.c +1 -1
@@ -263,7 +263,7 @@ int blkdev_is_cdrom(int fd)
 #endif
 }
 
-#ifdef TEST_PROGRAM
+#ifdef TEST_PROGRAM_BLKDEV
 #include <stdio.h>
 #include <stdlib.h>
 #include <fcntl.h>
lib/loopdev.c +86 -3

119 lines not on this page · raw message

libmount/src/Makefile.am +1 -0
@@ -23,6 +23,7 @@ libmount_la_SOURCES =	mountP.h version.c
 			$(top_srcdir)/lib/strutils.c \
 			$(top_srcdir)/lib/env.c \
 			$(top_srcdir)/lib/loopdev.c \
+			$(top_srcdir)/lib/blkdev.c \
 			$(top_srcdir)/lib/sysfs.c \
 			$(top_srcdir)/lib/linux_version.c
 
mount/mount.8 +16 -0
@@ -2844,6 +2844,22 @@ and
 .BR ioctl
 families of functions) may lead to inconsistent result due to the lack of
 consistency check in kernel even if noac is used.
+.PP
+The
+.B loop
+option with the
+.B offset
+or
+.B sizelimit
+options used may fail when using older kernels if the
+.B mount
+command can't confirm that the size of the block device has been configured
+as requested. This situation can be worked around by using
+the
+.B losetup
+command manually before calling
+.B mount
+with the configured loop device.
 .SH HISTORY
 A
 .B mount
sys-utils/Makefile.am +2 -1
@@ -34,7 +34,8 @@ losetup_SOURCES = losetup.c \
 		$(top_srcdir)/lib/loopdev.c \
 		$(top_srcdir)/lib/canonicalize.c \
 		$(top_srcdir)/lib/xgetpass.c \
-		$(top_srcdir)/lib/strutils.c
+		$(top_srcdir)/lib/strutils.c \
+		$(top_srcdir)/lib/blkdev.c
 
 if HAVE_STATIC_LOSETUP
 bin_PROGRAMS += losetup.static
sys-utils/losetup.c +4 -15
@@ -107,20 +107,6 @@ static int show_all_loops(struct loopdev
 	return 0;
 }
 
-static int set_capacity(struct loopdev_cxt *lc)
-{
-	int fd = loopcxt_get_fd(lc);
-
-	if (fd < 0)
-		warn(_("%s: open failed"), loopcxt_get_device(lc));
-	else if (ioctl(fd, LOOP_SET_CAPACITY) != 0)
-		warnx(_("%s: set capacity failed"), loopcxt_get_device(lc));
-	else
-		return 0;
-
-	return -1;
-}
-
 static int delete_loop(struct loopdev_cxt *lc)
 {
 	if (loopcxt_delete_device(lc))
@@ -394,7 +380,10 @@ int main(int argc, char **argv)
 			warn(_("%s"), loopcxt_get_device(&lc));
 		break;
 	case A_SET_CAPACITY:
-		res = set_capacity(&lc);
+		res = loopcxt_set_capacity(&lc);
+		if (res)
+			warn(_("%s: set capacity failed"),
+			        loopcxt_get_device(&lc));
 		break;
 	default:
 		usage(stderr);
Signature · 2 lines
Jeff Mahoney
SUSE Labs
maintainer
#6 lore raw
Quoted 9 lines · Jeff Mahoney
 include/loopdev.h        |    1 
 lib/Makefile.am          |    4 +-
 lib/blkdev.c             |    2 -
 lib/loopdev.c            |   89 +++++++++++++++++++++++++++++++++++++++++++++--
 libmount/src/Makefile.am |    1 
 mount/mount.8            |   16 ++++++++
 sys-utils/Makefile.am    |    3 +
 sys-utils/losetup.c      |   19 ++--------
 8 files changed, 114 insertions(+), 21 deletions(-)

It seems you have some very old version (or stable/ branch? :-) I have ported the patch to the current git master and merged.

Thanks!

Karel

Signature · 2 lines
 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com