Re: [dm-devel] [PATCH 00/10] fixes for kpartx -d

2017-05-05 Thread Martin Wilck 
On Sat, 2017-05-06 at 00:30 +0200, Martin Wilck wrote:
> On Fri, 2017-05-05 at 23:18 +0100, Alasdair G Kergon wrote:
> > On Sat, May 06, 2017 at 12:05:49AM +0200, Martin Wilck wrote:
> > >   3) kpartx should only delete "partitions", which are single-
> > > target
> > >  linear mappings into a block device. Other maps should not
> > > be
> > > touched.
> > 
> > The prefix on the dm device's uuid should guarantee this: all
> > devices
> > kpartx creates should have the same initial characters (a
> > not-quite-standard form "part" IIRC instead of "KPARTX-") and any
> > devices without those initial characters must be ignored.
> 
> This works only for partitions on DM devices, not e.g. for loop
> devices. These devices obviously have no DM UUID; and thus kpartx
> also
> doesn't set an UUID for the partition devices it creates.
> That's the main point of this series.

Moreover: before even looking at the UUID, kpartx discards mappings
that are not "linear" and don't map into the device in question. I
added the additional case that it should also disregard mappings with
two or more targets which can't be regarded as simple "linear" type
(but in the current code, they are).

With the UUID test in place, this may seem kind of redundant, but it
follows the general logic that obvious non-partition devices should be
discarded before checking the UUID.

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 00/10] fixes for kpartx -d

2017-05-05 Thread Martin Wilck 
On Fri, 2017-05-05 at 23:18 +0100, Alasdair G Kergon wrote:
> On Sat, May 06, 2017 at 12:05:49AM +0200, Martin Wilck wrote:
> >   3) kpartx should only delete "partitions", which are single-
> > target
> >  linear mappings into a block device. Other maps should not be
> > touched.
> 
> The prefix on the dm device's uuid should guarantee this: all devices
> kpartx creates should have the same initial characters (a
> not-quite-standard form "part" IIRC instead of "KPARTX-") and any
> devices without those initial characters must be ignored.

This works only for partitions on DM devices, not e.g. for loop
devices. These devices obviously have no DM UUID; and thus kpartx also
doesn't set an UUID for the partition devices it creates.
That's the main point of this series.

Martin

-- 
Dr. Martin Wilck , Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 08/10] kpartx: use partition UUID for non-DM devices

2017-05-05 Thread Alasdair G Kergon 
On Sat, May 06, 2017 at 12:05:57AM +0200, Martin Wilck wrote:
> Introduce a "fake" UUID for these devices to make sure kpartx
> deletes only devices it had created previously. Otherwise kpartx
> might e.g. delete LVM LVs that are inside a device it is trying
> to delete partitions for. It seems to be wiser to make sure the
> user delete these manually before running "kpartx -d".
> 
> With the fake UUID in place, we can re-introduce the UUID check
> for non-DM device that was removed in the earlier patch
> "kpartx: dm_remove_partmaps: support non-dm devices".
> 
> This disables also deletion of partition mappings created
> by earlier versions of kpartx. If kpartx has been updated after
> partition mappings were created, the "-f" flag can be used
> to force delting these partitions, too.
 
Indeed - always supply a uuid for every dm device created.
Add a standard prefix to the beginning of it, ideally "KPARTX-" to be
consistent with other dm tools ("LVM-", "CRYPT-" etc.) if you're
able to break compatibility, but "part" is still unique and adequate.  
(Stacked devices can end up with a stack of prefixes - only the leftmost one
counts.)

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 00/10] fixes for kpartx -d

2017-05-05 Thread Alasdair G Kergon 
On Sat, May 06, 2017 at 12:05:49AM +0200, Martin Wilck wrote:
>   3) kpartx should only delete "partitions", which are single-target
>  linear mappings into a block device. Other maps should not be touched.

The prefix on the dm device's uuid should guarantee this: all devices
kpartx creates should have the same initial characters (a
not-quite-standard form "part" IIRC instead of "KPARTX-") and any
devices without those initial characters must be ignored.

Alasdair

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 03/10] kpartx: remove is_loop_device

2017-05-05 Thread Martin Wilck 
This function is not used any more.

Signed-off-by: Martin Wilck 
---
 kpartx/lopart.c | 31 ---
 kpartx/lopart.h |  1 -
 2 files changed, 32 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 2eb3f631..44f0c277 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -62,37 +62,6 @@ xstrdup (const char *s)
return t;
 }
 
-int is_loop_device(const char *device)
-{
-   struct stat statbuf;
-   int loopmajor;
-#if 1
-   loopmajor = 7;
-#else
-   FILE *procdev;
-   char line[100], *cp;
-
-   loopmajor = 0;
-
-   if ((procdev = fopen(PROC_DEVICES, "r")) != NULL) {
-
-   while (fgets (line, sizeof(line), procdev)) {
-
-   if ((cp = strstr (line, " loop\n")) != NULL) {
-   *cp='\0';
-   loopmajor=atoi(line);
-   break;
-   }
-   }
-
-   fclose(procdev);
-   }
-#endif
-   return (loopmajor && stat(device, ) == 0 &&
-   S_ISBLK(statbuf.st_mode) &&
-   major(statbuf.st_rdev) == loopmajor);
-}
-
 #define SIZE(a) (sizeof(a)/sizeof(a[0]))
 
 char *find_loop_by_file(const char *filename)
diff --git a/kpartx/lopart.h b/kpartx/lopart.h
index a512353b..d3bad10a 100644
--- a/kpartx/lopart.h
+++ b/kpartx/lopart.h
@@ -1,6 +1,5 @@
 extern int verbose;
 extern int set_loop (const char *, const char *, int, int *);
 extern int del_loop (const char *);
-extern int is_loop_device (const char *);
 extern char * find_unused_loop_device (void);
 extern char * find_loop_by_file (const char *);
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 08/10] kpartx: use partition UUID for non-DM devices

2017-05-05 Thread Martin Wilck 
For dm devices, kpartx uses an UUID check at partition removal
to make sure it only deletes partitions it previously created.
Non-DM parent devices such as loop devices don't generally have
a UUID.

Introduce a "fake" UUID for these devices to make sure kpartx
deletes only devices it had created previously. Otherwise kpartx
might e.g. delete LVM LVs that are inside a device it is trying
to delete partitions for. It seems to be wiser to make sure the
user delete these manually before running "kpartx -d".

With the fake UUID in place, we can re-introduce the UUID check
for non-DM device that was removed in the earlier patch
"kpartx: dm_remove_partmaps: support non-dm devices".

This disables also deletion of partition mappings created
by earlier versions of kpartx. If kpartx has been updated after
partition mappings were created, the "-f" flag can be used
to force delting these partitions, too.

Signed-off-by: Martin Wilck 
---
 kpartx/devmapper.c |  2 +-
 kpartx/kpartx.c| 29 +
 2 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 5380b2e9..40ec26cf 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -505,7 +505,7 @@ do_foreach_partmaps (const char * mapname, const char *uuid,
/*
 * skip if uuids don't match
 */
-   if (is_dmdev && uuid && dm_compare_uuid(uuid, names->name)) {
+   if (uuid && dm_compare_uuid(uuid, names->name)) {
if (rd->verbose)
printf("%s: is not a kpartx partition. Not 
removing\n",
   names->name);
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index e2056b7f..d9057fba 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -60,6 +60,26 @@ struct pt {
 int ptct = 0;
 int udev_sync = 0;
 
+/*
+ * UUID format for partitions created on non-DM devices
+ * ${UUID_PREFIX}${MAJOR}:${MINOR}-${NONDM_UUID_SUFFIX}"
+ * The suffix should be sufficiently random to avoid unintended conflicts,
+ * and shouldn't be changed.
+ * The value below is a bas64-encoded 96-bit random number.
+ */
+#define NONDM_UUID_SUFFIX "-kpartx-Wh5pYvM7uc60hh74"
+
+static char *
+nondm_create_uuid(dev_t devt)
+{
+#define NONDM_UUID_BUFLEN (32 + sizeof(NONDM_UUID_SUFFIX))
+   static char uuid_buf[NONDM_UUID_BUFLEN];
+   snprintf(uuid_buf, sizeof(uuid_buf), "%u:%u%s",
+major(devt), minor(devt), NONDM_UUID_SUFFIX);
+   uuid_buf[NONDM_UUID_BUFLEN-1] = '\0';
+   return uuid_buf;
+}
+
 static void
 addpts(char *t, ptreader f)
 {
@@ -360,6 +380,15 @@ main(int argc, char **argv){
uuid = dm_mapuuid(mapname);
}
 
+   /*
+* We are called for a non-DM device.
+* Make up a fake UUID for the device, unless "-d -f" is given.
+* This allows deletion of partitions created with older kpartx
+* versions which didn't use the fake UUID during creation.
+*/
+   if (!uuid && !(what == DELETE && force_devmap))
+   uuid = nondm_create_uuid(buf.st_rdev);
+
if (!mapname)
mapname = device + off;
else if (!force_devmap &&
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 02/10] kpartx: avoid ioctl error for loop devices

2017-05-05 Thread Martin Wilck 
Commit 3d709241 causes kpartx to attempt a dm ioctl on a loop
device. This causes an error message
"device-mapper: table ioctl on loop4 failed: No such device or address".

Fixes: 3d709241 "kpartx: sanitize delete partitions"
Signed-off-by: Martin Wilck 
---
 kpartx/kpartx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index 58e60ffe..e9b09492 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -362,7 +362,7 @@ main(int argc, char **argv){
 
if (!mapname)
mapname = device + off;
-   if (!force_devmap &&
+   else if (!force_devmap &&
 dm_no_partitions(mapname)) {
/* Feature 'no_partitions' is set, return */
return 0;
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 01/10] kpartx: test-kpartx: new unit test program

2017-05-05 Thread Martin Wilck 
This is a unit test program for kpartx, in particular for deleting
partitions.

NOTE: This test program fails with current kpartx; full patch series
needed to make it work.

Signed-off-by: Martin Wilck 
---
 kpartx/test-kpartx | 253 +
 1 file changed, 253 insertions(+)
 create mode 100755 kpartx/test-kpartx

diff --git a/kpartx/test-kpartx b/kpartx/test-kpartx
new file mode 100755
index ..03e8030e
--- /dev/null
+++ b/kpartx/test-kpartx
@@ -0,0 +1,253 @@
+#! /bin/bash
+
+# This is a unit test program for kpartx, in particular for deleting 
partitions.
+#
+# The rationale is the following:
+#
+#  1) kpartx should delete all mappings it created beforehand.
+#  2) kpartx should handle partitions on dm devices and other devices
+# (e.g. loop devices) equally well.
+#  3) kpartx should only delete "partitions", which are single-target
+# linear mappings into a block device. Other maps should not be touched.
+#  4) kpartx should only delete mappings it created itself beforehand.
+# In particular, it shouldn't delete LVM LVs, even if they are fully
+# contained in the block device at hand and thus look like partitions
+# in the first place. (For historical compatibility reasons, we allow
+# such mappings to be deleted with the -f/--force flag).
+#  5) DM map names may be changed, thus kpartx shouldn't rely on them to
+# check whether a mapping is a partition of a particular device. It is
+# legal for a partition of /dev/loop0 to be named "loop0".
+
+# Note: This program tries hard to clean up, but if tests fail,
+# stale DM or loop devices may keep lurking around.
+
+# Set WORKDIR in environment to existing dir to for persistence
+# WARNING:  exisiting files will be truncated.
+# If empty, test will be done in temporary dir
+: ${WORKDIR:=}
+# Set this environment variable to test an alternative kpartx executable
+: ${KPARTX:=}
+# Time to wait for device nodes to appear (microseconds)
+: ${WAIT_US:=10}
+
+# IMPORTANT: The ERR trap is essential for this program to work correctly!
+trap 'LINE=$LINENO; trap - ERR; echo "== error in $BASH_COMMAND on line $LINE 
==" >&2; exit 1' ERR
+trap 'cleanup' 0
+
+CLEANUP=:
+cleanup() {
+trap - ERR
+trap - 0
+if [[ $OK ]]; then
+   echo == all tests completed successfully == >&2
+else
+   echo == step $STEP failed == >&2
+fi
+eval "$CLEANUP" &>/dev/null
+}
+
+push_cleanup() {
+CLEANUP="$@;$CLEANUP"
+}
+
+pop_cleanup() {
+# CAUTION: simplistic
+CLEANUP=${CLEANUP#*;}
+}
+
+step() {
+STEP="$@"
+echo == Test step: $STEP == >&2
+}
+
+mk_partitions() {
+parted -s $1 mklabel msdos
+parted -s -- $1 mkpart prim ext2 1MiB -1s
+}
+
+step preparation
+
+[[ $UID -eq 0 ]]
+[[ $KPARTX ]] || {
+if [[ -x $PWD/kpartx/kpartx ]]; then
+   KPARTX=$PWD/kpartx/kpartx
+else
+   KPARTX=$(which kpartx)
+fi
+}
+[[ $KPARTX ]]
+
+FILE1=kpartx1
+FILE2=kpartx2
+FILE3=kpartx3
+SIZE=$((1024*1024*1024))  # use bytes as units here
+SECTSIZ=512
+OFFS=32# offset of linear mapping into dev, sectors
+VG=kpvg  # volume group name
+LV=kplv  # logical vol name
+LVMCONF='devices { filter = [ "a|/dev/loop.*|", r".*" ] }'
+
+OK=
+
+[[ $WORKDIR ]] || {
+WORKDIR=$(mktemp -d /tmp/kpartx-XX)
+push_cleanup 'rm -rf $WORKDIR'
+}
+
+push_cleanup "cd $PWD"
+cd "$WORKDIR"
+
+step "create loop devices"
+truncate -s $SIZE $FILE1
+truncate -s $SIZE $FILE2
+truncate -s $SIZE $FILE3
+
+LO1=$(losetup -f $FILE1 --show)
+push_cleanup 'losetup -d $LO1'
+LO2=$(losetup -f $FILE2 --show)
+push_cleanup 'losetup -d $LO2'
+LO3=$(losetup -f $FILE3 --show)
+push_cleanup 'losetup -d $LO3'
+
+[[ $LO1 && $LO2 && $LO3 && -b $LO1 && -b $LO2 && -b $LO3 ]]
+DEV1=$(stat -c "%t:%T" $LO1)
+DEV2=$(stat -c "%t:%T" $LO2)
+DEV3=$(stat -c "%t:%T" $LO3)
+
+usleep $WAIT_US
+
+step "create DM devices (spans)"
+# Create two linear mappings spanning two loopdevs.
+# One of them gets a pathological name colliding with
+# the loop device name.
+# These mappings must not be removed by kpartx.
+# They also serve as DM devices to test partition removal on those.
+
+TABLE="\
+0 $((SIZE/SECTSIZ-OFFS)) linear $DEV1 $OFFS
+$((SIZE/SECTSIZ-OFFS)) $((SIZE/SECTSIZ-OFFS)) linear $DEV2 $OFFS"
+
+SPAN1=kpt
+SPAN2=$(basename $LO2)
+dmsetup create $SPAN1 <<<"$TABLE"
+push_cleanup 'dmsetup remove -f $SPAN1'
+
+dmsetup create $SPAN2 <<<"$TABLE"
+push_cleanup 'dmsetup remove -f $SPAN2'
+
+usleep $WAIT_US
+[[ -b /dev/mapper/$SPAN1 ]]
+[[ -b /dev/mapper/$SPAN2 ]]
+
+step "create vg on $LO3"
+# On the 3rd loop device, we create a VG and an LV
+# The LV should not be removed by kpartx.
+pvcreate --config "$LVMCONF" -f $LO3
+vgcreate --config "$LVMCONF" $VG $LO3
+push_cleanup 'vgremove --config "$LVMCONF" -f $VG'
+lvcreate --config "$LVMCONF" -L $((SIZE/2))B -n $LV $VG
+push_cleanup 'lvremove --config "$LVMCONF" -f $VG/$LV'
+usleep $WAIT_US
+
+[[ -b /dev/mapper/$VG-$LV ]]
+
+#

[dm-devel] [PATCH 06/10] kpartx: don't treat multi-linear mappings as partitions

2017-05-05 Thread Martin Wilck 
kpartx -d treats any map that has a "linear" mapping into a
device as first target as a partition of this device.
This is wrong, because linear mappings may consist of
several pieces, combining multiple devices into one
(LVM logical volumes are an example of such a
mapping). Partitions have to be single-target mappings.

Fix this by returning an error in dm_type() if a map with
multiple targets is encountered. The "type" of a map with
two or more targets is a difficult concept, anyway.

test case:
truncate -s 1G test0 test1
losetup /dev/loop0 test0
losetup /dev/loop1 test1
dmsetup create <
---
 kpartx/devmapper.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index d6ccd000..5380b2e9 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -390,10 +390,11 @@ dm_type(const char * name, char * type)
goto out;
 
/* Fetch 1st target */
-   dm_get_next_target(dmt, NULL, , ,
-  _type, );
-
-   if (!target_type)
+   if (dm_get_next_target(dmt, NULL, , ,
+  _type, ) != NULL)
+   /* more than one target */
+   r = -1;
+   else if (!target_type)
r = -1;
else if (!strcmp(target_type, type))
r = 1;
@@ -494,7 +495,7 @@ do_foreach_partmaps (const char * mapname, const char *uuid,
/*
 * skip if devmap target is not "linear"
 */
-   if (!dm_type(names->name, "linear")) {
+   if (dm_type(names->name, "linear") != 1) {
if (rd->verbose)
printf("%s: is not a linear target. Not 
removing\n",
   names->name);
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 09/10] kpartx: use absolute path for regular files

2017-05-05 Thread Martin Wilck 
kpartx supports being called for a regular file, in which
case it assumes that it should set up a loop device or use
an existing one. Because the loopinfo.lo_name field contains
an absolute path, matching with existing loop devices fails
for relative paths. Matching by basename only would be unreliable.

Therefore, convert relative to absolute path for regular files.

Signed-off-by: Martin Wilck 
---
 kpartx/kpartx.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index d9057fba..3bd16452 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -350,7 +350,13 @@ main(int argc, char **argv){
 
if (S_ISREG (buf.st_mode)) {
/* already looped file ? */
-   loopdev = find_loop_by_file(device);
+   char rpath[PATH_MAX];
+   if (realpath(device, rpath) == NULL) {
+   fprintf(stderr, "Error: %s: %s\n", device,
+   strerror(errno));
+   exit (1);
+   }
+   loopdev = find_loop_by_file(rpath);
 
if (!loopdev && what == DELETE)
exit (0);
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 10/10] kpartx: find_loop_by_file: use sysfs

2017-05-05 Thread Martin Wilck 
Rather then searching through all of /dev, look up loop devices
in /sys/devices/virtual/block. This is cleaner and more robust
(/dev/loop$Xp$Y symlinks may confuse kpartx).

Signed-off-by: Martin Wilck 
---
 kpartx/lopart.c | 42 +++---
 1 file changed, 31 insertions(+), 11 deletions(-)

diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 44f0c277..0521d7dc 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -68,25 +68,46 @@ char *find_loop_by_file(const char *filename)
 {
DIR *dir;
struct dirent *dent;
-   char dev[64], *found = NULL;
+   char dev[64], *found = NULL, *p;
int fd;
struct stat statbuf;
struct loop_info loopinfo;
+   const char VIRT_BLOCK[] = "/sys/devices/virtual/block";
+   char path[PATH_MAX];
 
-   dir = opendir("/dev");
+   dir = opendir(VIRT_BLOCK);
if (!dir)
return NULL;
 
while ((dent = readdir(dir)) != NULL) {
if (strncmp(dent->d_name,"loop",4))
continue;
-   if (!strcmp(dent->d_name, "loop-control"))
-   continue;
-   sprintf(dev, "/dev/%s", dent->d_name);
 
-   fd = open (dev, O_RDONLY);
+   if (snprintf(path, PATH_MAX, "%s/%s/dev", VIRT_BLOCK,
+dent->d_name) >= PATH_MAX)
+   continue;
+
+   fd = open(path, O_RDONLY);
if (fd < 0)
-   break;
+   continue;
+
+   if (read(fd, dev, sizeof(dev)) <= 0) {
+   close(fd);
+   continue;
+   }
+
+   close(fd);
+
+   dev[sizeof(dev)-1] = '\0';
+   p = strchr(dev, '\n');
+   if (p != NULL)
+   *p = '\0';
+   if (snprintf(path, PATH_MAX, "/dev/block/%s", dev) >= PATH_MAX)
+   continue;
+
+   fd = open (path, O_RDONLY);
+   if (fd < 0)
+   continue;
 
if (fstat (fd, ) != 0 ||
!S_ISBLK(statbuf.st_mode)) {
@@ -99,13 +120,12 @@ char *find_loop_by_file(const char *filename)
continue;
}
 
+   close (fd);
+
if (0 == strcmp(filename, loopinfo.lo_name)) {
-   close (fd);
-   found = xstrdup(dev);
+   found = realpath(path, NULL);
break;
}
-
-   close (fd);
}
closedir(dir);
return found;
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 07/10] libmultipath: don't treat multi-linear mappings as partitions

2017-05-05 Thread Martin Wilck 
dm_type is used in libmultipath only to check whether a mapping
is "linear", with the intention to test if it represents a
"partition". This test returns TRUE also for mappings with
multiple targets, the first of which happens to be a linear
mapping into the target device. This is questionable, it's
hard to assign a "type" to such maps anyway.

Fix this by returning an error for multi-target maps.
This is analogous to the patch
"kpartx: don't treat multi-linear mappings as partitions".

Signed-off-by: Martin Wilck 
---
 libmultipath/devmapper.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 5fb9d9ac..c19dcb62 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -572,7 +572,7 @@ out:
  * returns:
  *1 : match
  *0 : no match
- *   -1 : empty map
+ *   -1 : empty map, or more than 1 target
  */
 int dm_type(const char *name, char *type)
 {
@@ -594,10 +594,11 @@ int dm_type(const char *name, char *type)
goto out;
 
/* Fetch 1st target */
-   dm_get_next_target(dmt, NULL, , ,
-  _type, );
-
-   if (!target_type)
+   if (dm_get_next_target(dmt, NULL, , ,
+  _type, ) != NULL)
+   /* multiple targets */
+   r = -1;
+   else if (!target_type)
r = -1;
else if (!strcmp(target_type, type))
r = 1;
@@ -1185,9 +1186,9 @@ do_foreach_partmaps (const char * mapname,
do {
if (
/*
-* if devmap target is "linear"
+* if there is only a single "linear" target
 */
-   (dm_type(names->name, TGT_PART) > 0) &&
+   (dm_type(names->name, TGT_PART) == 1) &&
 
/*
 * and both uuid end with same suffix starting
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 04/10] kpartx: dm_remove_partmaps: support non-dm devices

2017-05-05 Thread Martin Wilck 
Commit  3d709241 improved partition removal, but broke support
for handling partitions of non-dm devices such as loop devices
or RAM disks.

This requires passing the dev_t of the device down to
do_foreach_partmap(). Doing so, there's little use in trying
to derive major/minor numbers from the "mapname" any more
(which actually is the device name for non-DM devices).
But we can use this to find out whether the device in question
is a dm device.

The test for UUID match doesn't work for non-DM devices (this
shall be improved in a later patch in this series).

The test for equal name of parent dev and partition is only valid
for dm devices. For non-dm devices such as loop, "/dev/mapper/loop0"
could, theoretically, be a partition of "/dev/loop0"
(and we don't want to rely on map names).

Fixes: 3d709241 "kpartx: sanitize delete partitions"
Signed-off-by: Martin Wilck 
---
 kpartx/devmapper.c | 26 +-
 kpartx/devmapper.h |  2 +-
 kpartx/kpartx.c|  3 ++-
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index cf6650c6..8f48a705 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -8,6 +8,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "devmapper.h"
 
 #define UUID_PREFIX "part%d-"
@@ -418,8 +419,8 @@ dm_compare_uuid(const char *mapuuid, const char *partname)
return 1;
 
if (!strncmp(partuuid, "part", 4)) {
-   char *p = strstr(partuuid, "mpath-");
-   if (p && !strcmp(mapuuid, p))
+   char *p = strchr(partuuid, '-');
+   if (p && !strcmp(mapuuid, p + 1))
r = 0;
}
free(partuuid);
@@ -432,6 +433,7 @@ struct remove_data {
 
 static int
 do_foreach_partmaps (const char * mapname, const char *uuid,
+dev_t devt,
 int (*partmap_func)(const char *, void *),
 void *data)
 {
@@ -443,6 +445,7 @@ do_foreach_partmaps (const char * mapname, const char *uuid,
int major, minor;
char dev_t[32];
int r = 1;
+   int is_dmdev = 1;
 
if (!(dmt = dm_task_create(DM_DEVICE_LIST)))
return 1;
@@ -460,15 +463,20 @@ do_foreach_partmaps (const char * mapname, const char 
*uuid,
goto out;
}
 
-   if (dm_devn(mapname, , ))
-   goto out;
+   if (dm_devn(mapname, , ) ||
+   (major != major(devt) || minor != minor(devt)))
+   /*
+* The latter could happen if a dm device "/dev/mapper/loop0"
+* exits while kpartx is called on "/dev/loop0".
+*/
+   is_dmdev = 0;
 
-   sprintf(dev_t, "%d:%d", major, minor);
+   sprintf(dev_t, "%d:%d", major(devt), minor(devt));
do {
/*
 * skip our devmap
 */
-   if (!strcmp(names->name, mapname))
+   if (is_dmdev && !strcmp(names->name, mapname))
goto next;
 
/*
@@ -496,7 +504,7 @@ do_foreach_partmaps (const char * mapname, const char *uuid,
/*
 * skip if uuids don't match
 */
-   if (dm_compare_uuid(uuid, names->name)) {
+   if (is_dmdev && uuid && dm_compare_uuid(uuid, names->name)) {
if (rd->verbose)
printf("%s: is not a kpartx partition. Not 
removing\n",
   names->name);
@@ -537,10 +545,10 @@ remove_partmap(const char *name, void *data)
 }
 
 int
-dm_remove_partmaps (char * mapname, char *uuid, int verbose)
+dm_remove_partmaps (char * mapname, char *uuid, dev_t devt, int verbose)
 {
struct remove_data rd = { verbose };
-   return do_foreach_partmaps(mapname, uuid, remove_partmap, );
+   return do_foreach_partmaps(mapname, uuid, devt, remove_partmap, );
 }
 
 #define FEATURE_NO_PART "no_partitions"
diff --git a/kpartx/devmapper.h b/kpartx/devmapper.h
index 9988ec0f..2e28c780 100644
--- a/kpartx/devmapper.h
+++ b/kpartx/devmapper.h
@@ -18,7 +18,7 @@ char * dm_mapname(int major, int minor);
 dev_t dm_get_first_dep(char *devname);
 char * dm_mapuuid(const char *mapname);
 int dm_devn (const char * mapname, int *major, int *minor);
-int dm_remove_partmaps (char * mapname, char *uuid, int verbose);
+int dm_remove_partmaps (char * mapname, char *uuid, dev_t devt, int verbose);
 int dm_no_partitions(char * mapname);
 
 #endif /* _KPARTX_DEVMAPPER_H */
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index e9b09492..e2056b7f 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -451,7 +451,8 @@ main(int argc, char **argv){
break;
 
case DELETE:
-   r = dm_remove_partmaps(mapname, uuid, verbose);
+   r = dm_remove_partmaps(mapname, uuid, buf.st_rdev,
+

[dm-devel] [PATCH 00/10] fixes for kpartx -d

2017-05-05 Thread Martin Wilck 
Working on a bug report about kpartx not properly removing
partitions for loop devices, I realized a number of glitches
and improperly handled corner cases in the kpartx code for
deleting partitions. Some mappings are not deleted although
they should be, and others are deleted although that is clearly
wrong.

This patch series fixes the issues I found. The series starts
with a test program demonstrating the problems. The program
succeeds only after all patches of this series are applied.

Here is my summary of what I think how kpartx should behave:

  1) kpartx should delete all mappings it created beforehand.
  2) kpartx should handle partitions on dm devices and other devices
 (e.g. loop devices) equally well.
  3) kpartx should only delete "partitions", which are single-target
 linear mappings into a block device. Other maps should not be touched.
  4) kpartx should only delete mappings it created itself beforehand.
 In particular, it shouldn't delete LVM LVs, even if they are fully
 contained in the block device at hand and thus look like partitions
 in the first place. (For historical compatibility reasons, allow
 such mappings to be deleted with the -f/--force flag).
  5) DM map names may be changed, thus kpartx shouldn't rely on them to
 check whether a mapping is a partition of a particular device. It is
 legal for a partition of /dev/loop0 to be named "loop0".

One patch has an obvious libdevmapper equivalent and is therefore
included although this series is otherwise focused only on kpartx.

Feedback is welcome.

Martin Wilck (10):
  kpartx: test-kpartx: new unit test program
  kpartx: avoid ioctl error for loop devices
  kpartx: remove is_loop_device
  kpartx: dm_remove_partmaps: support non-dm devices
  kpartx: dm_devn: return error for non-existent device
  kpartx: don't treat multi-linear mappings as partitions
  libmultipath: don't treat multi-linear mappings as partitions
  kpartx: use partition UUID for non-DM devices
  kpartx: use absolute path for regular files
  kpartx: find_loop_by_file: use sysfs

 kpartx/devmapper.c   |  39 +---
 kpartx/devmapper.h   |   2 +-
 kpartx/kpartx.c  |  42 +++-
 kpartx/lopart.c  |  73 ++
 kpartx/lopart.h  |   1 -
 kpartx/test-kpartx   | 253 +++
 libmultipath/devmapper.c |  15 +--
 7 files changed, 356 insertions(+), 69 deletions(-)
 create mode 100755 kpartx/test-kpartx

-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [PATCH 05/10] kpartx: dm_devn: return error for non-existent device

2017-05-05 Thread Martin Wilck 
For non-existent maps (ENXIO from ioctl()), dm_task_run and
dm_task_get_info return success. We need to check info.exists.

Signed-off-by: Martin Wilck 
---
 kpartx/devmapper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 8f48a705..d6ccd000 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -293,7 +293,7 @@ dm_devn (const char * mapname, int *major, int *minor)
if (!dm_task_run(dmt))
goto out;
 
-   if (!dm_task_get_info(dmt, ))
+   if (!dm_task_get_info(dmt, ) || info.exists == 0)
goto out;
 
*major = info.major;
-- 
2.12.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[dm-devel] [git pull] device mapper fixes for 4.12-rc1

2017-05-05 Thread Mike Snitzer 
Hi Linus,

The following changes since commit 7b66f13207e60e7c550af730986e77e38a0c69a3:

  Merge tag 'for-4.12/dm-post-merge-changes' of 
git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm 
(2017-05-03 10:34:03 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git 
tags/for-4.12/dm-fixes

for you to fetch changes up to 10add84e276432d9dd8044679a1028dd4084117e:

  dm cache metadata: fail operations if fail_io mode has been established 
(2017-05-05 14:40:13 -0400)

Please pull, thanks.
Mike


- DM cache metadata fixes to short-circuit operations that require the
  metadata not be in 'fail_io' mode.  Otherwise crashes are possible.

- a DM cache fix to address the inability to adapt to continuous IO that
  happened to also reflect a changing working set (which required old
  blocks be demoted before the new working set could be promoted)

- a DM cache smq policy cleanup that fell out from reviewing the above

- fix the Kconfig help text for CONFIG_DM_INTEGRITY


Joe Thornber (1):
  dm cache policy smq: allow demotions to happen even during continuous IO

Mike Snitzer (3):
  dm cache policy smq: cleanup free_target_met() and clean_target_met()
  dm integrity: improve the Kconfig help text for DM_INTEGRITY
  dm cache metadata: fail operations if fail_io mode has been established

 drivers/md/Kconfig   | 15 +--
 drivers/md/dm-cache-metadata.c   | 12 
 drivers/md/dm-cache-policy-smq.c | 30 ++
 3 files changed, 39 insertions(+), 18 deletions(-)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] remove REQ_OP_WRITE_SAME

2017-05-05 Thread Christoph Hellwig 
On Thu, Apr 13, 2017 at 10:23:10PM -0400, Martin K. Petersen wrote:
> The other thing that keeps me a bit on the fence is that a bunch of the
> plumbing to handle a bio with a payload different from bi_size is needed
> for the copy offload token. I'm hoping to have those patches ready for
> 4.13. Right now there are a bunch of places where handling of
> REQ_OP_COPY_IN and REQ_OP_COPY_OUT share conditionals with WRITE SAME.

Any chance to get a sneak preview of that work?

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Re: [dm-devel] [PATCH 25/27] block: remove the discard_zeroes_data flag

2017-05-05 Thread Nicholas A. Bellinger 
On Wed, 2017-05-03 at 10:33 -0400, Mike Snitzer wrote:
> On Tue, May 02 2017 at 11:33pm -0400,
> Nicholas A. Bellinger  wrote:
> 
> > On Tue, 2017-05-02 at 09:23 +0200, h...@lst.de wrote:
> > > On Tue, May 02, 2017 at 12:16:13AM -0700, Nicholas A. Bellinger wrote:
> > > > Or, another options is use bdev_write_zeroes_sectors() to determine when
> > > > dev_attrib->unmap_zeroes_data should be set.
> > > 
> > > Yes, that in combination with your patch to use bdev_write_zeroes_sectors
> > > for zeroing from write same seems like the right fix.
> > 
> > The larger target/iblock conversion patch looks like post v4.12 material
> > at this point, so to avoid breakage wrt to existing LBPRZ behavior, I'll
> > plan to push the following patch post -rc1.
> > 
> > diff --git a/drivers/target/target_core_device.c 
> > b/drivers/target/target_core_device.c
> > index d2f089c..e7caf78 100644
> > --- a/drivers/target/target_core_device.c
> > +++ b/drivers/target/target_core_device.c
> > @@ -851,7 +851,7 @@ bool target_configure_unmap_from_queue(struct 
> > se_dev_attrib *attrib,
> > attrib->unmap_granularity = q->limits.discard_granularity / 
> > block_size;
> > attrib->unmap_granularity_alignment = q->limits.discard_alignment /
> > block_size;
> > -   attrib->unmap_zeroes_data = 0;
> > +   attrib->unmap_zeroes_data = (q->limits.max_write_zeroes_sectors);
> > return true;
> >  }
> >  EXPORT_SYMBOL(target_configure_unmap_from_queue);
> > 
> 
> Completely a nit but: why the extra parenthesis?

dev_attrib->unmap_zeros_data is only compared as a bool.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel