Patch looks good. Only one comment: I would leave map_dev as an inside
function to compare_tables. This would stress the point that it is
only used by this function. While we could leave it outside for it to
be uesed, I would prefer to avoid using it as it is not general enough.
I mean, map_dev might imply mapping major,minor to a dev path, or a dev
path to a mount point or something different of what is currently
implemented.
regards.
On Tue, Feb 24, 2009 at 10:42:43PM +0100, Hans de Goede wrote:
> We are comparing tables in other places too, so make the dmraid table
> equal function a global function named compare_tables. This is a preparation
> patch for modifying the python code to match the C-code table handling changes.
> ---
> device.py | 158 ++++++++++++++++++++++++++-----------------------------------
> 1 files changed, 67 insertions(+), 91 deletions(-)
>
> diff --git a/device.py b/device.py
> index 34a361a..aae1204 100644
> --- a/device.py
> +++ b/device.py
> @@ -48,6 +48,69 @@ def removeDeviceMap(map):
> pass
> map.remove()
>
> +def map_dev(path):
> + if path[0] != '/':
> + return path
> +
> + try:
> + statinfo = _os.stat(path)
> + if not _stat.S_ISBLK(statinfo.st_mode):
> + return path
> +
> + return "%d:%d" % (statinfo.st_rdev/256, statinfo.st_rdev%256, )
> + except:
> + return path
> +
> +# Helper function for get_map
> +# The tables will be considered the same if, everything else being the
> +# same, they contain the same sets of devices.
> +def compare_tables(table1, table2):
> + table1 = str(table1).strip().split(' ')
> + table2 = str(table2).strip().split(' ')
> + table1sets = []
> + table2sets = []
> +
> + # We do this to avoid the index out of range exception
> + if len(table1) != len(table2):
> + return False
> +
> + for i in range(len(table1)):
> + d1 = map_dev(table1[i])
> + d2 = map_dev(table2[i])
> +
> + # when at least one changes its a device.
> + if (table1[i] != d1) or (table2[i] != d2):
> + # The d{1,2} will always have the major:minor string
> + # We also need what comes after the dev, the offset.
> + try:
> + table1sets.append("%s %s" % (d1, table1[i+1]))
> + table2sets.append("%s %s" % (d2, table2[i+1]))
> + i += 1
> + except IndexError, msg:
> + # The device must have an offset, if not its nonesense
> + return False
> + continue
> +
> + # these are not devices
> + if d1 == d2:
> + continue
> +
> + if d1 != d2:
> + return False
> +
> + # For mirror sets the devices can be in disorder.
> + if table1[2] == "mirror":
> + if set(table1sets) != set(table2sets):
> + return False
> +
> + # For none mirror the devs have to be in order.
> + else:
> + for i in range(len(table1sets)):
> + if table1sets[i] != table2sets[i]:
> + return False
> +
> + return True
> +
> class BlockDev:
> def get_major(self):
> return self._BlockDev__device.major
> @@ -370,34 +433,11 @@ class MultiPath:
>
> # we get "/dev/hda" from one and "3:0" from the other, so we have to
> # fix up the device name
> - def map_dev(path):
> + def munge_dev(path):
> if path[0] != '/':
> return path.strip()
>
> - bd = path.strip()
> - try:
> - newpath=path.split('/')[-1]
> - pos = 0
> - dev = None
> - num = None
> - while pos < len(newpath):
> - if newpath[pos].isdigit():
> - dev = newpath[:pos]
> - num = newpath[pos:]
> - break
> - pos += 1
> - if dev is None:
> - dev = newpath
> - sysnewpath = None
> - if num is None:
> - sysnewpath = '/sys/block/%s/dev' % (dev,)
> - else:
> - sysnewpath = '/sys/block/%s/%s%s/dev' % (dev, dev, num)
> - f = open(sysnewpath, 'r')
> - l = f.readline()
> - bd = l.strip()
> - except:
> - pass
> + bd = map_dev(path)
> # starting with 2.6.17-1.2510.fc6 or so, there's an implicit
> # minimum IOs of 1000, which gets _added_ to the line "dmsetup ls"
> # shows. This sucks.
> @@ -406,7 +446,7 @@ class MultiPath:
> tableParts = [0, self.size, 'multipath']
>
> params = '0 0 1 1 round-robin 0 %s 1 %s' % (len(self.bdevs), \
> - _string.join(map(map_dev, self.bdevs)))
> + _string.join(map(munge_dev, self.bdevs)))
> tableParts.append(params)
>
> import dm as _dm
> @@ -658,69 +698,6 @@ class RaidSet:
> pass
> bdev = property(get_bdev, None, None, "block.BlockDev")
>
> - # Helper function for get_map
> - # The tables will be considered the same if, everything else being the
> - # same, they contain the same sets of devices.
> - def equal(self, table1, table2):
> - def map_dev(path):
> - if path[0] != '/':
> - return path
> -
> - try:
> - statinfo = _os.stat(path)
> - if not _stat.S_ISBLK(statinfo.st_mode):
> - return path
> -
> - return "%d:%d" % (statinfo.st_rdev/256, statinfo.st_rdev%256, )
> - except:
> - return path
> -
> - table1 = table1.strip().split(' ')
> - table2 = table2.strip().split(' ')
> - table1sets = []
> - table2sets = []
> -
> - # We do this to avoid the index out of range exception
> - if len(table1) != len(table2):
> - return False
> -
> - for i in range(len(table1)):
> - d1 = map_dev(table1[i])
> - d2 = map_dev(table2[i])
> -
> - # when at least one changes its a device.
> - if (table1[i] != d1) or (table2[i] != d2):
> - # The d{1,2} will always have the major:minor string
> - # We also need what comes after the dev, the offset.
> - try:
> - table1sets.append("%s %s" % (d1, table1[i+1]))
> - table2sets.append("%s %s" % (d2, table2[i+1]))
> - i += 1
> - except IndexError, msg:
> - # The device must have an offset, if not its nonesense
> - return False
> - continue
> -
> - # these are not devices
> - if d1 == d2:
> - continue
> -
> - if d1 != d2:
> - return False
> -
> - # For mirror sets the devices can be in disorder.
> - if self.rs.type == "mirror":
> - if set(table1sets) != set(table2sets):
> - return False
> -
> - # For none mirror the devs have to be in order.
> - else:
> - for i in range(len(table1sets)):
> - if table1sets[i] != table2sets[i]:
> - return False
> -
> - return True
> -
> def get_map(self):
> if not self._RaidSet__map is None:
> return self._RaidSet__map
> @@ -728,8 +705,7 @@ class RaidSet:
> import dm as _dm
>
> for map in _dm.maps():
> - # XXX wtf? why's it have a space at the end sometimes?
> - if self.equal(str(map.table), str(self.rs.dmTable)):
> + if compare_tables(map.table, self.rs.dmTable):
> self._RaidSet__map = map
> self.active = True
> del _dm
> --
> 1.6.1.3
>
> _______________________________________________
> Anaconda-devel-list mailing list
> Anaconda-devel-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/anaconda-devel-list
--
Joel Andres Granados
Brno, Czech Republic, Red Hat.
_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
