Re: [PATCH] libvirt_vm.py: Set ignore_status to False in virsh_cmd().

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

On 01/11/2012 05:06 AM, tangchen wrote:
> utils.run()'s parameter "ignore_status" is set to "True" in virsh_cmd().
> In this case we are not able to know whether the command succeeds.
> This patch sets it to "False", and utils.run() will throw an exception
> when command fails.

Problem with this is that some commands may fail and that is not a fatal 
problem, for example:

13:49:26 ERROR| Test failed: CmdError: Command </usr/bin/virsh -c 
qemu:///system domstate vm1> failed, rc=1, Command returned non-zero 
exit status    [context: preprocessing]
* Command:
     /usr/bin/virsh -c qemu:///system domstate vm1
Exit status: 1
Duration: 0.0250420570374

stderr:
error: failed to get domain 'vm1'
error: Domain not found: no domain with matching name 'vm1'

This is just a function to probe whether the domain exists or not, so it 
shouldn't throw an exception. I agree we can do better handling of 
failures, but a more thorough patch handling failures on the functions 
dependent on virsh_cmd is needed.

So I'm rejecting the patch. Feel free to open an issue contemplating 
this perceived problem, and we can work towards a more appropriate patch.

>
> Signed-off-by: Tang Chen<tangchen@cn.fujitsu.com>
> ---
>   client/virt/libvirt_vm.py |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py
> index c825661..6f30f36 100644
> --- a/client/virt/libvirt_vm.py
> +++ b/client/virt/libvirt_vm.py
> @@ -38,7 +38,7 @@ def virsh_cmd(cmd, uri = ""):
>       if uri:
>           uri_arg = "-c " + uri
>
> -    cmd_result = utils.run("%s %s %s" % (VIRSH_EXEC, uri_arg, cmd), ignore_status=True,
> +    cmd_result = utils.run("%s %s %s" % (VIRSH_EXEC, uri_arg, cmd), ignore_status=False,
>                              verbose=DEBUG)
>       if DEBUG:
>           if cmd_result.stdout.strip():