Xen devel - Sep 2005 - Re: [Xen-changelog] The dom destroy path is doing a dom_get on a non-existent domain to

> # HG changeset patch
> # User shand@ubuntu.eng.hq.xensource.com
> # Node ID fb72c78604c673bb07915675620bce3b7630e323
> # Parent  122779b57b40600dcaf2d147ca82ec23da8e2c96
<snip>
> It then handles the exception in dom_get by returning None, which
> callers already expect to mean failure.
This is not actually true, right?  There are several places in the
code that blindly use the return value of dom_get(), assuming that it
is actually anything but None.  The closest example is in the update()
function, several lines down:

  self.info = info or dom_get(self.domid)
  self.memory = self.info[''mem_kb''] / 1024

Clearly, we need to decide what behavior dom_get() is expected to
have.  Do we expect it to return None if the domid is not found, or do
we expect the domain we''re asking about to always be present
(i.e. throw an exception if not)?

If nobody has strong feelings either way, I''ll work up a patch to fix
it.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms@us.ibm.com


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

On Sep 14, 2005, at 18:07 , Dan Smith wrote:
>
>
>> # HG changeset patch
>> # User shand@ubuntu.eng.hq.xensource.com
>> # Node ID fb72c78604c673bb07915675620bce3b7630e323
>> # Parent  122779b57b40600dcaf2d147ca82ec23da8e2c96
>>
>
> <snip>
>
>
>> It then handles the exception in dom_get by returning None, which
>> callers already expect to mean failure.
>>
>
> This is not actually true, right?  There are several places in the
> code that blindly use the return value of dom_get(), assuming that it
> is actually anything but None.  The closest example is in the update()
> function, several lines down:
>
>   self.info = info or dom_get(self.domid)
Ah, my bad. I misinterpreted this as a boolean expression at first  
glance.  Note, dom_get was already returning None when the domain  
wasn''t found, but another exception was preventing it from getting  
that far. We''re just moving the exceptions around, now.

>   self.memory = self.info[''mem_kb''] / 1024
>
> Clearly, we need to decide what behavior dom_get() is expected to
> have.  Do we expect it to return None if the domid is not found, or do
> we expect the domain we''re asking about to always be present
> (i.e. throw an exception if not)?
>
> If nobody has strong feelings either way, I''ll work up a patch to
fix
> it.
My original fix was to allow dom_get to raise the exception, and I  
just ignored it in delete(), since we are not expecting to find the  
domain in the first place there.  On second thought, this was  
probably a better approach, since the other uses of dom_get would be  
better off if they got exceptions instead of None.

robert
>
> -- 
> Dan Smith
> IBM Linux Technology Center
> Open Hypervisor Team
> email: danms@us.ibm.com
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
--
Robert Read
robert@xensource.com



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

> > This is not actually true, right?  There are several places in the
> > code that blindly use the return value of dom_get(), assuming that it
> > is actually anything but None.  The closest example is in the update()
> > function, several lines down:
> >
> >   self.info = info or dom_get(self.domid)
>
> Ah, my bad. I misinterpreted this as a boolean expression at first
> glance.  Note, dom_get was already returning None when the domain
> wasn''t found, but another exception was preventing it from getting
> that far. We''re just moving the exceptions around, now.
Well really it is a boolean expression but it follows the Perl/Python 
convention of using short-circuit evaluation of booleans for things like 
control flow.  It''s fine once you''ve run into it.  My
favourite is the Python
idiom for the ternary conditional:

a = (b and c) or d)

Which actually means (assuming c isn''t false, or equivalent):

a = b ? c : d;

:-D

Cheers,
Mark
> >   self.memory = self.info[''mem_kb''] / 1024
> >
> > Clearly, we need to decide what behavior dom_get() is expected to
> > have.  Do we expect it to return None if the domid is not found, or do
> > we expect the domain we''re asking about to always be present
> > (i.e. throw an exception if not)?
> >
> > If nobody has strong feelings either way, I''ll work up a
patch to fix
> > it.
>
> My original fix was to allow dom_get to raise the exception, and I
> just ignored it in delete(), since we are not expecting to find the
> domain in the first place there.  On second thought, this was
> probably a better approach, since the other uses of dom_get would be
> better off if they got exceptions instead of None.
>
> robert
>
> > --
> > Dan Smith
> > IBM Linux Technology Center
> > Open Hypervisor Team
> > email: danms@us.ibm.com
> >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
> --
> Robert Read
> robert@xensource.com
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel