cam: fix xpt_bus_register and xpt_bus_deregister return errno

[imp]

Summary

xpt_bus_register and xpt_bus_deregister returns a hybrid error that's
neither a cam_status, nor an errno, but a mix of both. Update
xpt_bus_register and xpt_bus_deregister to return an errno. The vast
majority of current users compare against zero, which can also be
spelled CAM_SUCCESS. Nobody uses CAM_FAILURE, so remove that symbol
to prevent comfusion (nothing returns it either).

Where the return value is saved, ensure that the variable 'error' is
used to store an errno and 'status' is used to store a cam_status where
it makes the code clearer (usually just in functions that already mix
and match). Where the return value isn't used at all, avoid storing it
at all.

[mav]

First, I see this change as extremely invasive with pretty small benefit.
Second, I also not sure what benefit of using cam_status here, where it is not about doing I/O, that most of statuses represent.
Third, in 99e7a4ad9e6fe53868cb15449667ad46814d692b Scott switched cam_periph_acquire() from cam_status to errno, so this change goes wrong direction.

I'd prefer switch to errno instead here, that IMO is more logical and would keep existing KPI/KBI due to CAM_SUCCESS == 0.

[imp]

I'd picked cam_status as that is what a couple of existing SIMs expected that, and that is what had been returned before when CAM_SUCCESS wasn't the return value. I do like errno even better. I'll get rid of CAM_FAILURE, though, since it was never used and was never right to use here.

Thanks

[imp]

start to apply mav's feedback

[imp]

This should address mav's concerns.

[imp]

don't need to really update ps3 cdrom driver as part of this.

[imp]

I like the scope of this change better. Thanks for the suggestion mav@. IT's also MFC-able.

[mav]

sys/cam/cam_xpt.c

936

Wouldn't it be better to return the error here?

sys/cam/cam_xpt_sim.h

44

The same should probably be done for xpt_bus_deregister() for symmetry. It is not so broken and it will be a change of KPI, but its return code seems really checked only by pvscsi and umass.

[imp]

actually update xpt_bud_deregister too... Nobody is checking the return value so
it doesn't matter from a compat point of view, but why return anything?

[mav]

In D30860#694619, @imp wrote:

Nobody is checking the return value

pvscsi and umass check it for CAM_REQ_CMP and should be updated.

[imp]

mav caught two places I missed where xpt_bus_deregister checks the return value.

[scottl]

Yeah, my feeling is still that C function return values should be errno's, and that cam_status should be reserved for use primarily in the status fields of CCBs and related structures. If a C function needs to return a cam_status, it should return an errno and pass the cam_status by reference.