Re: [PATCH] Fix xm shutdown

[Masaki Kanno <kanno.masaki@jp.fujitsu.com>]

Hi Ewan,

Thanks for your advice.
I thought that the purpose of checking in shutdown() was for -all option,
so I approved twice checking.

The following is new patch.

Best Regards,
 Kan

diff -r 0255f48b757f tools/python/xen/xm/shutdown.py
--- a/tools/python/xen/xm/shutdown.py   Sun Dec  4 19:12:00 2005
+++ b/tools/python/xen/xm/shutdown.py   Wed Dec 14 10:17:38 2005
@@ -57,7 +57,10 @@
     dom0_name = sxp.child_value(server.xend_domain(0), 'name')
     for x in [dom0_name, DOM0_ID]:
         if x in doms:
-            doms.remove(x)
+            if opts.vals.all:
+                doms.remove(x)
+            else:
+                opts.err("Can't specify Domain-0")
     for d in doms:
         server.xend_domain_shutdown(d, mode)
     if wait:


Ewan Mellor wrote:
>On Tue, Dec 13, 2005 at 06:54:05PM +0900, Masaki Kanno wrote:
>
>> Hi,
>> 
>> This patch adds error message to xm shutdown command. 
>> 
>> I ran xm shutdown command specifying Domain-0. (example: xm shutdown 0)
>> However, the error message was not shown. 
>> I think that the error message should be shown.
>> 
>> Signed-off-by: Masaki Kanno <kanno.masaki@jp.fujitsu.com>
>> 
>> Best Regards,
>>  Kan
>> 
>> diff -r 0255f48b757f tools/python/xen/xm/shutdown.py
>> --- a/tools/python/xen/xm/shutdown.py   Sun Dec  4 19:12:00 2005
>> +++ b/tools/python/xen/xm/shutdown.py   Tue Dec 13 13:28:26 2005
>> @@ -92,6 +92,10 @@
>>      if len(args) < 1: opts.err('Missing domain')
>>      dom = args[0]
>>      mode = shutdown_mode(opts)  
>> +    dom0_name = sxp.child_value(server.xend_domain(0), 'name')
>> +    for x in [dom0_name, DOM0_ID]:
>> +        if x in dom:
>> +            opts.err("Can't specify Domain-0")
>>      shutdown(opts, [ dom ], mode, opts.vals.wait)
>>      
>>  def main(argv):
>
>I agree that an error message would be better than silently ignoring this.
>However, your patch now leaves us performing this check twice -- once in
>main_dom(), and once in shutdown().  Could you not just show the error message
>if the check fails in shutdown() instead?
>
>Ewan.