* [PATCH] xen: handle TapdiskException when add device failed
@ 2014-03-20 10:36 李义
2014-03-20 11:38 ` Wei Liu
0 siblings, 1 reply; 10+ messages in thread
From: 李义 @ 2014-03-20 10:36 UTC (permalink / raw)
To: Ian Campbell, Jan Beulich, ian.jackson; +Cc: xen-devel
[-- Attachment #1: Type: text/plain, Size: 2347 bytes --]
>From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 18:25:03 +0800
Subject: [PATCH] xen: handle TapdiskException when add device failed
handle the TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/XendDomainInfo.py | 4 ++--
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/python/xen/xend/XendDomainInfo.py
b/tools/python/xen/xend/XendDomainInfo.py
index 8d4ff5c..2270ab1 100644
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
from xen.xend.XendAPIConstants import *
from xen.xend.XendCPUPool import XendCPUPool
from xen.xend.server.DevConstants import xenbusState
-from xen.xend.server.BlktapController import TapdiskController
+from xen.xend.server.BlktapController import TapdiskController,
TapdiskException
from xen.xend.XendVMMetrics import XendVMMetrics
@@ -861,7 +861,7 @@ class XendDomainInfo:
# blktap1
dev_type = self.getBlockDeviceClass(devid)
self._waitForDevice(dev_type, devid)
- except VmError, ex:
+ except (VmError, TapdiskException), ex:
del self.info['devices'][dev_uuid]
if dev_type == 'pci':
for dev in dev_config_dict['devs']:
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..d28fb68 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname=args[1]
+ uname_list=uname.split(":")
+ back_dev=uname_list[len(uname_list)-1]
+ front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
[-- Attachment #2: 0001-xen-handle-TapdiskException-when-add-device-failed.patch --]
[-- Type: application/octet-stream, Size: 2350 bytes --]
From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 18:25:03 +0800
Subject: [PATCH] xen: handle TapdiskException when add device failed
handle the TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/XendDomainInfo.py | 4 ++--
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/python/xen/xend/XendDomainInfo.py b/tools/python/xen/xend/XendDomainInfo.py
index 8d4ff5c..2270ab1 100644
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
from xen.xend.XendAPIConstants import *
from xen.xend.XendCPUPool import XendCPUPool
from xen.xend.server.DevConstants import xenbusState
-from xen.xend.server.BlktapController import TapdiskController
+from xen.xend.server.BlktapController import TapdiskController, TapdiskException
from xen.xend.XendVMMetrics import XendVMMetrics
@@ -861,7 +861,7 @@ class XendDomainInfo:
# blktap1
dev_type = self.getBlockDeviceClass(devid)
self._waitForDevice(dev_type, devid)
- except VmError, ex:
+ except (VmError, TapdiskException), ex:
del self.info['devices'][dev_uuid]
if dev_type == 'pci':
for dev in dev_config_dict['devs']:
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..d28fb68 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname=args[1]
+ uname_list=uname.split(":")
+ back_dev=uname_list[len(uname_list)-1]
+ front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 10:36 [PATCH] xen: handle TapdiskException when add device failed 李义
@ 2014-03-20 11:38 ` Wei Liu
2014-03-20 12:13 ` 李义
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Wei Liu @ 2014-03-20 11:38 UTC (permalink / raw)
To: 李义
Cc: wei.liu2, ian.jackson, Ian Campbell, Jan Beulich, xen-devel
On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
> >From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
> From: Yi Li <peteryili@tencent.com>
> Date: Thu, 20 Mar 2014 18:25:03 +0800
> Subject: [PATCH] xen: handle TapdiskException when add device failed
>
> handle the TapdiskException when add device
> and check the args when using xm block-attach
>
> Signed-off-by: Yi Li <peteryili@tencent.com>
> ---
> tools/python/xen/xend/XendDomainInfo.py | 4 ++--
> tools/python/xen/xm/main.py | 12 ++++++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/tools/python/xen/xend/XendDomainInfo.py
> b/tools/python/xen/xend/XendDomainInfo.py
> index 8d4ff5c..2270ab1 100644
> --- a/tools/python/xen/xend/XendDomainInfo.py
> +++ b/tools/python/xen/xend/XendDomainInfo.py
> @@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
> from xen.xend.XendAPIConstants import *
> from xen.xend.XendCPUPool import XendCPUPool
> from xen.xend.server.DevConstants import xenbusState
> -from xen.xend.server.BlktapController import TapdiskController
> +from xen.xend.server.BlktapController import TapdiskController,
> TapdiskException
>
This line is wrapped by your email client.
> from xen.xend.XendVMMetrics import XendVMMetrics
>
> @@ -861,7 +861,7 @@ class XendDomainInfo:
> # blktap1
> dev_type = self.getBlockDeviceClass(devid)
> self._waitForDevice(dev_type, devid)
> - except VmError, ex:
> + except (VmError, TapdiskException), ex:
> del self.info['devices'][dev_uuid]
> if dev_type == 'pci':
> for dev in dev_config_dict['devs']:
> diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
> index 5ed781c..d28fb68 100644
> --- a/tools/python/xen/xm/main.py
> +++ b/tools/python/xen/xm/main.py
> @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
> def xm_block_attach(args):
> arg_check(args, 'block-attach', 4, 5)
>
> + uname=args[1]
> + uname_list=uname.split(":")
> + back_dev=uname_list[len(uname_list)-1]
> + front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
Coding style: need space between "=".
And hardcoding a path in code is not very good practice. Further more
what's the rationale to restrict everything in "/dev"? Isn't the most
rationale thing to do is for user to use the exact path he / she
provides?
> + if not os.path.exists(back_dev):
> + print "Error: %s not exist " % back_dev
> + sys.exit(1)
> +
> + if os.path.exists(front_dev):
> + print "Error: %s already exists " % front_dev
> + sys.exit(1)
> +
> if serverType == SERVER_XEN_API:
> dom = args[0]
> uname = args[1]
> --
> 1.7.1
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 11:38 ` Wei Liu
@ 2014-03-20 12:13 ` 李义
2014-03-20 12:47 ` Fwd: " 李义
2014-03-20 14:18 ` 李义
[not found] ` <CAJfdMYBwS6FDGF5-m8OC1CLZpMQ-7bviHzSOQkVr7XFN0=Hccw@mail.gmail.com>
` (2 subsequent siblings)
3 siblings, 2 replies; 10+ messages in thread
From: 李义 @ 2014-03-20 12:13 UTC (permalink / raw)
To: Wei Liu, yilikernel; +Cc: ian.jackson, Ian Campbell, Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 5633 bytes --]
>From 2098e71261cf6a8b65524033f321d80a4f70c29d Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 19:55:10 +0800
Subject: [PATCH] xen: handle TapdiskException when add device failed
handle the TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/XendDomainInfo.py | 4 ++--
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/python/xen/xend/XendDomainInfo.py
b/tools/python/xen/xend/XendDomainInfo.py
index 8d4ff5c..2270ab1 100644
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
from xen.xend.XendAPIConstants import *
from xen.xend.XendCPUPool import XendCPUPool
from xen.xend.server.DevConstants import xenbusState
-from xen.xend.server.BlktapController import TapdiskController
+from xen.xend.server.BlktapController import TapdiskController,
TapdiskException
from xen.xend.XendVMMetrics import XendVMMetrics
@@ -861,7 +861,7 @@ class XendDomainInfo:
# blktap1
dev_type = self.getBlockDeviceClass(devid)
self._waitForDevice(dev_type, devid)
- except VmError, ex:
+ except (VmError, TapdiskException), ex:
del self.info['devices'][dev_uuid]
if dev_type == 'pci':
for dev in dev_config_dict['devs']:
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..ce5d9a2 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname = args[1]
+ uname_list = uname.split(":")
+ back_dev = uname_list[len(uname_list)-1]
+ front_dev = args[2]
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
2014-03-20 19:38 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
> On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
> > >From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
> > From: Yi Li <peteryili@tencent.com>
> > Date: Thu, 20 Mar 2014 18:25:03 +0800
> > Subject: [PATCH] xen: handle TapdiskException when add device failed
> >
> > handle the TapdiskException when add device
> > and check the args when using xm block-attach
> >
> > Signed-off-by: Yi Li <peteryili@tencent.com>
> > ---
> > tools/python/xen/xend/XendDomainInfo.py | 4 ++--
> > tools/python/xen/xm/main.py | 12 ++++++++++++
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/python/xen/xend/XendDomainInfo.py
> > b/tools/python/xen/xend/XendDomainInfo.py
> > index 8d4ff5c..2270ab1 100644
> > --- a/tools/python/xen/xend/XendDomainInfo.py
> > +++ b/tools/python/xen/xend/XendDomainInfo.py
> > @@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
> > from xen.xend.XendAPIConstants import *
> > from xen.xend.XendCPUPool import XendCPUPool
> > from xen.xend.server.DevConstants import xenbusState
> > -from xen.xend.server.BlktapController import TapdiskController
> > +from xen.xend.server.BlktapController import TapdiskController,
> > TapdiskException
> >
>
> This line is wrapped by your email client.
>
> > from xen.xend.XendVMMetrics import XendVMMetrics
> >
> > @@ -861,7 +861,7 @@ class XendDomainInfo:
> > # blktap1
> > dev_type = self.getBlockDeviceClass(devid)
> > self._waitForDevice(dev_type, devid)
> > - except VmError, ex:
> > + except (VmError, TapdiskException), ex:
> > del self.info['devices'][dev_uuid]
> > if dev_type == 'pci':
> > for dev in dev_config_dict['devs']:
> > diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
> > index 5ed781c..d28fb68 100644
> > --- a/tools/python/xen/xm/main.py
> > +++ b/tools/python/xen/xm/main.py
> > @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
> > def xm_block_attach(args):
> > arg_check(args, 'block-attach', 4, 5)
> >
> > + uname=args[1]
> > + uname_list=uname.split(":")
> > + back_dev=uname_list[len(uname_list)-1]
> > + front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
>
> Coding style: need space between "=".
>
> And hardcoding a path in code is not very good practice. Further more
> what's the rationale to restrict everything in "/dev"? Isn't the most
> rationale thing to do is for user to use the exact path he / she
> provides?
>
> > + if not os.path.exists(back_dev):
> > + print "Error: %s not exist " % back_dev
> > + sys.exit(1)
> > +
> > + if os.path.exists(front_dev):
> > + print "Error: %s already exists " % front_dev
> > + sys.exit(1)
> > +
> > if serverType == SERVER_XEN_API:
> > dom = args[0]
> > uname = args[1]
> > --
> > 1.7.1
>
>
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
>
>
[-- Attachment #1.2: Type: text/html, Size: 7325 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
[not found] ` <20140320122506.GH16807@zion.uk.xensource.com>
@ 2014-03-20 12:27 ` Wei Liu
0 siblings, 0 replies; 10+ messages in thread
From: Wei Liu @ 2014-03-20 12:27 UTC (permalink / raw)
To: 李义; +Cc: Wei Liu, xen-devel
CC Xen-devel...
On Thu, Mar 20, 2014 at 12:25:06PM +0000, Wei Liu wrote:
> You forgot to CC xen-devel. And plese don't top-post...
>
> On Thu, Mar 20, 2014 at 07:58:13PM +0800, 李义 wrote:
> > >From 2098e71261cf6a8b65524033f321d80a4f70c29d Mon Sep 17 00:00:00 2001
> > From: Yi Li <peteryili@tencent.com>
> > Date: Thu, 20 Mar 2014 19:55:10 +0800
> > Subject: [PATCH] xen: handle TapdiskException when add device failed
> >
> > handle the TapdiskException when add device
> > and check the args when using xm block-attach
> > Signed-off-by: Yi Li <peteryili@tencent.com>
> > ---
> > tools/python/xen/xend/XendDomainInfo.py | 4 ++--
> > tools/python/xen/xm/main.py | 12 ++++++++++++
> > 2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/python/xen/xend/XendDomainInfo.py
> > b/tools/python/xen/xend/XendDomainInfo.py
> > index 8d4ff5c..2270ab1 100644
> > --- a/tools/python/xen/xend/XendDomainInfo.py
> > +++ b/tools/python/xen/xend/XendDomainInfo.py
> > @@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
> > from xen.xend.XendAPIConstants import *
> > from xen.xend.XendCPUPool import XendCPUPool
> > from xen.xend.server.DevConstants import xenbusState
> > -from xen.xend.server.BlktapController import TapdiskController
> > +from xen.xend.server.BlktapController import TapdiskController,
> > TapdiskException
> >
>
> This line is still damaged. You need to tweak your email client
> (presumably gmail) to stop doing that. Or teach git send-email to use
> your SMTP directly instead of copy-and-paste.
>
> > from xen.xend.XendVMMetrics import XendVMMetrics
> >
> > @@ -861,7 +861,7 @@ class XendDomainInfo:
> > # blktap1
> > dev_type = self.getBlockDeviceClass(devid)
> > self._waitForDevice(dev_type, devid)
> > - except VmError, ex:
> > + except (VmError, TapdiskException), ex:
>
> (Not claiming to be a Xend expert, but from a maintainability point of
> view.)
>
> I don't understand why you can catch a TapdiskException at this point.
> Shouldn't it be caught during VM creation and represented as VmError?
>
> >From the look of various callsite of "raise VmError", it does show that
> when you fail to create a device, VmError is raised. And the exception
> handling code suggests that as well.
>
> > del self.info['devices'][dev_uuid]
> > if dev_type == 'pci':
> > for dev in dev_config_dict['devs']:
> > diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
> > index 5ed781c..ce5d9a2 100644
> > --- a/tools/python/xen/xm/main.py
> > +++ b/tools/python/xen/xm/main.py
> > @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
> > def xm_block_attach(args):
> > arg_check(args, 'block-attach', 4, 5)
> >
> > + uname = args[1]
> > + uname_list = uname.split(":")
> > + back_dev = uname_list[len(uname_list)-1]
> > + front_dev = args[2]
> > + if not os.path.exists(back_dev):
> > + print "Error: %s not exist " % back_dev
> > + sys.exit(1)
> > +
> > + if os.path.exists(front_dev):
> > + print "Error: %s already exists " % front_dev
> > + sys.exit(1)
> > +
> > if serverType == SERVER_XEN_API:
> > dom = args[0]
> > uname = args[1]
> > --
> > 1.7.1
> >
>
> You can strip the rest if you're sending a new patch.
>
> Wei.
>
> > 2014-03-20 19:38 GMT+08:00, Wei Liu <wei.liu2@citrix.com>:
> > > On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
> > >> >From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
> > >> From: Yi Li <peteryili@tencent.com>
> > >> Date: Thu, 20 Mar 2014 18:25:03 +0800
> > >> Subject: [PATCH] xen: handle TapdiskException when add device failed
> > >>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Fwd: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 12:13 ` 李义
@ 2014-03-20 12:47 ` 李义
2014-03-20 14:18 ` 李义
1 sibling, 0 replies; 10+ messages in thread
From: 李义 @ 2014-03-20 12:47 UTC (permalink / raw)
To: Wei Liu, yilikernel; +Cc: ian.jackson, Ian Campbell, Jan Beulich, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2307 bytes --]
>From 2098e71261cf6a8b65524033f321d80a4f70c29d Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 19:55:10 +0800
Subject: [PATCH] xen: handle TapdiskException when add device failed
handle the TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/XendDomainInfo.py | 4 ++--
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/tools/python/xen/xend/XendDomainInfo.py
b/tools/python/xen/xend/XendDomainInfo.py
index 8d4ff5c..2270ab1 100644
--- a/tools/python/xen/xend/XendDomainInfo.py
+++ b/tools/python/xen/xend/XendDomainInfo.py
@@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
from xen.xend.XendAPIConstants import *
from xen.xend.XendCPUPool import XendCPUPool
from xen.xend.server.DevConstants import xenbusState
-from xen.xend.server.BlktapController import TapdiskController
+from xen.xend.server.BlktapController import TapdiskController,
TapdiskException
from xen.xend.XendVMMetrics import XendVMMetrics
@@ -861,7 +861,7 @@ class XendDomainInfo:
# blktap1
dev_type = self.getBlockDeviceClass(devid)
self._waitForDevice(dev_type, devid)
- except VmError, ex:
+ except (VmError, TapdiskException), ex:
del self.info['devices'][dev_uuid]
if dev_type == 'pci':
for dev in dev_config_dict['devs']:
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..ce5d9a2 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname = args[1]
+ uname_list = uname.split(":")
+ back_dev = uname_list[len(uname_list)-1]
+ front_dev = args[2]
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
[-- Attachment #1.2: Type: text/html, Size: 3088 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 11:38 ` Wei Liu
2014-03-20 12:13 ` 李义
[not found] ` <CAJfdMYBwS6FDGF5-m8OC1CLZpMQ-7bviHzSOQkVr7XFN0=Hccw@mail.gmail.com>
@ 2014-03-20 12:55 ` Jan Beulich
2014-03-20 13:05 ` 李义
2014-03-20 13:28 ` Ian Jackson
3 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2014-03-20 12:55 UTC (permalink / raw)
To: Wei Liu, yilikernel; +Cc: ian.jackson, Ian Campbell, xen-devel
>>> On 20.03.14 at 12:38, Wei Liu <wei.liu2@citrix.com> wrote:
> On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
>> diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
>> index 5ed781c..d28fb68 100644
>> --- a/tools/python/xen/xm/main.py
>> +++ b/tools/python/xen/xm/main.py
>> @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
>> def xm_block_attach(args):
>> arg_check(args, 'block-attach', 4, 5)
>>
>> + uname=args[1]
>> + uname_list=uname.split(":")
>> + back_dev=uname_list[len(uname_list)-1]
>> + front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
>
> Coding style: need space between "=".
>
> And hardcoding a path in code is not very good practice. Further more
> what's the rationale to restrict everything in "/dev"? Isn't the most
> rationale thing to do is for user to use the exact path he / she
> provides?
And iirc this kind of conditional isn't being supported by all Python
versions we support.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 12:55 ` Jan Beulich
@ 2014-03-20 13:05 ` 李义
2014-03-20 13:31 ` Ian Jackson
0 siblings, 1 reply; 10+ messages in thread
From: 李义 @ 2014-03-20 13:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: ian.jackson, Wei Liu, Ian Campbell, xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2173 bytes --]
>From 5cc4dc68d29b01cc08761813af0c2eb6831f0260 Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 20:58:55 +0800
Subject: [PATCH] xen: remove TapdiskException when add device
using the VmError instead of TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/server/BlktapController.py | 5 +----
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/tools/python/xen/xend/server/BlktapController.py
b/tools/python/xen/xend/server/BlktapController.py
index 60079eb..c8e5e24 100644
--- a/tools/python/xen/xend/server/BlktapController.py
+++ b/tools/python/xen/xend/server/BlktapController.py
@@ -198,9 +198,6 @@ class Blktap2Controller(BlktapController):
self.waitForBackend_destroy(backpath)
TapdiskController.destroy(path)
-class TapdiskException(Exception):
- pass
-
class TapdiskController(object):
'''class which encapsulates all tapdisk control operations'''
@@ -229,7 +226,7 @@ class TapdiskController(object):
stdout.close()
stderr.close()
if rc:
- raise TapdiskException('%s failed (%s %s %s)' % \
+ raise VmError('%s failed (%s %s %s)' % \
(args, rc, out, err))
return out
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..ce5d9a2 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname = args[1]
+ uname_list = uname.split(":")
+ back_dev = uname_list[len(uname_list)-1]
+ front_dev = args[2]
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
[-- Attachment #1.2: Type: text/html, Size: 2802 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 11:38 ` Wei Liu
` (2 preceding siblings ...)
2014-03-20 12:55 ` Jan Beulich
@ 2014-03-20 13:28 ` Ian Jackson
3 siblings, 0 replies; 10+ messages in thread
From: Ian Jackson @ 2014-03-20 13:28 UTC (permalink / raw)
To: Wei Liu
Cc: 李义, ian.jackson, Ian Campbell, Jan Beulich, xen-devel
Wei Liu writes ("Re: [Xen-devel] [PATCH] xen: handle TapdiskException when add device failed"):
> On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
> > handle the TapdiskException when add device
> > and check the args when using xm block-attach
...
> > + if not os.path.exists(back_dev):
> > + print "Error: %s not exist " % back_dev
> > + sys.exit(1)
I worry that this will break something and that we don't really
understand when tapdisk things are paths and when they might be
something else (iscsi target strings perhaps?)
I think at this stage in xm/xend's life I don't want to change this.
The exception handling part is probably OK...
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 13:05 ` 李义
@ 2014-03-20 13:31 ` Ian Jackson
0 siblings, 0 replies; 10+ messages in thread
From: Ian Jackson @ 2014-03-20 13:31 UTC (permalink / raw)
To: 李义; +Cc: Wei Liu, Ian Campbell, Jan Beulich, xen-devel
李义 writes ("Re: [Xen-devel] [PATCH] xen: handle TapdiskException when add device failed"):
> From 5cc4dc68d29b01cc08761813af0c2eb6831f0260 Mon Sep 17 00:00:00 2001
> From: Yi Li <peteryili@tencent.com>
> Date: Thu, 20 Mar 2014 20:58:55 +0800
> Subject: [PATCH] xen: remove TapdiskException when add device
>
> using the VmError instead of TapdiskException when add device
> and check the args when using xm block-attach
Thanks for these resubmissions. If I may comment: it's probably wise
to wait a bit before respinning your patch, in case of comments from
other reviewers (who may disagree with the approach or indeed with the
first people to reply).
Also, when you resubmit, please include a version number, like this:
Subject: [PATCH v5] xen: remove TapdiskException when add device
(although I've lost track of what version we're on...)
git-format-patch has an option to do that.
Thanks,
Ian.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] xen: handle TapdiskException when add device failed
2014-03-20 12:13 ` 李义
2014-03-20 12:47 ` Fwd: " 李义
@ 2014-03-20 14:18 ` 李义
1 sibling, 0 replies; 10+ messages in thread
From: 李义 @ 2014-03-20 14:18 UTC (permalink / raw)
To: xen-devel; +Cc: ian.jackson, 李义, Wei Liu
[-- Attachment #1.1: Type: text/plain, Size: 8151 bytes --]
>From 367b54c018885d5a72bf7d5e76b71516d0342aec Mon Sep 17 00:00:00 2001
From: Yi Li <peteryili@tencent.com>
Date: Thu, 20 Mar 2014 21:52:05 +0800
Subject: [PATCH v3] xen: remove TapdiskException when add device
using the VmError instead of TapdiskException when add device
and check the args when using xm block-attach
Signed-off-by: Yi Li <peteryili@tencent.com>
---
tools/python/xen/xend/server/BlktapController.py | 5 +----
tools/python/xen/xm/main.py | 12 ++++++++++++
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/tools/python/xen/xend/server/BlktapController.py
b/tools/python/xen/xend/server/BlktapController.py
index 60079eb..c8e5e24 100644
--- a/tools/python/xen/xend/server/BlktapController.py
+++ b/tools/python/xen/xend/server/BlktapController.py
@@ -198,9 +198,6 @@ class Blktap2Controller(BlktapController):
self.waitForBackend_destroy(backpath)
TapdiskController.destroy(path)
-class TapdiskException(Exception):
- pass
-
class TapdiskController(object):
'''class which encapsulates all tapdisk control operations'''
@@ -229,7 +226,7 @@ class TapdiskController(object):
stdout.close()
stderr.close()
if rc:
- raise TapdiskException('%s failed (%s %s %s)' % \
+ raise VmError('%s failed (%s %s %s)' % \
(args, rc, out, err))
return out
diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
index 5ed781c..ce5d9a2 100644
--- a/tools/python/xen/xm/main.py
+++ b/tools/python/xen/xm/main.py
@@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
def xm_block_attach(args):
arg_check(args, 'block-attach', 4, 5)
+ uname = args[1]
+ uname_list = uname.split(":")
+ back_dev = uname_list[len(uname_list)-1]
+ front_dev = args[2]
+ if not os.path.exists(back_dev):
+ print "Error: %s not exist " % back_dev
+ sys.exit(1)
+
+ if os.path.exists(front_dev):
+ print "Error: %s already exists " % front_dev
+ sys.exit(1)
+
if serverType == SERVER_XEN_API:
dom = args[0]
uname = args[1]
--
1.7.1
2014-03-20 20:13 GMT+08:00 李义 <yilikernel@gmail.com>:
> From 2098e71261cf6a8b65524033f321d80a4f70c29d Mon Sep 17 00:00:00 2001
> From: Yi Li <peteryili@tencent.com>
> Date: Thu, 20 Mar 2014 19:55:10 +0800
>
> Subject: [PATCH] xen: handle TapdiskException when add device failed
>
> handle the TapdiskException when add device
> and check the args when using xm block-attach
> Signed-off-by: Yi Li <peteryili@tencent.com>
> ---
> tools/python/xen/xend/XendDomainInfo.py | 4 ++--
> tools/python/xen/xm/main.py | 12 ++++++++++++
> 2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/tools/python/xen/xend/XendDomainInfo.py
> b/tools/python/xen/xend/XendDomainInfo.py
> index 8d4ff5c..2270ab1 100644
> --- a/tools/python/xen/xend/XendDomainInfo.py
> +++ b/tools/python/xen/xend/XendDomainInfo.py
> @@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
> from xen.xend.XendAPIConstants import *
> from xen.xend.XendCPUPool import XendCPUPool
> from xen.xend.server.DevConstants import xenbusState
> -from xen.xend.server.BlktapController import TapdiskController
> +from xen.xend.server.BlktapController import TapdiskController,
> TapdiskException
>
> from xen.xend.XendVMMetrics import XendVMMetrics
>
> @@ -861,7 +861,7 @@ class XendDomainInfo:
> # blktap1
> dev_type = self.getBlockDeviceClass(devid)
> self._waitForDevice(dev_type, devid)
> - except VmError, ex:
> + except (VmError, TapdiskException), ex:
> del self.info['devices'][dev_uuid]
> if dev_type == 'pci':
> for dev in dev_config_dict['devs']:
> diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
> index 5ed781c..ce5d9a2 100644
>
> --- a/tools/python/xen/xm/main.py
> +++ b/tools/python/xen/xm/main.py
> @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
> def xm_block_attach(args):
> arg_check(args, 'block-attach', 4, 5)
>
> + uname = args[1]
> + uname_list = uname.split(":")
> + back_dev = uname_list[len(uname_list)-1]
> + front_dev = args[2]
> + if not os.path.exists(back_dev):
> + print "Error: %s not exist " % back_dev
> + sys.exit(1)
> +
> + if os.path.exists(front_dev):
> + print "Error: %s already exists " % front_dev
> + sys.exit(1)
> +
> if serverType == SERVER_XEN_API:
> dom = args[0]
> uname = args[1]
> --
> 1.7.1
>
>
> 2014-03-20 19:38 GMT+08:00 Wei Liu <wei.liu2@citrix.com>:
>
> On Thu, Mar 20, 2014 at 06:36:54PM +0800, 李义 wrote:
>> > >From 79220a11ce64324228ba0390790773b41bf5f156 Mon Sep 17 00:00:00 2001
>> > From: Yi Li <peteryili@tencent.com>
>> > Date: Thu, 20 Mar 2014 18:25:03 +0800
>> > Subject: [PATCH] xen: handle TapdiskException when add device failed
>> >
>> > handle the TapdiskException when add device
>> > and check the args when using xm block-attach
>> >
>> > Signed-off-by: Yi Li <peteryili@tencent.com>
>> > ---
>> > tools/python/xen/xend/XendDomainInfo.py | 4 ++--
>> > tools/python/xen/xm/main.py | 12 ++++++++++++
>> > 2 files changed, 14 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/tools/python/xen/xend/XendDomainInfo.py
>> > b/tools/python/xen/xend/XendDomainInfo.py
>> > index 8d4ff5c..2270ab1 100644
>> > --- a/tools/python/xen/xend/XendDomainInfo.py
>> > +++ b/tools/python/xen/xend/XendDomainInfo.py
>> > @@ -65,7 +65,7 @@ from xen.xend.XendConstants import *
>> > from xen.xend.XendAPIConstants import *
>> > from xen.xend.XendCPUPool import XendCPUPool
>> > from xen.xend.server.DevConstants import xenbusState
>> > -from xen.xend.server.BlktapController import TapdiskController
>> > +from xen.xend.server.BlktapController import TapdiskController,
>> > TapdiskException
>> >
>>
>> This line is wrapped by your email client.
>>
>> > from xen.xend.XendVMMetrics import XendVMMetrics
>> >
>> > @@ -861,7 +861,7 @@ class XendDomainInfo:
>> > # blktap1
>> > dev_type = self.getBlockDeviceClass(devid)
>> > self._waitForDevice(dev_type, devid)
>> > - except VmError, ex:
>> > + except (VmError, TapdiskException), ex:
>> > del self.info['devices'][dev_uuid]
>> > if dev_type == 'pci':
>> > for dev in dev_config_dict['devs']:
>> > diff --git a/tools/python/xen/xm/main.py b/tools/python/xen/xm/main.py
>> > index 5ed781c..d28fb68 100644
>> > --- a/tools/python/xen/xm/main.py
>> > +++ b/tools/python/xen/xm/main.py
>> > @@ -2650,6 +2650,18 @@ def parse_block_configuration(args):
>> > def xm_block_attach(args):
>> > arg_check(args, 'block-attach', 4, 5)
>> >
>> > + uname=args[1]
>> > + uname_list=uname.split(":")
>> > + back_dev=uname_list[len(uname_list)-1]
>> > + front_dev=args[2] if ("/dev" in args[2]) else ("/dev/" + args[2])
>>
>> Coding style: need space between "=".
>>
>> And hardcoding a path in code is not very good practice. Further more
>> what's the rationale to restrict everything in "/dev"? Isn't the most
>> rationale thing to do is for user to use the exact path he / she
>> provides?
>>
>> > + if not os.path.exists(back_dev):
>> > + print "Error: %s not exist " % back_dev
>> > + sys.exit(1)
>> > +
>> > + if os.path.exists(front_dev):
>> > + print "Error: %s already exists " % front_dev
>> > + sys.exit(1)
>> > +
>> > if serverType == SERVER_XEN_API:
>> > dom = args[0]
>> > uname = args[1]
>> > --
>> > 1.7.1
>>
>>
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xen.org
>> > http://lists.xen.org/xen-devel
>>
>>
>
[-- Attachment #1.2: Type: text/html, Size: 10733 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2014-03-20 14:18 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-20 10:36 [PATCH] xen: handle TapdiskException when add device failed 李义
2014-03-20 11:38 ` Wei Liu
2014-03-20 12:13 ` 李义
2014-03-20 12:47 ` Fwd: " 李义
2014-03-20 14:18 ` 李义
[not found] ` <CAJfdMYBwS6FDGF5-m8OC1CLZpMQ-7bviHzSOQkVr7XFN0=Hccw@mail.gmail.com>
[not found] ` <20140320122506.GH16807@zion.uk.xensource.com>
2014-03-20 12:27 ` Wei Liu
2014-03-20 12:55 ` Jan Beulich
2014-03-20 13:05 ` 李义
2014-03-20 13:31 ` Ian Jackson
2014-03-20 13:28 ` Ian Jackson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.