Re: [Xen-devel] [PATCH 02/29] libxl: Comment cleanups

[Hongyang Yang <yanghy@xxxxxxxxxxxxxx>]

å 02/11/2015 04:09 AM, Ian Jackson åé:
> * Add two comments in libxl_remus_disk_drbd documenting buggy handling
>    of the hotplug script exit status.
>
> * Add a section heading for async exec in libxl_aoutils.c
>
> * Mention the right function name (libxl__ev_child_fork, not
>    libxl__ev_fork) in libxl_internal.h
>
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>
> CC: Wen Congyang <wency@xxxxxxxxxxxxxx>
> CC: Lai Jiangshan <laijs@xxxxxxxxxxxxxx>
For async exec and Remus part,
Acked-by: Yang Hongyang <yanghy@xxxxxxxxxxxxxx>

> ---
> v2: New patch in this version of the series.
> ---
>   tools/libxl/libxl_aoutils.c         |    2 ++
>   tools/libxl/libxl_internal.h        |    2 +-
>   tools/libxl/libxl_remus_disk_drbd.c |    2 ++
>   3 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxl/libxl_aoutils.c b/tools/libxl/libxl_aoutils.c
> index b10d2e1..44dc222 100644
> --- a/tools/libxl/libxl_aoutils.c
> +++ b/tools/libxl/libxl_aoutils.c
> @@ -451,6 +451,8 @@ int libxl__openptys(libxl__openpty_state *op,
>       return rc;
>   }
>
> +/*----- async exec -----*/
> +
>   static void async_exec_timeout(libxl__egc *egc,
>                                  libxl__ev_time *ev,
>                                  const struct timeval *requested_abs)
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index 934465a..99db92a 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -1393,7 +1393,7 @@ _hidden int libxl__spawn_record_pid(libxl__gc*, 
> libxl__spawn_state*,
>    * This is a NOT function for waiting for ordinary child processes.
>    * If you want to run (fork/exec/wait) subprocesses from libxl:
>    *  - Make your libxl entrypoint use the ao machinery
> - *  - Use libxl__ev_fork, and use the callback programming style
> + *  - Use libxl__ev_child_fork, and use the callback programming style
>    *
>    * This function is intended for interprocess communication with a
>    * service process.  If the service process does not respond quickly,
> diff --git a/tools/libxl/libxl_remus_disk_drbd.c 
> b/tools/libxl/libxl_remus_disk_drbd.c
> index 3215f93..afe9b61 100644
> --- a/tools/libxl/libxl_remus_disk_drbd.c
> +++ b/tools/libxl/libxl_remus_disk_drbd.c
> @@ -145,6 +145,8 @@ static void match_async_exec_cb(libxl__egc *egc,
>
>       if (status) {
>           rc = ERROR_REMUS_DEVOPS_DOES_NOT_MATCH;
> +        /* BUG: seems to assume that any exit status means `no match' */
> +        /* BUG: exit status will have been logged as an error */
Yes, the error handling needs to be improved. I will prepare a patch for this.
The comments is fine for now.

>           goto out;
>       }
--
Thanks,
Yang.

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