[linux-lvm] libdm cannot swap names between two child volumes

[M.H. Tsai]

2015-06-04 20:14 GMT+08:00 Zdenek Kabelac <zkabelac at redhat.com>:
> Please provide  sequence of  'ioctl' where you think there is a bug.
> (or disclose your code using libdm).

Assume that there's a top-level LV "lv1",
and two sub-LVs "lv1_child1" and "lv1_child2".
The table of the top-level LV is "0 1234 my-target-type 253:1 253:2",
as following:

# dmsetup ls --tree
vg1-lv1 (253:3)
 ├─ vg1-lv1_child1 (253:1)
 └─ vg1-lv2_child2 (253:2)

I want to swap the ordering of subLVs, by using a dm-reload IOCTL:
dmsetup reload vg1-lv1 --table "0 1234 my-target-type 253:2 253:1"
dmsetup suspend vg1-lv1
dmsetup resume vg1-lv1

Then swap names of the two subLVs, to make the first child named "vg1_child1"
dmsetup rename vg1-lv1_child1 vg1-lv1_tmp
dmsetup rename vg1-lv1_child2 vg1-lv1_child1
dmsetup rename vg1-lv1_tmp vg1-lv1_child2


In LVM, the above action is:
logical_volume *parent = ...;  // vg1-lv1
logical_volume *child1 = seg_lv(parent, 0);
logical_volume *child2 = seg_lv(parent, 1);
// swap the subLV ordering
struct lv_segment_area tmp = first_seg(parent)->areas[0];
first_seg(parent)->areas[0] = first_seg(parent)->areas[1];
first_seg(parent)->areas[1] = tmp;
// swap the subLV names
char const *tmp = child1->name;
child1->name = child2->name;
child2->name = tmp;
lv_update_and_reload(parent);  // in lv_manip.c


Apply my previous patch on LVM2.2.02.105, the result IOCTLs are:
dm reload vg1-lv1  // swap child1 and child2
dm resume vg1-lv1
dm suspend vg1-lv1
dm suspend vg1-lv1_child1
dm suspend vg1-lv1_child2
dm rename vg1-lv1_child1 // temporarily renamed to vg1-lv1_child2_tmp,
but do not resume
dm rename vg1-lv1_child2 // renamed to vg1-lv1_child1
dm resume vg1-lv1_child1  // resume the renamed device
dm rename vg1-lv1_child2_tmp // renamed to the actual name "vg1-lv1_child2"
dm resume vg1-lv1_child2  // resume the actually renamed device

(There's might be an issue in my previous patch: I should not resume
the temporarily renamed device)
diff --git a/libdm/libdm-deptree.c b/libdm/libdm-deptree.c
index b0c5f13..3776200 100644
--- a/libdm/libdm-deptree.c
+++ b/libdm/libdm-deptree.c
@@ -1858,7 +1858,9 @@ int dm_tree_activate_children(struct dm_tree_node *dnode,
                                }
                                child->name = new_name;

-                               if (!rename_conflict)
+                               if (rename_conflict)
+                                       continue;
+                               else
                                        child->props.new_name = NULL;
                        }


However, then current LVM cannot accomplish the above IOCTLs,
due to the infinite loop in dm_tree_activate_children(). Also, It doesn't
assign a temporary name to avoid naming conflict.

If libdm doesn't support this feature, then the client need to
invoke lv_update_and_reload() twice to swap names, which might not
be efficiency:

// invoke lv_update_and_reload() twice to swap the subLV names
char const *tmp = child1->name;
child1->name = child2->name;
child2->name = "a_temporary_name";
lv_update_and_reload(parent);  // in lv_manip.c
child2->name = tmp;
lv_update_and_reload(parent);  // in lv_manip.c


> Dne 4.6.2015 v 12:02 M.H. Tsai napsal(a):
>> Hi All,
>> I found that there's a potential bug in libdm when I try to swap sub
>> volume's name. By executing the following code, the function
>> dm_tree_activate_children() will go into infinite loop since that
>> _rename_conflict_exists() still reports "resolvable=1" in this case.
>> logical_volume *parent = ...;
>> logical_volume *child1 = seg_lv(parent, 0);
>> logical_volume *child2 = seg_lv(parent, 1);
>> char const *tmp = child1->name;
>> child1->name = child2->name;
>> child2->name = tmp;
>> resume_lv(cmd, parent);
>>
>> Although this scenario might not happen through command-line
>> operation, some programmers might need this feature. I think that this
>> issue could be resolve in libdm by assigning a temporary name, as the
>> following patch does. Is it safe to do this?