Re: [PATCH 3/4] storage: FS backend adjust error message on error path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 06/03/2015 01:44 PM, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1181087
> 
> Currently the assumption on the error message is that there are
> no source device path's defined when the != 1 check fails, but in
s/path's/paths

"...when the != 1 check fails" I can't help it, I know what you're
saying but somehow I keep thinking about it in C logic --> when a check
fails, it means that the structured block following the check will be
skipped...anyway, it's nothing, maybe I'm just thinking about it too
much, this one's your call.
> reality the value could 0 or 2 or more, so adjust the error message
> accordingly to make it clearer what the error really is.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/storage/storage_backend_fs.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
> index 9a1343d..3c39646 100644
> --- a/src/storage/storage_backend_fs.c
> +++ b/src/storage/storage_backend_fs.c
> @@ -362,8 +362,13 @@ virStorageBackendFileSystemValidateFS(virStoragePoolObjPtr pool)
>          }
>      } else {
>          if (pool->def->source.ndevice != 1) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           "%s", _("missing source device"));
> +            if (pool->def->source.ndevice == 0)
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("missing source device"));
> +            else
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
I'd say this should be CONFIG_UNSUPPORTED rather than INTERNAL_ERROR
> +                               _("Expected exactly 1 device for the "
                                     ^ lowercase again
> +                                 "storage pool"));
>              return -1;
>          }
>      }
> 

ACK with those minor adjustments (as I
said, the one in the commit message is up to you).

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]