[PATCH 0 of 3] xend pvscsi fixes

[PATCH 0 of 3] xend pvscsi fixes

[Olaf Hering]

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(-)

[PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs

[Olaf Hering]

# 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

Re: [PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs

[Ian Campbell]

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

Re: [PATCH 1 of 3] xend/pvscsi: fix passing of SCSI control LUNs

[Olaf Hering]

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

[PATCH 2 of 3] xend/pvscsi: fix usage of persistant device names for SCSI devices

[Olaf Hering]

# 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)

Re: [PATCH 2 of 3] xend/pvscsi: fix usage of persistant device names for SCSI devices

[Ian Campbell]

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

[PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0

[Olaf Hering]

# 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])

Re: [PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0

[Ian Campbell]

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

Re: [PATCH 3 of 3] xend/pvscsi: update sysfs parser for Linux 3.0

[Olaf Hering]

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