FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > Ubuntu > Ubuntu Server Development

 
 
LinkBack Thread Tools
 
Old 01-20-2010, 08:35 PM
Hans de Goede
 
Default Make PartitionDevice have its own teardown()

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 ? 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).

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.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 01-20-2010, 11:03 PM
Peter Jones
 
Default Make PartitionDevice have its own teardown()

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)
+
def _getSize(self):
""" Get the device's size. """
size = self._size
--
1.6.5.2

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 01-20-2010, 11:34 PM
David Lehman
 
Default Make PartitionDevice have its own teardown()

On Wed, 2010-01-20 at 19:03 -0500, 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.

Ack.

> ---
> 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)
> +
> def _getSize(self):
> """ Get the device's size. """
> size = self._size


_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 01-21-2010, 06:04 PM
Peter Jones
 
Default Make PartitionDevice have its own teardown()

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@redhat.com>
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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 01-21-2010, 06:48 PM
David Lehman
 
Default Make PartitionDevice have its own teardown()

See way below...

On Thu, 2010-01-21 at 14:04 -0500, Peter Jones wrote:
> 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@redhat.com>
> 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))

PartitioningError is for partition allocation failures.
DeviceTeardownError would be most appropriate here.

Otherwise it looks fine.

Dave

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 

Thread Tools




All times are GMT. The time now is 03:08 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org