ocp: fix unchecked return value of ocp_get_uuid_index

[sahmed-ibm]

The get_c0_log_page(), ocp_get_log_simple() and
ocp_fw_activation_history_log() functions all call ocp_get_uuid_index() but discard the return value. ocp_get_uuid_index() returns a non-zero value on failure, leaving uidx at its default of 0.

Proceeding with uidx set to 0 encodes the wrong UUID index into the CDW14 field, causing log page retrievals to use an invalid UUID index rather than failing cleanly.

Check the return value of ocp_get_uuid_index() in all three callers, log the error using libnvme_strerror(-ret) since ocp_get_uuid_index() returns the error code directly and does not set errno, and return the error to the caller.

Also fix ocp_find_uuid_index() which incorrectly returned -errno on failure instead of propagating the negative error code returned by libnvme_find_uuid(). libnvme_find_uuid() does not set errno; it returns a negative error code directly, so -errno produced a stale unrelated value.

[brandon-paupore-sndk]

Proceeding with uidx set to 0 encodes the wrong UUID index into the CDW14 field, causing log page retrievals to use an invalid UUID index rather than failing cleanly.

FWIW, the OCP spec since at least 2.0 has a requirement UUID-5 that using a UUID index of 0 for any of their defined log page (or feature for UUID-3) IDs will target their version. It's also not really an "invalid UUID index", per the NVMe spec it's "no UUID index", where the controller determines what vendor specific information to return (such as the OCP version if compliant with those OCP requirements).

[HaroPanosyan]

Proceeding with uidx set to 0 encodes the wrong UUID index into the CDW14 field, causing log page retrievals to use an invalid UUID index rather than failing cleanly.

FWIW, the OCP spec since at least 2.0 has a requirement UUID-5 that using a UUID index of 0 for any of their defined log page (or feature for UUID-3) IDs will target their version. It's also not really an "invalid UUID index", per the NVMe spec it's "no UUID index", where the controller determines what vendor specific information to return (such as the OCP version if compliant with those OCP requirements).

Trying to clarify my understanding and getting help here.

From both UUID-3 and UUID-5 I can deduce 0 index should be OCP. Is this correct?
For logs it should be OK, users can get the log and verify GUID in the log to confirm for OCP. However, for set feature this might be problematic or destructive, since setting a feature may alter the drive, unlike reading a log.
More importantly, this notion of adding index 0 to UUID-3/5 seems unnecessary, because UUID-1 and NVMe-AD-10. From those I can deduce OCP index can be cleanly extracted from the drive, hence no need for index 0. Is this correct?

Thanks in advance.

[brandon-paupore-sndk]

From both UUID-3 and UUID-5 I can deduce 0 index should be OCP. Is this correct?

Yeah, those requirements indicate that index 0 should be for OCP content with Get/Set Features and Get Log Page, in addition to any actual defined index for OCP content. The base NVMe behavior where no UUID is specified means only that some vendor specific information is returned, while these requirements further define what that vendor specific information should be.

For logs it should be OK, users can get the log and verify GUID in the log to confirm for OCP. However, for set feature this might be problematic or destructive, since setting a feature may alter the drive, unlike reading a log. More importantly, this notion of adding index 0 to UUID-3/5 seems unnecessary, because UUID-1 and NVMe-AD-10. From those I can deduce OCP index can be cleanly extracted from the drive, hence no need for index 0. Is this correct?

Yep, assuming that the UUID list retrieval works and that the OCP UUID is correctly populated in that list as required in UUID-1 then there should be a specific UUID index for OCP content. But if that fails then it would still be defined behavior to use a UUID index of 0 due to the other requirements.

Note also that any of these defined requirements can be reported through the OCP 0xC5 log page as being unsupported by the controller. It seems that an "OCP-compliant" drive could be designed to only support use of UUID index 0 for OCP content, assuming it doesn't support UUID-1 (or perhaps UUID indexes more broadly). In theory that log page could be checked to verify these requirements, but that'd also depend on the same UUID behavior so I'm not sure it'd help here.