On 01/20/2010 04:35 PM, Hans de Goede wrote:
> Hi,
>
> On 01/21/2010 01:03 AM, Peter Jones wrote:
>> This makes PartitionDevice.teardown() remove device-mapper tables when
>> appropriate, so devices using them don't have to clean up the partition
>> and all the conditional testing that comes with that.
>> ---
>> storage/devices.py | 19 +++++++++++++++++++
>> 1 files changed, 19 insertions(+), 0 deletions(-)
>>
>> diff --git a/storage/devices.py b/storage/devices.py
>> index 3bb0991..237fc75 100644
>> --- a/storage/devices.py
>> +++ b/storage/devices.py
>> @@ -1314,6 +1314,25 @@ class PartitionDevice(StorageDevice):
>>
>> self.exists = False
>>
>> + def teardown(self, recursive=None):
>> + """ Close, or tear down, a device. """
>> + log_method_call(self, self.name, status=self.status)
>> + if not self.exists and not recursive:
>> + raise DeviceError("device has not been created", self.name)
>> +
>> + if self.status:
>> + if self.format.exists:
>> + self.format.teardown()
>> + devmap = block.getMap(major=self.major, minor=self.minor)
>> + if devmap:
>> + try:
>> + block.removeDeviceMap(devmap)
>> + except Exception as e:
>> + raise PartitioningError("failed to tear down
>> device-mapper partition %s: %s" % (self.name, e))
>> +
>> + udev_settle()
>> + StorageDevice.teardown(self, recursive=recursive)
>> +
>
> Erm this is going to be a problem for dmraid, if you do this you
> should also add a modified setup() which restores the partitions
> again, because nothing else is going to do that for dmraid sets.
>
> But I wonder, are there reasons to take the mpath set down once up ?
Well, there certainly are if the only place we've got partition setup
is in the MultipathDevice, not the PartitionDevice, which is currently
the case (and I'm not aiming to change that; realization of partitions
is easy there.)
> With BIOS RAID (both the mdraid and dmraid flavors) I started out
> implementing setup() and teardown() methods too. But in the end it
> turned out it was better to just set them up once, and be done with
> it (partition handling problems was one of the reasons to do this,
> pyblock and parted both modifying the partition mappings in turn got
> ugly pretty quickly).>
> So currently for dmraid the model is:
>
> 1) Assemble it using pyblock, which creates the initial partition mappings
> 2) Don't let pyblock do anything else with it
> 3) Let parted modify the mappings as it adds / removes partitions
> (when executing actions).
I tried this (among about 6 other approaches now...) for mpath and had trouble
getting it to work, mostly because we currently get teardowns for devices
before they have 0 holders. On normal disk devices, that fails /unless/
the only holder are the partitions, and they themselves have no holders.
This behavior is amazingly hard to emulate without having tracking of the
children.
The version I've got here works for mpath. So, given that, How about if I do
it as the patch below, instead.
> You're above patch breaks this as, once the teardown removes the
> partition nothing is going to restore it. Also keep in mind that
> certain actions, only setup and teardown a single device. So you
> cannot count on the disk setup() restoring the partition mappings, if
> you are going to do this you need to add a setup() which restores the
> mapping. But I think using the dmraid model instead is better.
I realize it's kindof weirdly asymetrical, but the MultipathDevice.setup()
does create the mapping.
commit a1f6eb3d1fd515f47ccb2aa1ae6ba67e7a0342d2
Author: Peter Jones <pjones@xxxxxxxxxx>
Date: Wed Jan 20 18:56:59 2010 -0500
Make PartitionDevice have its own teardown() when used with mpath.
This makes PartitionDevice.teardown() remove device-mapper tables when
appropriate, so devices using them don't have to clean up the partition
and all the conditional testing that comes with that.
diff --git a/storage/devices.py b/storage/devices.py
index 9020463..8bc68c3 100644
--- a/storage/devices.py
+++ b/storage/devices.py
@@ -1314,6 +1314,26 @@ class PartitionDevice(StorageDevice):
self.exists = False
+ def teardown(self, recursive=None):
+ """ Close, or tear down, a device. """
+ log_method_call(self, self.name, status=self.status)
+ if not self.exists and not recursive:
+ raise DeviceError("device has not been created", self.name)
+
+ if self.status:
+ if self.format.exists:
+ self.format.teardown()
+ if self.parents[0].type == 'dm-multipath':
+ devmap = block.getMap(major=self.major, minor=self.minor)
+ if devmap:
+ try:
+ block.removeDeviceMap(devmap)
+ except Exception as e:
+ raise PartitioningError("failed to tear down device-mapper partition %s: %s" % (self.name, e))
+ udev_settle()
+
+ StorageDevice.teardown(self, recursive=recursive)
+
def _getSize(self):
""" Get the device's size. """
size = self._size
--
Peter
What we need is either less corruption, or more chances to
participate in it.
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
