Commit db8e5f1c authored by Iustin Pop's avatar Iustin Pop
Browse files

Fix bug in drbd8 replace disks on current nodes



Currently the drbd8 replace-disks on the same node (i.e. -p or -s) has
a bug in that it does modify the instance disk temporarily before
changing it back to the same value. However, we don't need to, and
shouldn't do that: what this operation do is simply change the LVM
configuration on the node, but otherwise the instance disks keep the
same configuration as before.

In the current code, this change back-and-forth is fine *unless* we
fail during attaching the new LVs to DRBD; in which case, we're left
with a half-modified disk, which is entirely wrong.

So we change the code in two ways:

- use temporary copies of the disk children in the old_lvs var
- stop updating disk.children

Which means that the instance should not be modified anymore (except
maybe for SetDiskID, which is a legacy and unfortunate decision that
will have to cleaned up sometime).
Signed-off-by: default avatarIustin Pop <iustin@google.com>
Reviewed-by: default avatarMichael Hanselmann <hansmi@google.com>
parent 7afca87f
......@@ -8422,6 +8422,12 @@ class TLReplaceDisks(Tasklet):
(node_name, self.instance.name))
def _CreateNewStorage(self, node_name):
"""Create new storage on the primary or secondary node.
This is only used for same-node replaces, not for changing the
secondary node, hence we don't want to modify the existing disk.
"""
iv_names = {}
for idx, dev in enumerate(self.instance.disks):
......@@ -8443,7 +8449,7 @@ class TLReplaceDisks(Tasklet):
logical_id=(vg_meta, names[1]))
new_lvs = [lv_data, lv_meta]
old_lvs = dev.children
old_lvs = [child.Copy() for child in dev.children]
iv_names[dev.iv_name] = (dev, old_lvs, new_lvs)
# we pass force_create=True to force the LVM creation
......@@ -8568,10 +8574,14 @@ class TLReplaceDisks(Tasklet):
rename_new_to_old)
result.Raise("Can't rename new LVs on node %s" % self.target_node)
# Intermediate steps of in memory modifications
for old, new in zip(old_lvs, new_lvs):
new.logical_id = old.logical_id
self.cfg.SetDiskID(new, self.target_node)
# We need to modify old_lvs so that removal later removes the
# right LVs, not the newly added ones; note that old_lvs is a
# copy here
for disk in old_lvs:
disk.logical_id = ren_fn(disk, temp_suffix)
self.cfg.SetDiskID(disk, self.target_node)
......@@ -8591,10 +8601,6 @@ class TLReplaceDisks(Tasklet):
"volumes"))
raise errors.OpExecError("Can't add local storage to drbd: %s" % msg)
dev.children = new_lvs
self.cfg.Update(self.instance, feedback_fn)
cstep = 5
if self.early_release:
self.lu.LogStep(cstep, steps_total, "Removing old storage")
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment