[PATCH] Error message for device not found at blkif.py

[PATCH] Error message for device not found at blkif.py

[Glauber de Oliveira Costa]

[-- Attachment #1: Type: text/plain, Size: 293 bytes --]

Hi,

if blkdev_name_to_number() fails to assign a number to the specified
device, it returns None, causing a far-away-from-self-explanatory
message to be delivered. Better test for this condition and say exactly
what happened.


-- 
Glauber de Oliveira Costa
Red Hat Inc.
"Free as in Freedom"

[-- Attachment #2: errormsg.patch --]
[-- Type: text/plain, Size: 639 bytes --]

--- xen-3.0.3-testing-11633/./tools/python/build/lib.linux-x86_64-2.4/xen/xend/server/blkif.py.orig	2006-09-28 18:52:39.000000000 -0400
+++ xen-3.0.3-testing-11633/./tools/python/build/lib.linux-x86_64-2.4/xen/xend/server/blkif.py	2006-10-18 15:58:28.000000000 -0400
@@ -81,6 +81,9 @@ class BlkifController(DevController):
                          'acm_policy' : policy})
 
         devid = blkif.blkdev_name_to_number(dev)
+        if not devid:
+            raise VmError('Unable to find number for device (%s)'%(dev))
+
         front = { 'virtual-device' : "%i" % devid,
                   'device-type' : dev_type
                 }

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

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

Re: [PATCH] Error message for device not found at blkif.py

[Anthony Liguori]

Instead of throwing a VmError, could you subclass VmError with a more 
specific error and throw that?

That will pass higher level info about the error to xm which means we 
can do a better job in there (this is especially important when we 
eventually localize xm).

Thanks,

Anthony Liguori


Glauber de Oliveira Costa wrote:
> Hi,
> 
> if blkdev_name_to_number() fails to assign a number to the specified
> device, it returns None, causing a far-away-from-self-explanatory
> message to be delivered. Better test for this condition and say exactly
> what happened.
> 
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- xen-3.0.3-testing-11633/./tools/python/build/lib.linux-x86_64-2.4/xen/xend/server/blkif.py.orig	2006-09-28 18:52:39.000000000 -0400
> +++ xen-3.0.3-testing-11633/./tools/python/build/lib.linux-x86_64-2.4/xen/xend/server/blkif.py	2006-10-18 15:58:28.000000000 -0400
> @@ -81,6 +81,9 @@ class BlkifController(DevController):
>                           'acm_policy' : policy})
>  
>          devid = blkif.blkdev_name_to_number(dev)
> +        if not devid:
> +            raise VmError('Unable to find number for device (%s)'%(dev))
> +
>          front = { 'virtual-device' : "%i" % devid,
>                    'device-type' : dev_type
>                  }
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] Error message for device not found at blkif.py

[Glauber de Oliveira Costa]

On Wed, Oct 18, 2006 at 03:59:30PM -0500, Anthony Liguori wrote:
> Instead of throwing a VmError, could you subclass VmError with a more 
> specific error and throw that?
Fore sure I can. But what does this condition have so differently from
others that justifies that for it only? Although I agree with you that 
such a specificiness is good, VmError is being thrown everywhere, meaning 
that your proposal would require touching a great amount of code change.
Is there any plans/opposals for that?
 

-- 
Glauber de Oliveira Costa
Red Hat Inc.
"Free as in Freedom"

Re: [PATCH] Error message for device not found at blkif.py

[Anthony Liguori]

Glauber de Oliveira Costa wrote:
> On Wed, Oct 18, 2006 at 03:59:30PM -0500, Anthony Liguori wrote:
>   
>> Instead of throwing a VmError, could you subclass VmError with a more 
>> specific error and throw that?
>>     
> Fore sure I can. But what does this condition have so differently from
> others that justifies that for it only?

Good question.  Previously, we only threw opaque errors back over the 
wire.  A few months ago, we changed that so that we could pass 
exceptions over the wire with specific Fault ids.  The new Xend API 
should be even better for this.

So, the answer we know have the ability to do useful things with this 
info so it's now a best practice for new code :-)

I'm not suggesting you go change everything, just in your patch.

Regards,

Anthony Liguori

>  Although I agree with you that 
> such a specificiness is good, VmError is being thrown everywhere, meaning 
> that your proposal would require touching a great amount of code change.
> Is there any plans/opposals for that?
>  
>
>   

Re: [PATCH] Error message for device not found at blkif.py

[Anthony Liguori]

Glauber de Oliveira Costa wrote:
> On Wed, Oct 18, 2006 at 03:59:30PM -0500, Anthony Liguori wrote:
>   
>> Instead of throwing a VmError, could you subclass VmError with a more 
>> specific error and throw that?
>>     
> Fore sure I can. But what does this condition have so differently from
> others that justifies that for it only? Although I agree with you that 
> such a specificiness is good, VmError is being thrown everywhere, meaning 
> that your proposal would require touching a great amount of code change.
> Is there any plans/opposals for that?
>  
>   

BTW, check out tools/python/xen/xend/XendError.py for an example of 
where we're doing this.

Regards,

Anthony Liguori