From mboxrd@z Thu Jan  1 00:00:00 1970
From: Lucas Meneghel Rodrigues <lmr@redhat.com>
Subject: Re: [PATCH] libvirt_vm.py: Set ignore_status to False in
	virsh_cmd().
Date: Thu, 12 Jan 2012 13:53:39 -0200
Message-ID: <4F0F0203.5010600@redhat.com>
References: <4F0D3508.3020606@cn.fujitsu.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Cc: autotest@test.kernel.org, kvm@vger.kernel.org
To: tangchen <tangchen@cn.fujitsu.com>
Return-path: <autotest-bounces@test.kernel.org>
In-Reply-To: <4F0D3508.3020606@cn.fujitsu.com>
List-Unsubscribe: <http://test.kernel.org/cgi-bin/mailman/listinfo/autotest>,
	<mailto:autotest-request@test.kernel.org?subject=unsubscribe>
List-Archive: <http://test.kernel.org/pipermail/autotest>
List-Post: <mailto:autotest@test.kernel.org>
List-Help: <mailto:autotest-request@test.kernel.org?subject=help>
List-Subscribe: <http://test.kernel.org/cgi-bin/mailman/listinfo/autotest>,
	<mailto:autotest-request@test.kernel.org?subject=subscribe>
Sender: autotest-bounces@test.kernel.org
Errors-To: autotest-bounces@test.kernel.org
List-Id: kvm.vger.kernel.org

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():