On Wed, 2010-09-22 at 13:18 +0200, Boaz Harrosh wrote: > On 09/22/2010 09:16 AM, Nicholas A. Bellinger wrote: > > On Mon, 2010-09-20 at 15:06 -0700, Joel Becker wrote: > >> [Sorry on the delay, I was out of town] > >> > > > > Hi Joel, > > > > Many, thanks for your followup on this item, my comments are below. > > > >> On Fri, Sep 10, 2010 at 12:52:03PM -0700, Nicholas A. Bellinger wrote: > >>> On Fri, 2010-09-10 at 12:44 -0700, Joel Becker wrote: > >>>> You can refcount without check_link(). > >>> > >>> So what do you recommend here..? > >> > >> That your ACL object, or whatever it is that considers itself to > >> be refcounted by the number of links, keep track of that and only free > >> itself when all are gone rather than freeing itself when the first goes > >> away. > >> > > > > Ok, I see what you mean by internal refcounting within the configfs > > consumer to handle this case.. > > > >>> The problem is that the 'unlink sub_child/group1/src_0/src_link' can't > >>> signal to the other struct config_group to also call an internal 'unlink > >>> sub_child/group2/dst_0/dst_link' to drop the child link outside of it's > >>> struct config_group. > >> > >> Nor should it. I'm asking what is so wrong about a world where > >> sub_child/group1/src_0/src_link is gone but > >> sub_child/group2/dst_0/dst_link remains? Maybe that target object can't > >> work anymore, but the user broke it by breaking the link. > >> > > > > Yes, so trying to avoid the unlink alltogether here was my main > > intention thus far. > > > > Actually leaving the sub_child/group2/dst_0/dst_link in the example here > > would be acceptable for the TCM MappedLUN case, because really we never > > expect this case to this unless someone is poking at configfs directly, > > and our userspace code will never do this intentionally. > > > >>>> You're still fighting allowing the links to go away. You > >>>> haven't explained why that is necessary. You had a problem with a crash > >>>> because you expected one reference to your ACLs and actually have two, > >>>> but you can fix that without modifying configfs. > >>> > >>> If this is the case then I must be mis-understanding what you mean by > >>> configfs consumer refcounting from allow_link() and drop_link(). Can > >>> you give me a bit more detail where I should be looking..? > >> > >> Here's how I sort of understood things. First, you create the > >> src_link pointing to $object. This somehow allocates some sort of ACL > >> structure hanging off of $object. Then you create dst_link pointing to > >> src_link, which really ends up pointing to the $object. So now you have > >> src_link and dst_link pointing to $object. > >> Finally, someone unlinks src_link. This triggers $object to > >> free the ACL structure. When the caller later removes dst_link, it > >> crashes because it was expecting ACL to still be there. Do I have it > >> right? > > > > Correct. > > > >> I'm saying that $object should count how many people are > >> pointing to it, so that when you remove src_link, ACL is *not* freed. > >> It will only be freed when both src_link and dst_link are removed. This > >> way you do not crash. Perhaps I'm not understanding the ACL object. > >> Perhaps I'm missing the mechanism entirely. But I don't see why the ACL > >> object must necessarily be freed when one symlink is removed but not the > >> other. > >> > > > > No, I think your points here make perfect sense. I will look into a > > patch that leaves the TCM fabric MappedLUNs symlinks in place when the > > underlying TPG fabric LUN symlink is removed without breaking anything, > > but still does the necessary accounting to ensure that shutdown with > > active I/Os still works as expected. > > Perhaps a strengthen chmod here. And if then, done by root, a big fat > "shoot self in the foot" message in dmsg for the poking where you don't > need to. type. > Hmm, I don't believe configfs currently supports a distinction between permissions for something along these lines. > (BTW: could you re establish the link after it's deleted the way you > do at setup?) > Yes, the "dangling" MappedLUNs symlinks for this special case would need to be explictly removed via unlink(2) and then re-created using a new TPG LUN struct se_lun->lun_group containing a valid symlink back to a TCM Core backstore a valid configfs symlink source. Thanks! --nab -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
