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 > Debian > Debian User

 
 
LinkBack Thread Tools
 
Old 11-10-2009, 05:11 PM
Peter Jones
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

On 11/10/2009 05:59 AM, Ales Kozumplik wrote:
> patch created by Masahiro Matsuya (mmatsuya@redhat.com), he claims
> that the change works for him. I have reviewed Masahiro's patch
> carefully for not breaking existing stufff but did not do any tests
> of the new functionality. The BZ lists three new tests cases for the
> QA team.
> ---
> dmraid.py | 4 +-
> fsset.py | 8 +++++-
> iutil.py | 40 +++++++++++++++++++++++++++++++++
> kickstart.py | 46 ++++++++++++++++++++++++++++++-------
> yuminstall.py | 69 ++++++++++++++++++++++++++++++++++++++-------------------
> 5 files changed, 132 insertions(+), 35 deletions(-)
>
> diff --git a/dmraid.py b/dmraid.py
> index 4fb7129..ed05341 100644
> --- a/dmraid.py
> +++ b/dmraid.py
> @@ -82,7 +82,7 @@ class DmDriveCache:
> if self.cache.has_key(obj.name):
> del self.cache[obj.name][obj.name]
> for k,v in self.cache[obj.name].items():
> - log.debug("adding %s to isys cache" % (name,))
> + log.debug("adding %s to isys cache" % (k,))
> isys.cachedDrives[k] = v
> log.debug("removing %s from dm cache" % (obj,))
> del self.cache[obj.name]
> @@ -91,7 +91,7 @@ class DmDriveCache:
> oldname = 'mapper/' + obj.name
> if isys.cachedDrives.has_key(oldname):
> dmNameUpdates[obj.name] = newname
> - self.remove(oldname)
> + self.remove(obj.name)
> # XXX why doesn't setting the property work?
> obj.set_name(newname)
> self.add(obj)

I'd like to see this bugfix in a separate commit with a commit message that
explains why we should be using e.g. obj.name instead of oldname is the right
thing here, so we can figure it out later.

> diff --git a/fsset.py b/fsset.py
> index 771a482..54753a0 100644
> --- a/fsset.py
> +++ b/fsset.py
> @@ -1380,8 +1380,14 @@ class FileSystemSet:
> if entry.mountpoint:
> # use LABEL if the device has a label except for multipath
> # devices and LUKS devices, which always use devname
> + ismpath = False
> + for mpdev in self.anaconda.id.diskset.mpList or []:
> + if not entry.device.getDevice().find(mpdev.get_name()) == -1:
> + ismpath = True
> + break
> +
> if entry.getLabel() and
> - entry.device.getDevice().find('mpath') == -1 and
> + not ismpath and
> not entry.device.crypto:
> device = "LABEL=%s" % (entry.getLabel(),)
> else:
> diff --git a/iutil.py b/iutil.py
> index d221d9c..289f5c7 100644
> --- a/iutil.py
> +++ b/iutil.py
> @@ -490,3 +490,43 @@ def inVmware():
> if "VMware" in out:
> return True
> return False
> +
> +
> +def getScsiDeviceByWwpnLunid(wwpn, lunid):
> + found = 0
> + for tgt in os.listdir('/sys/class/fc_transport'):
> + if tgt[:6] != "target":
> + continue
> + dir= '/sys/class/fc_transport/%s' % tgt
> + f = open('%s/port_name' % dir, "r")
> + tgt_wwpn = f.readline()
> + tgt_wwpn = tgt_wwpn.rstrip("
")
> + if tgt_wwpn.startswith("0x"):
> + tgt_wwpn = tgt_wwpn[2:]
> + tgt_wwpn = tgt_wwpn.upper()
> + f.close()
> + ## check the first match only
> + if tgt_wwpn == wwpn:
> + found = 1
> + break
> +
> + if found == 0:
> + return ""
> +
> + scsi_hctl="%s:%s" % (tgt[6:], lunid)
> +
> + found = 0
> + for tgt in os.listdir('/sys/block'):
> + devf = '/sys/block/%s/device' % tgt
> + if not os.path.exists(devf):
> + continue
> + devf = os.readlink(devf)
> + devf = os.path.basename(devf)
> + if devf == scsi_hctl:
> + found = 1
> + break
> + if found == 0:
> + tgt = ""
> + return tgt
> +
> +
> diff --git a/kickstart.py b/kickstart.py
> index 37b4246..deb46d4 100644
> --- a/kickstart.py
> +++ b/kickstart.py
> @@ -133,6 +133,7 @@ class AnacondaKSHandlers(KickstartHandlers):
> def doBootloader (self, args):
> KickstartHandlers.doBootloader(self, args)
> dict = self.ksdata.bootloader
> + self.id.bootloader.updateDriveList()
>
> if dict["location"] == "none":
> location = None
> @@ -359,23 +360,50 @@ class AnacondaKSHandlers(KickstartHandlers):
> ds = DiskSet(self.anaconda)
> ds.startMPath()
>
> + from bdevid import bdevid as _bdevid
> + bd = _bdevid()
> + bd.load("scsi")
> +
> mpath = self.ksdata.mpaths[-1]
> - log.debug("Searching for mpath '%s'" % (mpath.name,))
> for mp in DiskSet.mpList or []:
> + newname = ""
> it = True
> - for dev in mpath.devices:
> - dev = dev.split('/')[-1]
> + for path in mpath.paths:
> + dev = path.device
> + log.debug("Searching for mpath having '%s' as a member, the scsi id or wwpn:lunid" % (dev,))
> log.debug("mpath '%s' has members %s" % (mp.name, list(mp.members)))
> + if dev.find(':') != -1:
> + (wwpn, lunid) = dev.split(':')
> + if wwpn != "" and lunid != "":
> + if wwpn.startswith("0x"):
> + wwpn = wwpn[2:]
> + wwpn = wwpn.upper()
> + scsidev = iutil.getScsiDeviceByWwpnLunid(wwpn, lunid)
> + if scsidev != "":
> + dev = "/dev/%s" % scsidev
> + log.debug("'%s' is a member of the multipath device WWPN '%s' LUNID '%s'" % (dev, wwpn, lunid))
> if not dev in mp.members:
> - log.debug("mpath '%s' does not have device %s, skipping"
> - % (mp.name, dev))
> - it = False
> - if it:
> + mpscsiid = bd.probe("/dev/mapper/%s" % mp.name)[0]['unique_id']
> + if dev != mpscsiid:
> + log.debug("mpath '%s' does not have device %s, skipping"
> + % (mp.name, dev))
> + it = False
> + else:
> + log.debug("Recognized --device=%s as the scsi id of '%s'" % (dev, mp.name))
> + newname = path.name
> + break
> + else:
> + log.debug("Recognized --device=%s as a member of '%s'" % (dev, mp.name))
> + newname = path.name
> + break
> + if it and mp.name != newname:
> log.debug("found mpath '%s', changing name to %s"
> - % (mp.name, mpath.name))
> - newname = mpath.name
> + % (mp.name, newname))
> + mpath.name = mp.name
> ds.renameMPath(mp, newname)
> + bd.unload("scsi")
> return
> + bd.unload("scsi")
> ds.startMPath()
>
> def doDmRaid(self, args):
> diff --git a/yuminstall.py b/yuminstall.py
> index 50e6bb5..df898bf 100644
> --- a/yuminstall.py
> +++ b/yuminstall.py
> @@ -1424,31 +1424,46 @@ class YumBackend(AnacondaBackend):
> if not os.path.isdir(d):
> os.makedirs(d, mode=0755)
>
> + mpdevlst = []
> + for mpdev in anaconda.id.diskset.mpList or []:
> + mpdevlst.append(mpdev.name)
> +
> for entry in anaconda.id.fsset.entries:
> dev = entry.device.getDevice()
> - if dev.find('mpath') != -1:
> - # grab just the mpathXXX part of the device name
> - mpathname = dev.replace('/dev/', ')
> - mpathname = mpathname.replace('mpath/', ')
> - mpathname = mpathname.replace('mapper/', ')
> -
> - # we only want 'mpathNNN' where NNN is an int, strip all
> - # trailing subdivisions of mpathNNN
> - trail = re.search("(?<=^mpath).*$", mpathname)
> - if trail is not None:
> - i = 1
> - major = None
> -
> - while i <= len(trail.group()):
> - try:
> - major = int(trail.group()[0:i])
> - i += 1
> - except:
> - break
> -
> - if major is not None:
> - mpathname = mpathname.replace(trail.group(), ')
> - mpathname = "%s%d" % (mpathname, major,)
> + for mpdev in mpdevlst:
> + # eliminate the major number (ex. mpath0 -> mpath)
> + pos = 0
> + while pos < len(mpdev):
> + if mpdev[pos].isdigit():
> + mpdev = mpdev[os]
> + break
> + pos += 1
> + if dev.find(mpdev) != -1:
> + # grab just the basename of the device
> + mpathname = dev.replace('/dev/', ')
> + mpathname = mpathname.replace('mpath/', ')
> + mpathname = mpathname.replace('mapper/', ')
> +
> + # In case of mpathNNNpMMM, we only want 'mpathNNN' where
> + # NNN is an int, strip all trailing subdivisions of mpathNNN
> + mpregex = "(?<=^%s).*$" % mpdev
> + trail = re.search(mpregex, mpathname)
> + if trail is not None:
> + i = 1
> + major = None
> +
> + while i <= len(trail.group()):
> + try:
> + major = int(trail.group()[0:i])
> + i += 1
> + except:
> + break
> +
> + if major is not None:
> + mpathname = mpathname.replace(trail.group(), ')
> + mpathname = "%s%d" % (mpathname, major,)
> + else:
> + continue
>
> # if we have seen this mpath device, continue
> if wwids != []:
> @@ -1624,6 +1639,14 @@ class YumBackend(AnacondaBackend):
>
> f.write('}

')
>
> + for (mpathname, id) in wwids:
> + if(mpathname.find("mpath") == -1):
> + # this mpath device was renamed
> + f.write('
multipath {
')
> + f.write(" wwid "%s"
" % (id,))
> + f.write(" alias "%s"
" % (mpathname,))
> + f.write('}

')
> +
> f.close()
>
> def checkSupportedUpgrade(self, anaconda):

This looks good to me.

--
Peter

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 11-10-2009, 06:49 PM
Chris Lumens
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

> + # In case of mpathNNNpMMM, we only want 'mpathNNN' where
> + # NNN is an int, strip all trailing subdivisions of mpathNNN
> + mpregex = "(?<=^%s).*$" % mpdev
> + trail = re.search(mpregex, mpathname)

I know this was in the code before being patched, but that's a pretty
awful regex. Can you come up with a way to make this more readable?

> @@ -1624,6 +1639,14 @@ class YumBackend(AnacondaBackend):
>
> f.write('}

')
>
> + for (mpathname, id) in wwids:
> + if(mpathname.find("mpath") == -1):
> + # this mpath device was renamed
> + f.write('
multipath {
')
> + f.write(" wwid "%s"
" % (id,))
> + f.write(" alias "%s"
" % (mpathname,))
> + f.write('}

')
> +
> f.close()
>
> def checkSupportedUpgrade(self, anaconda):

The fact that all this stuff is just sitting in doPreInstall is dire,
but there's nothing we can do about that now.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 11-11-2009, 09:04 AM
Ales Kozumplik
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

On 11/11/2009 10:56 AM, Ales Kozumplik wrote:

patch originated from Masahiro Matsuya (mmatsuya@redhat.com), he claims that the change works for him.
I have reviewed Masahiro's patch carefully for not breaking existing stufff but did not do any tests of the new functionality.
The BZ lists three new tests cases for the QA team.


Hi list,

This patch is an updated version of the similar patch from yesterday.
Peter recommended taking out a non-related part into a separate patch,
see here:

https://www.redhat.com/archives/anaconda-devel-list/2009-November/msg00186.html

In the new patch I also simplified regular expression in yuminstall.py,
as per Chris's comment.


Ales

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 11-11-2009, 12:46 PM
Ales Kozumplik
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

On 11/11/2009 10:56 AM, Ales Kozumplik wrote:

patch originated from Masahiro Matsuya (mmatsuya@redhat.com), he claims that the change works for him.
I have reviewed Masahiro's patch carefully for not breaking existing stufff but did not do any tests of the new functionality.
The BZ lists three new tests cases for the QA team.


I have a big question here:

What should I do with the patch after it's on rhel5-branch? I can't
simply merge it to master, the python source code changed a lot. I can't
build the same functionality from the scratch because I don't have the
hardware to test this on. And if we don't get it to master, someone will
probably complain about a regression. Suggestions please.


Thanks,
Ales

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 11-12-2009, 08:18 PM
Chris Lumens
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

> What should I do with the patch after it's on rhel5-branch? I can't
> simply merge it to master, the python source code changed a lot. I
> can't build the same functionality from the scratch because I don't
> have the hardware to test this on. And if we don't get it to master,
> someone will probably complain about a regression. Suggestions
> please.

Assuming this is a feature we want for RHEL6 and beyond, it's going to
need to be completely written from scratch. In fact, the multipath
kickstart command in master doesn't even do anything. It's just a stub.
If you want to look into it, go for it.

- Chris

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list
 
Old 11-13-2009, 08:11 AM
Ales Kozumplik
 
Default kickstart option to make mpath0 point to arbitrary LUN (#502768)

Assuming this is a feature we want for RHEL6 and beyond, it's going to
need to be completely written from scratch. In fact, the multipath
kickstart command in master doesn't even do anything. It's just a stub.
If you want to look into it, go for it.



Yep, I want to do that. I cloned the old BZ into #537352, so we keep
track. I'll take a look at it at some point soon unless someone beats me
to it.


Ales

_______________________________________________
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 04:47 PM.

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