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

Re: [libvirt] [PATCH 1/3] storage: mpath: Clean up some error handling

On 05/20/2010 01:23 PM, Cole Robinson wrote:
> We were squashing error messages in a few cases. Recode to follow common
> ret = -1 convention.

s/squashing/duplicating/, but I did check that the only caller,
virStorageBackendMpathNewVol does print a message when it gets a
negative result.

> @@ -43,39 +43,27 @@ virStorageBackendMpathUpdateVolTargetInfo(virStorageVolTargetPtr target,
>                                            unsigned long long *allocation,
>                                            unsigned long long *capacity)
>  {
> -    int ret = 0;
> +    int ret = -1;
>      int fd = -1;
>      if ((fd = open(target->path, O_RDONLY)) < 0) {
>          virReportSystemError(errno,
>                               _("cannot open volume '%s'"),
>                               target->path);
> -        ret = -1;
>          goto out;

Unfortunately, this means that we lose errno; if open fails, we get just
the less-specific message:

                              _("Failed to update volume for '%s'"),

Would it make more sense to refactor this in the other direction, to
make virStorageBackendMpathUpdateVolTargetInfo always print the error
message on failure, where we still have errno, and make
virStorageBackendMpathNewVol silent instead of duplicating the message?

Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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