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@xxxxxxxxxx>
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list