Re: [Xen-devel] [PATCH 2/2] libxl: implement libxl__xs_mknod using XS_WRITE rather than XS_MKDIR

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

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.

I think it's probably correct to add the logging but it would perhaps
be worth checking the call sites.


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).


> +  bool ok;   /* the return value from a boolean function */

Maybe say `the success return value from a boolean function' ?

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel