cam_xpt: Properly fail if a sim uses an unsupported transport.

[jhb]

Summary

The default xport ops for a new bus is xport_default, not NULL, so
check for that when determining if a bus failed to find a suitable
transport. In addition, the path needs to be freed with xpt_free_path
instead of a plain free so that the path's reference on the sim is
dropped; otherwise, cam_sim_free in the caller after xpt_bus_register
returns failure will hang forever.

Note that we have to exempt the xpt bus from this check.

[imp]

I think this is good. I know the xpot_free_path is good (nobody ever hits this path, so it could well be wrong when somebody writes a new xport that does).
I think it needs a comment since I just puzzled over it a good long time to understand how all the dots are connected for sure (eg, how does xport_default get on the set list, so that we match it).
I worry that there's some weird case where transport could be unspecified for SCSI (since it sets it to that for the speed negotiation), but I think I convinced myself that all path set it to something that's never unspecified: worst case is that it's UNKNOWN since that's what the devices default to when they aren't set (though I think they always are). But I'm not going to let my worries here get in the way.

sys/cam/cam_xpt.c

4021–4028

This likely merits a comment: Who the heck would be XPORT_UNSPECIFIED (the xpt bus) and why do we skip this loop if we do. Why is it OK to exempt the xpt bus from setting new_bus->xport? (because the xpt bus exports the xpt_default as its bus, which is what we set it to a dozen lines above).... I only say the latter because I just spent 10 minutes showing it was 'obviously' the same thing though several layers of indirection.

[imp]

I think this is good, but I'd be happier it ken or mav ticked the box too.

[mav]

xpt_free_path() definitely looks right, and the NULL check was obviously wrong.

I am not sure about original XPORT_UNSPECIFIED motivation, but may be, just a guess, it could have sense in cases like struct ccb_trans_settings to express that xport_specific field is not applicable. Though even there the edge between that and XPORT_UNKNOWN seems pretty small.

[jhb]

Also, FWIW, I ran into this when I first added my NVMe over Fabrics sim which uses a new XPORT_NVMF and promptly hit this case. Initially it panicked as it didn't match the NULL check so attempting to scan resulted in a panic from xpt_default_action due to an unhandled CCB. Once I fixed the NULL check I then got the forever hang described in the commit log due to free vs xpt_free_path. I did end up testing that my new SIM failed to register its bus and failed correctly before I added the new xport in nvme_xport.c to get my SIM working, so I did test the case of a SIM with an unhandled XPORT.

sys/cam/cam_xpt.c

4021–4028

Yeah, a comment would be good. I described it in the commit log, but not here.