On Thursday, March 8, 2012 at 9:30 AM, Noah Watkins wrote: > > On Mar 7, 2012, at 1:40 PM, Greg Farnum wrote: > > > CephMount::conf_get requires a buf_size. But we freely adjust that if we need more space, so is there any point to it? I can't see callers really having a better idea of size than we do. > Good point. I can't think of a reason to publicly expose the buffer length param in CephMount to users, but CephProxy is a private interface. On a related a related note: > > int ceph_conf_get(struct ceph_mount_info *cmount, const char *option, char *buf, size_t len); > > shouldn't the 'buf' parameter be char ** if libcephfs is going to be doing a re-alloc, or is there some voodoo behind the scenes? libcephfs doesn't do a re-alloc there; I was talking about the Java stuff when I said it freely adjusts. :) > > (Oh, now I see there's another one that doesn't take buf_size. Do we have a use case for the with-size option, though?) > > Similarly with getdnames > > Yes, the default buf_size interfaces are meant to be friendly, while the explicit buf_size interfaces are primarily meant for the unit tests so that the buffer expansion logic can be tested. It would be safe to leave it public, but maybe it is better for it to be protected and used explicitly by the unit tests. Ah, I see. Well, leaving the explicit buf_size versions public expands the public interface, so I vote to make them protected (or whatever's appropriate in Java; I forget how the privacy settings map). > > We don't implement CephMount::mkdir, just mkdirs? I *can* see use cases for not wanting recursive creates… > > I was going for the minimum to support Hadoop. I'll stick mkdirs on the short list of API expansion TODOs. Okay, makes sense. > > I like the states and require_state() stuff. It's making me notice that you need a little more documentation on stuff like closedir and what happens to the CephDirectory under success and error returns. :) > > The state checking is really helpful to catch set faults when blindly casting Java long to void* :) I wonder if there is a way to push the state checking even further down in libcephfs? Currently all errors are communicated through a generic CephException, but making things more specific as we go along will be nice. I'm not sure how we'd push it farther down in a way that's any cleaner, did you have some thoughts or just a wish to not do it in Java? Oh, and I forgot something important last time: there's no licensing associated with any of these files right now. :) I think we talked previously about making it Apache, but whatever it is should be clearly noted! -Greg -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
