On Wed, Feb 25, 2009 at 07:31:40PM +0100, Hans de Goede wrote:
>
>
> Joel Granados wrote:
>> 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.
>>
>
> Actually I made it global because it was duplicated (for a similar but
> different use) in class BlockDev, if you look at the patch you'll see it
> removes that duplicate copy and directly calls map_dev now. so map_dev
> cannot be insided the compare_tables function as it is used outside it.
aha. I understand, missed that. Its ok then.
>
> Regards,
>
> Hans
>
>
>
>> 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[
os]
>>> - 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@redhat.com
>>> 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@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
PATCH: pyblock: Make the dmraid table compare function a global function
