This series fixes the xend vscsi=[] handling. Up to now only block devices in ''H:C:T:L, ...'' or ''/dev/sdX, ...'' notation is recognized properly. If a SCSI device is not block device xend fails to parse the lscsi -g output. Also /dev/disk/by-*/* notation is not working properly. The fallback sysfs parser was also not updated for Linux 3.0. Changes: xend/pvscsi: fix passing of SCSI control LUNs xend/pvscsi: fix usage of persistant device names for SCSI devices xend/pvscsi: update sysfs parser for Linux 3.0 tools/python/xen/util/vscsi_util.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)
Olaf Hering
2012-Aug-23 18:24 UTC
[PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1345743306 -7200 # Node ID 52f3d52bacdecb2c8d7f8aa26e2600febc03b6dd # Parent e6ca45ca03c2e08af3a74b404166527b68fd1218 xend/pvscsi: fix passing of SCSI control LUNs Currently pvscsi can not pass SCSI devices that have just a scsi_generic node. In the following example sg3 is a control LUN for the disk sdd. But vscsi=[''4:0:2:0,0:0:0:0''] does not work because the internal ''devname'' variable remains None. Later writing p-devname to xenstore fails because None is not a valid string variable. Since devname is used for just informational purpose use sg also as devname. carron:~ $ lsscsi -g [0:0:0:0] disk ATA FK0032CAAZP HPF2 /dev/sda /dev/sg0 [4:0:0:0] disk HP P2000G3 FC/iSCSI T100 /dev/sdb /dev/sg1 [4:0:1:0] disk HP P2000G3 FC/iSCSI T100 /dev/sdc /dev/sg2 [4:0:2:0] storage HP HSV400 0950 - /dev/sg3 [4:0:2:1] disk HP HSV400 0950 /dev/sdd /dev/sg4 [4:0:3:0] storage HP HSV400 0950 - /dev/sg5 [4:0:3:1] disk HP HSV400 0950 /dev/sde /dev/sg6 Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r e6ca45ca03c2 -r 52f3d52bacde tools/python/xen/util/vscsi_util.py --- a/tools/python/xen/util/vscsi_util.py +++ b/tools/python/xen/util/vscsi_util.py @@ -105,6 +105,8 @@ def _vscsi_get_scsidevices_by_lsscsi(opt devname = None try: sg = s[-1].split(''/dev/'')[1] + if devname is None: + devname = sg scsi_id = _vscsi_get_scsiid(sg) except IndexError: sg = None
Olaf Hering
2012-Aug-23 18:24 UTC
[PATCH 2 of 3] xend/pvscsi: fix usage of persistant device names for SCSI devices
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1345743313 -7200 # Node ID 2b9992aea26cfebc2dda56d1a97a35dc3a5c8ce8 # Parent 52f3d52bacdecb2c8d7f8aa26e2600febc03b6dd xend/pvscsi: fix usage of persistant device names for SCSI devices Currently the callers of vscsi_get_scsidevices() do not pass a mask string. This will call "lsscsi -g ''[]''", which causes a lsscsi syntax error. As a result the sysfs parser _vscsi_get_scsidevices() is used. But this parser is broken and the specified names in the config file are not found. Using a mask ''*'' if no mask was given will call lsscsi correctly and the following config is parsed correctly: vscsi=[ ''/dev/sg3, 0:0:0:0'', ''/dev/disk/by-id/wwn-0x600508b4000cf1c30000800000410000, 0:0:0:1'' ] Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 52f3d52bacde -r 2b9992aea26c tools/python/xen/util/vscsi_util.py --- a/tools/python/xen/util/vscsi_util.py +++ b/tools/python/xen/util/vscsi_util.py @@ -150,7 +150,7 @@ def _vscsi_get_scsidevices_by_sysfs(): return devices -def vscsi_get_scsidevices(mask=""): +def vscsi_get_scsidevices(mask="*"): """ get all scsi devices information """ devices = _vscsi_get_scsidevices_by_lsscsi("[%s]" % mask) @@ -279,7 +279,7 @@ def get_scsi_device(pHCTL): return _make_scsi_record(scsi_info) return None -def get_all_scsi_devices(mask=""): +def get_all_scsi_devices(mask="*"): scsi_records = [] for scsi_info in vscsi_get_scsidevices(mask): scsi_record = _make_scsi_record(scsi_info)
Olaf Hering
2012-Aug-23 18:24 UTC
[PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0
# HG changeset patch # User Olaf Hering <olaf@aepfle.de> # Date 1345743331 -7200 # Node ID 482b9db173f2ceefed06346bec9e6d8cef9704fe # Parent 2b9992aea26cfebc2dda56d1a97a35dc3a5c8ce8 xend/pvscsi: update sysfs parser for Linux 3.0 The sysfs parser for /sys/bus/scsi/devices understands only the layout of kernel version 2.6.16. This looks as follows: /sys/bus/scsi/devices/1:0:0:0/block:sda is a symlink to /sys/block/sda/ /sys/bus/scsi/devices/1:0:0:0/scsi_generic:sg1 is a symlink to /sys/class/scsi_generic/sg1 Both directories contain a ''dev'' file with the major:minor information. This patch updates the used regex strings to match also the colon to make it more robust against possible future changes. In kernel version 3.0 the layout changed: /sys/bus/scsi/devices/ contains now additional symlinks to directories such as host1 and target1:0:0. This patch ignores these as they do not point to the desired scsi devices. They just clutter the devices array. The directory layout in ''1:0:0:0'' changed as well, the ''type:name'' notation was replaced with ''type/name'' directories: /sys/bus/scsi/devices/1:0:0:0/block/sda/ /sys/bus/scsi/devices/1:0:0:0/scsi_generic/sg1/ Both directories contain a ''dev'' file with the major:minor information. This patch adds additional code to walk the subdir to find the ''dev'' file to make sure the given subdirectory is really the kernel name. In addition this patch makes sure devname is not None. Signed-off-by: Olaf Hering <olaf@aepfle.de> diff -r 2b9992aea26c -r 482b9db173f2 tools/python/xen/util/vscsi_util.py --- a/tools/python/xen/util/vscsi_util.py +++ b/tools/python/xen/util/vscsi_util.py @@ -130,20 +130,36 @@ def _vscsi_get_scsidevices_by_sysfs(): for dirpath, dirnames, files in os.walk(sysfs_mnt + SYSFS_SCSI_PATH): for hctl in dirnames: + if len(hctl.split('':'')) != 4: + continue paths = os.path.join(dirpath, hctl) devname = None sg = None scsi_id = None for f in os.listdir(paths): realpath = os.path.realpath(os.path.join(paths, f)) - if re.match(''^block'', f) or \ - re.match(''^tape'', f) or \ - re.match(''^scsi_changer'', f) or \ - re.match(''^onstream_tape'', f): + if re.match(''^block:'', f) or \ + re.match(''^tape:'', f) or \ + re.match(''^scsi_changer:'', f) or \ + re.match(''^onstream_tape:'', f): devname = os.path.basename(realpath) + elif f == "block" or \ + f == "tape" or \ + f == "scsi_changer" or \ + f == "onstream_tape": + for dir in os.listdir(os.path.join(paths, f)): + if os.path.exists(os.path.join(paths, f, dir, "dev")): + devname = os.path.basename(dir) - if re.match(''^scsi_generic'', f): + if re.match(''^scsi_generic:'', f): sg = os.path.basename(realpath) + elif f == "scsi_generic": + for dir in os.listdir(os.path.join(paths, f)): + if os.path.exists(os.path.join(paths, f, dir, "dev")): + sg = os.path.basename(dir) + if sg: + if devname is None: + devname = sg scsi_id = _vscsi_get_scsiid(sg) devices.append([hctl, devname, sg, scsi_id])
Ian Campbell
2012-Aug-24 09:15 UTC
Re: [PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs
On Thu, 2012-08-23 at 19:24 +0100, Olaf Hering wrote:> # HG changeset patch > # User Olaf Hering <olaf@aepfle.de> > # Date 1345743306 -7200 > # Node ID 52f3d52bacdecb2c8d7f8aa26e2600febc03b6dd > # Parent e6ca45ca03c2e08af3a74b404166527b68fd1218 > xend/pvscsi: fix passing of SCSI control LUNs > > Currently pvscsi can not pass SCSI devices that have just a scsi_generic node. > In the following example sg3 is a control LUN for the disk sdd. > But vscsi=[''4:0:2:0,0:0:0:0''] does not work because the internal ''devname'' > variable remains None. Later writing p-devname to xenstore fails because None > is not a valid string variable.Just out of interest, would you not need to pass through 4:0:2:1 too?> Since devname is used for just informational purpose use sg also as devname. > > carron:~ $ lsscsi -g > [0:0:0:0] disk ATA FK0032CAAZP HPF2 /dev/sda /dev/sg0 > [4:0:0:0] disk HP P2000G3 FC/iSCSI T100 /dev/sdb /dev/sg1 > [4:0:1:0] disk HP P2000G3 FC/iSCSI T100 /dev/sdc /dev/sg2 > [4:0:2:0] storage HP HSV400 0950 - /dev/sg3 > [4:0:2:1] disk HP HSV400 0950 /dev/sdd /dev/sg4 > [4:0:3:0] storage HP HSV400 0950 - /dev/sg5 > [4:0:3:1] disk HP HSV400 0950 /dev/sde /dev/sg6 > > Signed-off-by: Olaf Hering <olaf@aepfle.de>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > diff -r e6ca45ca03c2 -r 52f3d52bacde tools/python/xen/util/vscsi_util.py > --- a/tools/python/xen/util/vscsi_util.py > +++ b/tools/python/xen/util/vscsi_util.py > @@ -105,6 +105,8 @@ def _vscsi_get_scsidevices_by_lsscsi(opt > devname = None > try: > sg = s[-1].split(''/dev/'')[1] > + if devname is None: > + devname = sg > scsi_id = _vscsi_get_scsiid(sg) > except IndexError: > sg = None > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Campbell
2012-Aug-24 09:20 UTC
Re: [PATCH 2 of 3] xend/pvscsi: fix usage of persistant device names for SCSI devices
On Thu, 2012-08-23 at 19:24 +0100, Olaf Hering wrote:> # HG changeset patch > # User Olaf Hering <olaf@aepfle.de> > # Date 1345743313 -7200 > # Node ID 2b9992aea26cfebc2dda56d1a97a35dc3a5c8ce8 > # Parent 52f3d52bacdecb2c8d7f8aa26e2600febc03b6dd > xend/pvscsi: fix usage of persistant device names for SCSI devices > > Currently the callers of vscsi_get_scsidevices() do not pass a mask > string. This will call "lsscsi -g ''[]''", which causes a lsscsi syntax > error. As a result the sysfs parser _vscsi_get_scsidevices() is used. > But this parser is broken and the specified names in the config file are > not found. > > Using a mask ''*'' if no mask was given will call lsscsi correctly and the > following config is parsed correctly: > > vscsi=[ > ''/dev/sg3, 0:0:0:0'', > ''/dev/disk/by-id/wwn-0x600508b4000cf1c30000800000410000, 0:0:0:1'' > ] > > Signed-off-by: Olaf Hering <olaf@aepfle.de>Acked-by: Ian Campbell <ian.campbell@citrix.com>> > diff -r 52f3d52bacde -r 2b9992aea26c tools/python/xen/util/vscsi_util.py > --- a/tools/python/xen/util/vscsi_util.py > +++ b/tools/python/xen/util/vscsi_util.py > @@ -150,7 +150,7 @@ def _vscsi_get_scsidevices_by_sysfs(): > return devices > > > -def vscsi_get_scsidevices(mask=""): > +def vscsi_get_scsidevices(mask="*"): > """ get all scsi devices information """ > > devices = _vscsi_get_scsidevices_by_lsscsi("[%s]" % mask) > @@ -279,7 +279,7 @@ def get_scsi_device(pHCTL): > return _make_scsi_record(scsi_info) > return None > > -def get_all_scsi_devices(mask=""): > +def get_all_scsi_devices(mask="*"):This one could be = None , but I don''t think it matters much.> scsi_records = [] > for scsi_info in vscsi_get_scsidevices(mask): > scsi_record = _make_scsi_record(scsi_info) > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Ian Campbell
2012-Aug-24 09:24 UTC
Re: [PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0
On Thu, 2012-08-23 at 19:24 +0100, Olaf Hering wrote:> # HG changeset patch > # User Olaf Hering <olaf@aepfle.de> > # Date 1345743331 -7200 > # Node ID 482b9db173f2ceefed06346bec9e6d8cef9704fe > # Parent 2b9992aea26cfebc2dda56d1a97a35dc3a5c8ce8 > xend/pvscsi: update sysfs parser for Linux 3.0 > > The sysfs parser for /sys/bus/scsi/devices understands only the layout > of kernel version 2.6.16. This looks as follows: > > /sys/bus/scsi/devices/1:0:0:0/block:sda is a symlink to /sys/block/sda/ > /sys/bus/scsi/devices/1:0:0:0/scsi_generic:sg1 is a symlink to /sys/class/scsi_generic/sg1 > > Both directories contain a ''dev'' file with the major:minor information. > This patch updates the used regex strings to match also the colon to > make it more robust against possible future changes. > > > In kernel version 3.0 the layout changed: > /sys/bus/scsi/devices/ contains now additional symlinks to directories > such as host1 and target1:0:0. This patch ignores these as they do not > point to the desired scsi devices. They just clutter the devices array. > > The directory layout in ''1:0:0:0'' changed as well, the ''type:name'' > notation was replaced with ''type/name'' directories: > > /sys/bus/scsi/devices/1:0:0:0/block/sda/ > /sys/bus/scsi/devices/1:0:0:0/scsi_generic/sg1/ > > Both directories contain a ''dev'' file with the major:minor information. > This patch adds additional code to walk the subdir to find the ''dev'' > file to make sure the given subdirectory is really the kernel name. > > > In addition this patch makes sure devname is not None.Did you test this with both pre- and post-3.0 kernels?> > Signed-off-by: Olaf Hering <olaf@aepfle.de> > > diff -r 2b9992aea26c -r 482b9db173f2 tools/python/xen/util/vscsi_util.py > --- a/tools/python/xen/util/vscsi_util.py > +++ b/tools/python/xen/util/vscsi_util.py > @@ -130,20 +130,36 @@ def _vscsi_get_scsidevices_by_sysfs(): > > for dirpath, dirnames, files in os.walk(sysfs_mnt + SYSFS_SCSI_PATH): > for hctl in dirnames: > + if len(hctl.split('':'')) != 4: > + continue > paths = os.path.join(dirpath, hctl) > devname = None > sg = None > scsi_id = None > for f in os.listdir(paths): > realpath = os.path.realpath(os.path.join(paths, f)) > - if re.match(''^block'', f) or \ > - re.match(''^tape'', f) or \ > - re.match(''^scsi_changer'', f) or \ > - re.match(''^onstream_tape'', f): > + if re.match(''^block:'', f) or \ > + re.match(''^tape:'', f) or \ > + re.match(''^scsi_changer:'', f) or \ > + re.match(''^onstream_tape:'', f): > devname = os.path.basename(realpath) > + elif f == "block" or \ > + f == "tape" or \ > + f == "scsi_changer" or \ > + f == "onstream_tape": > + for dir in os.listdir(os.path.join(paths, f)): > + if os.path.exists(os.path.join(paths, f, dir, "dev")): > + devname = os.path.basename(dir) > > - if re.match(''^scsi_generic'', f): > + if re.match(''^scsi_generic:'', f): > sg = os.path.basename(realpath) > + elif f == "scsi_generic": > + for dir in os.listdir(os.path.join(paths, f)): > + if os.path.exists(os.path.join(paths, f, dir, "dev")): > + sg = os.path.basename(dir) > + if sg: > + if devname is None: > + devname = sg > scsi_id = _vscsi_get_scsiid(sg) > devices.append([hctl, devname, sg, scsi_id]) > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
Olaf Hering
2012-Aug-24 11:17 UTC
Re: [PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs
On Fri, Aug 24, Ian Campbell wrote:> On Thu, 2012-08-23 at 19:24 +0100, Olaf Hering wrote: > > # HG changeset patch > > # User Olaf Hering <olaf@aepfle.de> > > # Date 1345743306 -7200 > > # Node ID 52f3d52bacdecb2c8d7f8aa26e2600febc03b6dd > > # Parent e6ca45ca03c2e08af3a74b404166527b68fd1218 > > xend/pvscsi: fix passing of SCSI control LUNs > > > > Currently pvscsi can not pass SCSI devices that have just a scsi_generic node. > > In the following example sg3 is a control LUN for the disk sdd. > > But vscsi=[''4:0:2:0,0:0:0:0''] does not work because the internal ''devname'' > > variable remains None. Later writing p-devname to xenstore fails because None > > is not a valid string variable. > > Just out of interest, would you not need to pass through 4:0:2:1 too?Yes, this is just an example how to pass the control LUN at all. Olaf
Olaf Hering
2012-Aug-24 11:20 UTC
Re: [PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0
On Fri, Aug 24, Ian Campbell wrote:> Did you test this with both pre- and post-3.0 kernels?I looked at a 2.6.16 and a 3.5 kernel, and runtime tested a 3.0 kernel. Ideally the sysfs parser should be removed and lsscsi should be mandatory. Olaf