Re: [PATCHv2] insufficient quoting between "tap-ctl list" and xend/server/BlktapController.py

[Philipp Hahn <hahn@univention.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 920 bytes --]

Hello Ian,

On Thursday 01 December 2011 19:31:04 you wrote:
> Philipp Hahn writes ("Re: [Xen-devel] [BUG] insufficient quoting 
between "tap-ctl list" and xend/server/BlktapController.py"):
> > As a quick work-around, the attached patch fixes the problem for me. That
> > is, until tap-ctl changes it's output format.
>
> Thanks, I have applied it.

Argh, I submitted the wrong patch. The "line.split(None, 4)" needs to be 
a "3", because 3 splits needs to be done to get the 4 parts.

0004 is the correct full patch, 0005 is the inter-diff.

Sorry for the mixup.

Sincerely
Philipp
-- 
Philipp Hahn           Open Source Software Engineer      hahn@univention.de
Univention GmbH        Linux for Your Business        fon: +49 421 22 232- 0
Mary-Somerville-Str.1  D-28359 Bremen                 fax: +49 421 22 232-99
                                                   http://www.univention.de/

[-- Attachment #1.1.2: 0004-tap-ctl.patch --]
[-- Type: text/x-diff, Size: 2832 bytes --]

Bug #18357: Fix tap-ctl parsing.

Shutting down a domU with xen-4.1.2 doesn't terminate the corresponding blktap2
process, since one (other) VM uses a image file, which contains spaces in its
file name.  /var/log/xen/xend-debug.log has the following information:

Unhandled exception in thread started by
Traceback (most recent call last):
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/BlktapController.py", line 199, in finishDeviceCleanup
    TapdiskController.destroy(path)
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/BlktapController.py", line 289, in destroy
    tapdisk = TapdiskController.fromDevice(device)
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/BlktapController.py", line 278, in fromDevice
    TapdiskController.list())
  File "/usr/lib/python2.6/dist-packages/xen/xend/server/BlktapController.py", line 256, in list
    key, value = pair.split('=')
ValueError: need more than 1 value to unpack

BlktapController calls "tap-ctl list", which outputs one line for each process.
Each line contains key=value pairs without any quoting.

# tap-ctl list | grep 'args=.*:.* '
pid=10145 minor=18 state=0 args=aio:/var/lib/libvirt/images/Xen Windows drivers (gplpv 308).iso

(passing the output of tap-ctl through a pipe is important, since it switches
to a different list-format when STDOUT is a tty.)

BlktapController splits the output into lines using \n, then each line at each
space, and finally each of these 'words' at the '=', which fails for the
filename.


Limit the number of splits as a fast work-around.
--- a/tools/python/xen/xend/server/BlktapController.py
+++ b/tools/python/xen/xend/server/BlktapController.py
@@ -249,11 +249,12 @@ class TapdiskController(object):
         _list = TapdiskController.exc('list')
         if not _list: return []
 
-        for line in _list.split('\n'):
+        for line in _list.splitlines():
             tapdisk = TapdiskController.Tapdisk()
 
-            for pair in line.split():
-                key, value = pair.split('=')
+            # Since 'tap-ctl list' does not escape blanks in the path, hard-code the current format using 4 pairs to prevent splitting the path
+            for pair in line.split(None, 3):
+                key, value = pair.split('=', 1)
                 if key == 'pid':
                     tapdisk.pid = value
                 elif key == 'minor':
@@ -264,7 +265,7 @@ class TapdiskController(object):
                 elif key == 'state':
                     tapdisk.state = value
                 elif key == 'args' and value.find(':') != -1:
-                    tapdisk.dtype, tapdisk.image = value.split(':')
+                    tapdisk.dtype, tapdisk.image = value.split(':', 1)
 
             tapdisks.append(tapdisk)
 

[-- Attachment #1.1.3: 0005-tap-ctl.patch --]
[-- Type: text/x-diff, Size: 785 bytes --]

Bug #18357: Fix tap-ctl parsing.

The previous patch was still wrong: Pythons split() expects the number of
splits, not the number of results.

Do 3 splits to get pid, minor, state and args.
--- a/tools/python/xen/xend/server/BlktapController.py
+++ b/tools/python/xen/xend/server/BlktapController.py
@@ -252,7 +252,7 @@ class TapdiskController(object):
             tapdisk = TapdiskController.Tapdisk()
 
             # Since 'tap-ctl list' does not escape blanks in the path, hard-code the current format using 4 pairs to prevent splitting the path
-            for pair in line.split(None, 4):
+            for pair in line.split(None, 3):
                 key, value = pair.split('=', 1)
                 if key == 'pid':
                     tapdisk.pid = value

[-- Attachment #1.2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

[-- Attachment #2: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel