Re: [PATCH] loopdev: sync capacity after setting it

[Jeff Mahoney <jeffm@suse.com>]

[-- Attachment #1: Type: text/plain, Size: 3105 bytes --]

On 3/18/13 5:27 AM, Karel Zak wrote:
> On Sun, Mar 17, 2013 at 10:18:28AM -0400, Jeff Mahoney wrote:
>> On 03/15/2013 05:04 PM, Jeff Mahoney wrote:
>>> 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.
>>
>> 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

-- 
Jeff Mahoney
SUSE Labs


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 881 bytes --]