Re: [Xen-devel] [PATCH] Fix xm create command for wrong scheduler parameters

[Ewan Mellor <ewan@xxxxxxxxxxxxx>]

On Tue, Feb 06, 2007 at 03:10:48PM +0900, Masaki Kanno wrote:

Content-Description: Mail message body
> Hi,
> 
> When I tested the xm create command with wrong scheduler parameters, 
> a domain existed with the paused state. Usually, if an error occurred 
> by the xm create command, the domain isn't created. 
> The xm start command also has same issue. 
> 
> # xm create /xen/vm1.conf cpu_weight=99999
> Using config file "/xen/vm1.conf".
> Error: weight is out of range
> # xm create /xen/vm2.conf cpu_cap=999
> Using config file "/xen/vm2.conf".
> Error: cap is out of range
> # xm list
> Name                                      ID   Mem VCPUs      State   Time(s)
> Domain-0                                   0   491     2     r-----    594.5
> vm1                                        6   256     1     --p---      0.0
> vm2                                        7   256     1     --p---      0.0
> 
> 
> This patch fixes the issue. If the xm create command fails with 
> wrong scheduler parameters, it destroys the domain in the proceeding 
> of xm create command. 
> 
> 
> Signed-off-by: Masaki Kanno <kanno.masaki@xxxxxxxxxxxxxx>
> 
> Best regards,
>  Kan
> 

> diff -r 8bc64a3a5054 tools/python/xen/xend/XendDomain.py
> --- a/tools/python/xen/xend/XendDomain.py     Mon Feb 05 16:40:19 2007 +0000
> +++ b/tools/python/xen/xend/XendDomain.py     Tue Feb 06 13:43:15 2007 +0900
> @@ -868,11 +868,15 @@ class XendDomain:
>              self._refresh()
>  
>              dominfo = XendDomainInfo.create(config)
> -            if XendNode.instance().xenschedinfo() == 'credit':
> -                self.domain_sched_credit_set(dominfo.getDomid(),
> -                                             dominfo.getWeight(),
> -                                             dominfo.getCap())
> -            return dominfo
> +            try:
> +                if XendNode.instance().xenschedinfo() == 'credit':
> +                    self.domain_sched_credit_set(dominfo.getDomid(),
> +                                                 dominfo.getWeight(),
> +                                                 dominfo.getCap())
> +                return dominfo
> +            except Exception, ex:
> +                self.domain_destroy(dominfo.getDomid())
> +                raise XendError(str(ex))
>          finally:
>              self.domains_lock.release()
>  
> @@ -945,10 +949,14 @@ class XendDomain:
>                                   POWER_STATE_NAMES[dominfo.state])
>              
>              dominfo.start(is_managed = True)
> -            if XendNode.instance().xenschedinfo() == 'credit':
> -                self.domain_sched_credit_set(dominfo.getDomid(),
> -                                             dominfo.getWeight(),
> -                                             dominfo.getCap())
> +            try:
> +                if XendNode.instance().xenschedinfo() == 'credit':
> +                    self.domain_sched_credit_set(dominfo.getDomid(),
> +                                                 dominfo.getWeight(),
> +                                                 dominfo.getCap())
> +            except Exception, ex:
> +                self.domain_destroy(dominfo.getDomid())
> +                raise XendError(str(ex))
>          finally:
>              self.domains_lock.release()
>          dominfo.waitForDevices()

I think that you should move this call to the end of XendDomainInfo.start(),
which already handles exceptions and destroys the domain if necessary.  That
would save duplicating the error handling here.

Ewan.



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