On Sat, Jul 17, 2010 at 06:07:30PM +0200, Matthias Bolte wrote:
2010/7/8 Chris Lalancette <clalance(a)redhat.com>:
> With the change to make vshError() responsible for printing
> all errors, there were some places in the code that would no
> longer properly print errors. The good news is that the vast
> majority of virsh was already printing errors, so this patch
> just cleans up the rest of the users to make them consistent.
>
> Signed-off-by: Chris Lalancette <clalance(a)redhat.com>
> ---
> tools/virsh.c | 228 +++++++++++++++++++++++++++++++++++++++++++--------------
> 1 files changed, 174 insertions(+), 54 deletions(-)
>
The patch as is look good, but I wonder how you spot all those places
that missed a vshError call. Did you manually go over the whole
virsh.c file and checked all libvirt API calls?
I just spot calls to virDomainMigrate and virDomainMigrateToURI
(around line 3040) that are not matched with a call to vshError, you
might want to fold that into your patch.
I sounds like there should be a final catch all case. In 'main' check
the return value of 'vshCommandRun' and if it is FALSE and vshError
hasn't been invoked then report the error.
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://deltacloud.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|