Bug 850470
| Summary: | sanlock masks useful error codes | ||
|---|---|---|---|
| Product: | Red Hat Enterprise Linux 6 | Reporter: | Jiri Denemark <jdenemar> |
| Component: | sanlock | Assignee: | David Teigland <teigland> |
| Status: | CLOSED NOTABUG | QA Contact: | yeylon <yeylon> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | 6.3 | CC: | ajia, cluster-maint, cwei, dyuan, lsu, mzhan, rbalakri, srevivo, ydu |
| Target Milestone: | rc | ||
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | 820173 | Environment: | |
| Last Closed: | 2015-09-30 14:06:48 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
|
Description
Jiri Denemark
2012-08-21 15:55:09 UTC
Let me get this straight, you want sanlock_align() to return EACCES in this case instead of ENODEV? Will it help libvirt in any way? I'm intentionally returning ENODEV for any problems related to the device, so I'll change this only if it will have a practical effect for the caller. Not really, the best would be if it just returned the real error. What if it fails because libvirt passes incorrect file name and the real error would be EISDIR, ENOENT, ENOSPC or anything else? Masking them all behind ENODEV makes it harder to spot the real problem. kicking this down the road I'd think that the sanlock log message is just what we'd want for troubleshooting:
> sanlock[27830]: 175679 open error -13 /var/lib/libvirt/sanlock/__LIBVIRT__DISKS__
That's what people actually see; they don't see the value returned in the api. The only reason to change the value in the api would be if libvirt uses it, but it doesn't. (In fact, this auto lease feature is not even used/enabled/supported in the first place.)
The reason this is not trivial to change is that changing the error value at the lowest open_disk level means we'd have to audit and probably adjust a lot of code that this return value propagates through, some of which depends on the current value returned.
People actually see the value returned in the API because libvirt uses the API and if it fails, the returned value is taken, translated into a string and reported as a libvirt error. Writing the exact error into a log file make sense for the sanlock daemon but sanlock client library that is supposed to be called by other apps should be able to report back why its API failed so that the calling app can present that to end users. The fix might not be trivial but the request for it is still valid. This cannot effect supportability because we simply don't use or support the code in question: - we do not support auto disk leases at all (which is what this bz is using) - in 6.4 leases in libvirt are not used at all (auto or otherwise) - eventually when vdsm starts passing leases through libvirt, even then libvirt will not be used to create the lockspaces (which is the api in question above) So, while the request is valid, and I intend to address it at some point, there is no time in the foreseeable future when this code will be used by a customer, or effect support in any way. This request was not resolved in time for the current release. Red Hat invites you to ask your support representative to propose this request, if still desired, for consideration in the next release of Red Hat Enterprise Linux. This is not a bug. If there's a specific issue that's a problem we will address that. |