On 07/03/2017 12:32 AM, Nicholas A. Bellinger wrote: > On Fri, 2017-06-23 at 01:18 -0500, Mike Christie wrote: >> The problem with the code currently is that tcmu will send a request to >> userspace but not wait for a reply, so it would always return success to >> lio. >> >> This is a problem for dev addition, because userspace could fail, but >> the kernel would have a se_device that could get mapped to a lun and >> reported to initiators. The requesting app would also get a success status >> and it would continue setup. >> >> For removal this causes a problem where userspace could have done a close >> after tcmu does the uio unregister device call. When the uio code is fixed >> to not use the parent, then this will cause a crash in uio_release. >> >> To fix these issues, this patchset made over Nicks for-next branch, >> makes the tcmu netlink operations synchronous. >> >> Patch 1: Fixes for reconfigure added recently. These will be needed in patch5. >> Patch 2 - 4: Add target layer support to be able to look up a se_device by id. >> Patch 5: Add sync netlink support to tcmu. >> Patch 6 - 8: Convert the rest of the target code to use the idr based >> id/lookup/iter code. >> Patch 9 - 10: tcmu cleanup and fixes based on previous patches. >> >> Changes in V4 >> - Dropped target_free_device cleanup patches, because I am looking into moving >> the tcmu refcount code to uio. Realized I am a dummy and cannot remove >> it, because users using the old tcmu-runner could hit crashes. >> >> Changes in V3 >> - Use DEFINE_IDR initializer. >> - Added patch 12 to rename target_free_device. >> - Use correct string style in xcopy code. >> >> > > Applied as-is with the extra -v2 for patch #1. Thanks for fixing that > up. ;) > > One concern though, patch #1 changes the configfs attribute name from > 'path' to 'config', which will break existing userspace. > > So this isn't really an option, even though I agree the original name > isn't particularly a good fit for what it actually does. > > That said, would you mind adding an incremental patch that re-adds the > original 'path' attribute with existing behavior, and keeps 'config' > with the new behavior..? > I can, but the patch to add the tcmu_attr_dev_path was never in a released kernel. It was just recently added in Bryant's patch: commit 0637d7f9698416bf745764edd4f6a20179c254ae Author: Bryant G. Ly <bryantly@xxxxxxxxxxxxxxxxxx> Date: Tue Jun 6 09:28:51 2017 -0500 tcmu: Make dev_config configurable that was merged the other week and only in your for-next branch. We do not need to keep userspace compatibility with that branch do we? -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
