Xen devel - Nov 2011 - [BUG] insufficient quoting between "tap-ctl list" and xend/server/BlktapController.py

Hello,

I have a problem shutting down a domU with xen-4.1.2, which 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.


tap-ctl should probably be changed to use some quoting when printing the 
filename.

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/


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

Hello,

On Friday 25 November 2011 17:21:08 Philipp Hahn wrote:
> 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.
As a quick work-around, the attached patch fixes the problem for me. That is, 
until tap-ctl changes it''s output format.
A more permanent solution would be to add proper quoting / escaping to tap-ctl 
and un-quoting / de-escaping  to BlktapController.py

Signed-off-by: Philipp Hahn <hahn@univention.de>

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/



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

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.
> A more permanent solution would be to add proper quoting / escaping to
tap-ctl
> and un-quoting / de-escaping  to BlktapController.py
xend is pretty much obsolete now and I don''t imagine we''ll be
doing
any rework like that soon I''m afraid.

Ian.

On Thu, Dec 01, 2011 at 06:31:04PM +0000, Ian Jackson
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.
> 
> > A more permanent solution would be to add proper quoting / escaping to
tap-ctl
> > and un-quoting / de-escaping  to BlktapController.py
> 
> xend is pretty much obsolete now and I don''t imagine
we''ll be doing
> any rework like that soon I''m afraid.
> 
I realize xend doesn''t get much attention anymore,
but it''s still the only way to use some features,
like: pvusb, pvscsi, remus, block scripts, etc..

So it''s good to apply at least bugfix patches :) 

-- Pasi

On Thu, 2011-12-01 at 19:37 +0000, Pasi Kärkkäinen
wrote:
> On Thu, Dec 01, 2011 at 06:31:04PM +0000, Ian Jackson 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.
> > 
> > > A more permanent solution would be to add proper quoting /
escaping to tap-ctl
> > > and un-quoting / de-escaping  to BlktapController.py
> > 
> > xend is pretty much obsolete now and I don''t imagine
we''ll be doing
> > any rework like that soon I''m afraid.
> > 
> 
> I realize xend doesn''t get much attention anymore,
> but it''s still the only way to use some features,
xend is deprecated. This ultimately means that xend will go away. If you
are dependent on these features then you need put effort into ensuring
that they become supported in libxl and xl. You should not assume that
xend will be around forever nor that someone else will do this work for
you before xend does go away. In other words people who need these
features need to provide patches for them. (I suppose someone could also
step up and offer to maintain xend properly, I''m not holding my breath
for that).
> like:
> remus, block scripts
AFAIK support for these in xl/libxl these are being worked on.
>  pvusb, pvscsi, 
I''m not aware of any effort to make these work with libxl.
> So it''s good to apply at least bugfix patches :) 
TO some extent that just gives people a false sense that they can sit on
xend forever, which is not the case. People need to realise that xend
_is_ going away and they need to start planning accordingly.
> 
> -- Pasi
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Hello,

On Thursday 01 December 2011 19:31:04 Ian Jackson 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.
Thanks for applying.

On Thursday 01 December 2011 20:51:05 Ian Campbell
wrote:
> xend is deprecated. This ultimately means that xend will go away. If you
> are dependent on these features then you need put effort into ensuring
> that they become supported in libxl and xl.
For our forthcomming release of UCS-3.0 we really wanted to switch from Xend 
to xl, but be had to switch back to using Xend, because libvirt still lacks 
some important features like migration; see 
<http://libvirt.org/hvsupport.html> for a comparison matrix. Back at the 
beginning of 2011 Univention sponsored Markus Gross to work on the xl-binding 
of libvirt, but after the end of his internship I have not seen that much 
work on the xl-binding in libvirt except from Jim Fehlig.

Sincerely
Philipp Hahn
-- 
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/

On Fri, 2011-12-02 at 08:51 +0000, Philipp Hahn wrote:
> Hello,
> 
> On Thursday 01 December 2011 19:31:04 Ian Jackson 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.
> 
> Thanks for applying.
> 
> On Thursday 01 December 2011 20:51:05 Ian Campbell wrote:
> > xend is deprecated. This ultimately means that xend will go away. If
you
> > are dependent on these features then you need put effort into ensuring
> > that they become supported in libxl and xl.
> 
> For our forthcomming release of UCS-3.0 we really wanted to switch from
Xend
> to xl, but be had to switch back to using Xend, because libvirt still lacks
> some important features like migration; see 
> <http://libvirt.org/hvsupport.html> for a comparison matrix. Back at
the
> beginning of 2011 Univention sponsored Markus Gross to work on the
xl-binding
> of libvirt, 
Thank you for putting your money where your mouth is here!
> but after the end of his internship I have not seen that much 
> work on the xl-binding in libvirt except from Jim Fehlig.
AFAIK Jim is the main man working on this stuff. Last I heard he was
working on migration but I don''t know the status, Jim?

Ian.

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/




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

Ian Campbell wrote:
> On Fri, 2011-12-02 at 08:51 +0000, Philipp Hahn wrote:
>   
>> Hello,
>>
>> On Thursday 01 December 2011 19:31:04 Ian Jackson 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.
>>>       
>> Thanks for applying.
>>
>> On Thursday 01 December 2011 20:51:05 Ian Campbell wrote:
>>     
>>> xend is deprecated. This ultimately means that xend will go away.
If you
>>> are dependent on these features then you need put effort into
ensuring
>>> that they become supported in libxl and xl.
>>>       
>> For our forthcomming release of UCS-3.0 we really wanted to switch from
Xend
>> to xl, but be had to switch back to using Xend, because libvirt still
lacks
>> some important features like migration; see 
>> <http://libvirt.org/hvsupport.html> for a comparison matrix. Back
at the
>> beginning of 2011 Univention sponsored Markus Gross to work on the
xl-binding
>> of libvirt, 
>>     
>
> Thank you for putting your money where your mouth is here!
>
>   
>> but after the end of his internship I have not seen that much 
>> work on the xl-binding in libvirt except from Jim Fehlig.
>>     
>
> AFAIK Jim is the main man working on this stuff. Last I heard he was
> working on migration but I don''t know the status, Jim?
>   
As you''ve probably noticed, I haven''t had much time to devote
to libxl
and the libxl libvirt driver :-(.  Migration, and some other important
but missing features, are on my todo list, but not quite sure when I''ll
get back to this work.

Regards,
Jim

Philipp Hahn writes ("Re: [PATCHv2] insufficient quoting between
"tap-ctl list" and
xend/server/BlktapController.py"):
> 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.
Oops.  Thanks for submitting the fix.  I have applied it.

But please remember next time to include a proper signed-off-by.  In
this case, since it was only a single-character change, I applied it
anyway (the interdiff, since the previous version went in already).

Also this kind of thing shows how little review xend patches get
nowadays.  Users of xend are well-advised to switch away before they
trip over some rotting code...

Ian.