Cathy Zhou wrote: > Sebastien Roy wrote: >> Cathy Zhou wrote: >>> I am wondering, we should always return ENOENT in >>> dls_devnet_hold_by_name() function [1]. >> ... >> > [1] Probably the only exception is the dls_mgmt_xxx() upcalll. If the >> > daemon fails for some reason, maybe EBADF should be returned. >> >> Hmm, I'm not sure if we want to mask the actual errors from >> dls_mgmt_get_linkid() by always returning either ENOENT. For example, >> if the upcall failed, the error code should have appropriately been >> returned by i_dls_mgmt_upcall(), and we shouldn't have to translate >> between one error code and another. If it's ENOMEM, then we'll return >> that, etc... >> >> EBADF usually means that the caller supplied an invalid >> file-descriptor, so it might be strange to have i_dls_mgmt_upcall() >> return that error code. Perhaps we need to debug the kinds of errors >> that i_dls_mgmt_upcall() returns separately. >> > I don't quite understand what you mean here. Can you please elaborate?
Take EBADF is one example. Having i_dls_mgmt_upcall() return EBADF means that callers will percolate this error code up to the originating operation, where EBADF makes no sense. ifconfig printing "Bad file number", for example, would make no sense to the administrator, as the file-descriptor in question is an implementation detail between dlmgmtd, libdladm, and the i_dls_mgmt_upcall() function. >>> I am not quite understand when we would reach line 1080, but I can >>> certainly test it out. >> >> I honestly don't know, which is why I left it alone. I'll change it >> to ENOENT. >> > I looked at it more, it could happen if there is another thread in > parallel which create a different link whose name is "link_under". I don't see a problem with returning ENOENT in that case, unless you have a better suggestion. > >>> The dls_devnet_set() function on line1105 could return EEXIST, if >>> this is a implicit VLAN name, but the same VLAN already exists using >>> another name, but in that case, we should also return ENOENT. >> > In this case. "ifconfig bge1000 plumb" just print out: > > "ifconfig: cannot open link "bge1000": File exists" > > I feel it is a little strange, as it seems implied that bge1000 has been > plumbed. I had changed this to ENOENT also. -Seb
