On 05/20/2010 06:10 PM, Eric Blake wrote:
> 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.
>
The error squashing I was refering to here was of
BackendUpdateVolTargetInfoFD.
>> @@ -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);
>
Ah, I see, I didn't notice the squashing going on in MpathNewVol.
> 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?
>
A failing function's error messages should not be overwritten, so
MpathNewVol is wrong here. Thanks for pointing that out, I'll update and
resend.
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list