Fix comparison. #742

akry · 2015-10-01T09:24:20Z

#741

axsh-bot · 2015-10-01T09:26:49Z

✅ be863c3 success - wakame-ci/rspec

axsh-bot · 2015-10-01T09:36:23Z

✅ be863c3 success - wakame-ci/cli/vdc-manage

axsh-bot · 2015-10-01T09:37:20Z

✅ be863c3 success - wakame-ci/cli/backup-cleaner

axsh-bot · 2015-10-01T09:53:22Z

✅ be863c3 success - wakame-ci/rpmbuild

axsh-bot · 2015-10-01T10:07:10Z

❌ be863c3 failure - wakame-ci/yumrepo/to-dropbox

axsh-bot · 2015-10-01T10:07:24Z

❌ be863c3 failure - wakame-ci/yumrepo/to-s3

triggers · 2015-10-02T15:18:35Z

find_loopdev(path) just seems to implement the -j option of losetup. (Maybe that option did not used to exist on losetup?) So something like this is an alternative:

losetup -j "$path" | cut -d: -f 1 | head -n 1

akry · 2015-10-07T03:30:29Z

[root@dev ~]# cat /etc/redhat-release
CentOS release 6.4 (Final)
[root@dev ~]# losetup -j
losetup: option requires an argument -- 'j'

Usage:
 losetup loop_device                             give info
 losetup -a | --all                              list all used
 losetup -d | --detach <loopdev> [<loopdev> ...] delete
 losetup -f | --find                             find unused
 losetup -c | --set-capacity <loopdev>           resize
 losetup -j | --associated <file> [-o <num>]     list all associated with <file>
 losetup [ options ] {-f|--find|loopdev} <file>  setup

Options:
 -e | --encryption <type> enable data encryption with specified <name/num>
 -h | --help              this help
 -o | --offset <num>      start at offset <num> into file
      --sizelimit <num>   loop limited to only <num> bytes of the file
 -p | --pass-fd <num>     read passphrase from file descriptor <num>
 -r | --read-only         setup read-only loop device
      --show              print device name (with -f <file>)
 -v | --verbose           verbose mode

akry · 2015-10-07T03:33:05Z

[root@dev ~]# cat /etc/redhat-release
CentOS release 6.5 (Final)
[root@dev ~]# losetup -j
losetup: option requires an argument -- 'j'

Usage:
 losetup loop_device                             give info
 losetup -a | --all                              list all used
 losetup -d | --detach <loopdev> [<loopdev> ...] delete
 losetup -f | --find                             find unused
 losetup -c | --set-capacity <loopdev>           resize
 losetup -j | --associated <file> [-o <num>]     list all associated with <file>
 losetup [ options ] {-f|--find|loopdev} <file>  setup

Options:
 -e | --encryption <type> enable data encryption with specified <name/num>
 -h | --help              this help
 -o | --offset <num>      start at offset <num> into file
      --sizelimit <num>   loop limited to only <num> bytes of the file
 -p | --pass-fd <num>     read passphrase from file descriptor <num>
 -r | --read-only         setup read-only loop device
      --show              print device name (with -f <file>)
 -v | --verbose           verbose mode
[root@dev ~]# rpm -qf /sbin/losetup
util-linux-ng-2.17.2-12.14.el6.x86_64

akry · 2015-10-07T03:36:30Z

[root@dev ~]# cat /etc/redhat-release
CentOS release 6.6 (Final)
[root@dev ~]# losetup -j
losetup: option requires an argument -- 'j'

Usage:
 losetup loop_device                             give info
 losetup -a | --all                              list all used
 losetup -d | --detach <loopdev> [<loopdev> ...] delete
 losetup -f | --find                             find unused
 losetup -c | --set-capacity <loopdev>           resize
 losetup -j | --associated <file> [-o <num>]     list all associated with <file>
 losetup [ options ] {-f|--find|loopdev} <file>  setup

Options:
 -e | --encryption <type> enable data encryption with specified <name/num>
 -h | --help              this help
 -o | --offset <num>      start at offset <num> into file
      --sizelimit <num>   loop limited to only <num> bytes of the file
 -p | --pass-fd <num>     read passphrase from file descriptor <num>
 -r | --read-only         setup read-only loop device
      --show              print device name (with -f <file>)
 -v | --verbose           verbose mode

[root@dev ~]# rpm -qf /sbin/losetup
util-linux-ng-2.17.2-12.18.el6.x86_64

akry · 2015-10-07T03:39:19Z

[root@dev ~]# cat /etc/redhat-release
CentOS release 6.7 (Final)
[root@dev ~]# losetup -j
losetup: option requires an argument -- 'j'

Usage:
 losetup loop_device                             give info
 losetup -a | --all                              list all used
 losetup -d | --detach <loopdev> [<loopdev> ...] delete
 losetup -f | --find                             find unused
 losetup -c | --set-capacity <loopdev>           resize
 losetup -j | --associated <file> [-o <num>]     list all associated with <file>
 losetup [ options ] {-f|--find|loopdev} <file>  setup

Options:
 -e | --encryption <type> enable data encryption with specified <name/num>
 -h | --help              this help
 -o | --offset <num>      start at offset <num> into file
      --sizelimit <num>   loop limited to only <num> bytes of the file
 -p | --pass-fd <num>     read passphrase from file descriptor <num>
 -r | --read-only         setup read-only loop device
      --show              print device name (with -f <file>)
 -v | --verbose           verbose mode

[root@dev ~]# rpm -qf /sbin/losetup
util-linux-ng-2.17.2-12.18.el6.x86_64

akry · 2015-10-07T03:46:17Z

[root@dev ~]# cat /etc/redhat-release
CentOS Linux release 7.1.1503 (Core)
[root@dev ~]# losetup -j
losetup: option requires an argument -- 'j'

Usage:
 losetup [options] [<loopdev>]
 losetup [options] -f | <loopdev> <file>

Options:
 -a, --all                     list all used devices
 -d, --detach <loopdev> [...]  detach one or more devices
 -D, --detach-all              detach all used devices
 -f, --find                    find first unused device
 -c, --set-capacity <loopdev>  resize device
 -j, --associated <file>       list all devices associated with <file>

 -l, --list                    list info about all or specified
 -o, --offset <num>            start at offset <num> into file
 -O, --output <cols>           specify columns to output for --list
     --sizelimit <num>         device limited to <num> bytes of the file
 -P, --partscan                create partitioned loop device
 -r, --read-only               setup read-only loop device
     --show                    print device name after setup (with -f)
 -v, --verbose                 verbose mode

 -h, --help     display this help and exit
 -V, --version  output version information and exit

Available --list columns:
         NAME  loop device name
    AUTOCLEAR  autoclear flag set
    BACK-FILE  device backing file
     BACK-INO  backing file inode number
 BACK-MAJ:MIN  backing file major:minor device number
      MAJ:MIN  loop device major:minor number
       OFFSET  offset from the beginning
     PARTSCAN  partscan flag set
           RO  read-only device
    SIZELIMIT  size limit of the file in bytes

For more details see losetup(8).
[root@dev ~]# rpm -qf /sbin/losetup
util-linux-2.23.2-22.el7_1.x86_64

akry · 2015-10-07T04:29:09Z

[root@localhost dcmgr]# losetup -j /var/lib/wakame-vdc/instances/i-mdfb2bla/metadata.img
[root@localhost dcmgr]# losetup -a
/dev/loop0: [2051]:141745283 (/var/lib/wakame-vdc/instances/i-1wllauqo/metadata.img (deleted))
/dev/loop1: [2051]:2382902 (/var/lib/wakame-vdc/instances/i-3rq0dr2o/metadata.img (deleted))
/dev/loop2: [2051]:71558156 (/var/lib/wakame-vdc/instances/i-owtdkugk/metadata.img (deleted))
/dev/loop3: [2051]:137275487 (/var/lib/wakame-vdc/instances/i-mdfb2bla/metadata.img (deleted))

akry · 2015-10-07T04:29:44Z

losetup -j doesn't show anything because the target file is already deleted.

akry · 2015-10-07T04:51:58Z

c3e5334

This has more optimistic way than the current implementation. Is the strict checking really necessary?

triggers · 2015-10-07T07:53:34Z

The code change looks good to me. Not sure what to think about the issues you bring up above about "already deleted" and "strict checking". (The old code should also have the same problem if the file is already deleted) (What do you mean by "more optimistic"?)

In the bigger picture... I suspect that using kpartx and all these workarounds is not the best solution. Usually we only want to mount one partition, and losetup can do that by itself, if given the proper offset. Then we avoid using device-mapper and invoking kpartx udev rules, so we avoid potential problems from this behind-the-scenes complexity. But that is probably a bigger change than we want to do now.

For reference, see the rambling comment in here, which I should probably edit soon:
https://github.com/triggers/mount-partition/blob/master/mount-partition.sh

So for this little change:
+1

akry · 2015-10-07T08:10:40Z

"strict checking" is just my personal point of view. The old code checks the file system identifier and the inode number that are derived from a target file, in turn my change only checks the file path.