[libvirt] [PATCH] storage_backend_fs.c: do not ignore probe failure

[Daniel P. Berrange]

On Mon, Jan 18, 2010 at 01:37:34PM +0100, Jim Meyering wrote:
> Daniel P. Berrange wrote:
> > On Mon, Jan 18, 2010 at 10:40:26AM +0100, Jim Meyering wrote:
> >> When virStorageBackendProbeTarget fails, it returns -1 or -2.
> >> The two uses below obviously intended to handle those cases
> >> differently, but due to a wrong comparison, they always treated a
> >> "real" (e.g., open) failure (-1) just like an ignorable "wrong file type"
> >> failure (-2).
> >>
> >> FYI, coverity reported that the "goto cleanup" statement was
> >> unreachable, which was true in each case, but hardly the real problem.
> ...
> >
> > I don't understand how this is making any difference. The method
> > virStorageBackendProbeTarget can return 0, -1 or -2, so
> >
> >     virStorageBackendProbeTarget < 0  catches the -1 and -2 cases
> >     virStorageBackendProbeTarget != 0 catches the -1 and -2 cases
> >
> > I only see this change making a difference if one of the error return values
> > is a positive integer greater than 0, but we don't have that here. What am
> > I missing ?
> 
> Good catch.
> I changed the wrong thing.  Real problem: misplaced ")".
> Here's the correct patch:
> 
> From 8834728541e38c6843c6941457596bba18ca4ae3 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Mon, 18 Jan 2010 10:34:53 +0100
> Subject: [PATCH] storage_backend_fs.c: do not ignore probe failure
> 
> * src/storage/storage_backend_fs.c (virStorageBackendFileSystemRefresh):
> Correct parentheses.  The documented intent is to ignore non-regular
> files, yet due to a parenthesization error all errors were handled
> that way.
> ---
>  src/storage/storage_backend_fs.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 4fe40b3..b03e4e9 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -1,7 +1,7 @@
>  /*
>   * storage_backend_fs.c: storage backend for FS and directory handling
>   *
> - * Copyright (C) 2007-2009 Red Hat, Inc.
> + * Copyright (C) 2007-2010 Red Hat, Inc.
>   * Copyright (C) 2007-2008 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -562,7 +562,7 @@ virStorageBackendFileSystemRefresh(virConnectPtr conn,
>                                                  &backingStore,
>                                                  &vol->allocation,
>                                                  &vol->capacity,
> -                                                &vol->target.encryption) < 0)) {
> +                                                &vol->target.encryption)) < 0) {
>              if (ret == -1)
>                  goto cleanup;
>              else {

ACK, makes sense now :-)

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|