On 02/06/2012 02:06 PM, Chris Evich wrote: > Most calls to virsh_cmd() were not able to detect any errors > or examine the command's stdout content. This patch changes > the behavior or virsh_cmd() to raise a CmdError exception if > the command resulted in a non-zero exit. The CmdResult > instance is passed through the exception for later > examination or printing where useful. > > Signed-off-by: Chris Evich<[email protected]> > --- > client/virt/libvirt_vm.py | 26 ++++++++++++++++---------- > 1 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py > index d53e43a..3c4a7f1 100644 > --- a/client/virt/libvirt_vm.py > +++ b/client/virt/libvirt_vm.py > @@ -31,22 +31,28 @@ def libvirtd_restart(): > > > def virsh_cmd(cmd, uri = ""): > + """ > + Append cmd to 'virsh' and execute, optionally return full results. > + > + @param: cmd: Command line to append to virsh command > + @param: uri: hypervisor URI to connect to > + @return: stdout of command > + """ > if VIRSH_EXEC is None: > raise ValueError('Missing command: virsh') > > uri_arg = "" > if uri: > uri_arg = "-c " + uri > - > - cmd_result = utils.run("%s %s %s" % (VIRSH_EXEC, uri_arg, cmd), > ignore_status=True, > - verbose=DEBUG) > - if DEBUG: > - if cmd_result.stdout.strip(): > - logging.debug("stdout: %s", cmd_result.stdout.strip()) > - if cmd_result.stderr.strip(): > - logging.debug("stderr: %s", cmd_result.stderr.strip()) > - > - return cmd_result.stdout.strip() > + cmd = "%s %s %s" % (VIRSH_EXEC, uri_arg, cmd) > + cmd_result = utils.run(cmd, ignore_status=True, verbose=DEBUG) > + out = cmd_result.stdout.strip() > + err = cmd_result.stderr.strip() > + ext = int(cmd_result.exit_status) > + if ext != 0: > + raise error.CmdError, (cmd, cmd_result)
^ Some comments here: * Since we're running the run cmd with the ignore_status=True flag, the above would be better expressed like this: cmd_result = utils.run(cmd, verbose=DEBUG) return cmd_result.stdout.strip() Shorter, more elegant. If one wants to debug what happened, the CmdError exception is formatted to show all the streams. Since on the following patch you are correctly handling the possible CmdError exceptions, then we can go with the above format without worrying too much, as the callers might handle the exception and show whatever info they want to. In case you prefer having more control of the info, we can change virsh_cmd to return the CmdResult objects instead and adjust all the code to take that into account. > + else: > + return out > > > def virsh_uri(uri = ""): _______________________________________________ Autotest mailing list [email protected] http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
