[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 2/2] libxl: implement libxl__xs_mknod using XS_WRITE rather than XS_MKDIR
> -----Original Message----- > From: Ian Jackson [mailto:Ian.Jackson@xxxxxxxxxxxxx] > Sent: 25 November 2015 14:04 > To: Paul Durrant > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini; Ian Campbell; Wei Liu > Subject: Re: [PATCH 2/2] libxl: implement libxl__xs_mknod using XS_WRITE > rather than XS_MKDIR > > Paul Durrant writes ("[PATCH 2/2] libxl: implement libxl__xs_mknod using > XS_WRITE rather than XS_MKDIR"): > > This patch modifies the implentation of libxl__xs_mknod() to use > XS_WRITE > > rather than XS_MKDIR since passing an empty value to the former will > > ensure that the path is both existent and empty upon return, rather than > > merely existent. The function return type is also changed to a libxl > > error value rather than a boolean, it's declaration is accordingly moved > > into the 'checked' section in libxl_internal.h, and a comment is added to > > clarify its semantics. > ... > > +/* On success, path will exist and will be empty */ > > +int libxl__xs_mknod(libxl__gc *gc, xs_transaction_t t, > > + const char *path, struct xs_permissions *perms, > > + unsigned int num_perms); > > I like the idea of making this function a `checked' one, but: > > /*----- "checked" xenstore access functions -----*/ > /* Each of these functions will check that it succeeded; if it > * fails it logs and returns ERROR_FAIL. > */ > > but your implementation does not log anything. Damn. I meant to add that too. > > I think it's probably correct to add the logging but it would perhaps > be worth checking the call sites. > Sure. > > While I am here, you maybe want to adjust the wording of a couple of > comments: > > > +/* On success, path will exist and will be empty */ > > You mean `will have an empty value'. WRITE does not delete any > subtrees (and libxl__xs_mknod shouldn't IMO). > Yes, I'll re-word. > > > + bool ok; /* the return value from a boolean function */ > > Maybe say `the success return value from a boolean function' ? > Ok. Paul > Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |