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

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

 



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:

        virStorageReportError(VIR_ERR_INTERNAL_ERROR,
                              _("Failed to update volume for '%s'"),
                              vol->target.path);

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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
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]