Re: [Xen-devel] [PATCH 20/20] libxl: ao: Convert libxl_run_bootloader

[Roger Pau Monnà <roger.pau@xxxxxxxxxxxxx>]

2012/3/21 Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>:
> Ian Jackson writes ("Re: [Xen-devel] [PATCH 20/20] libxl: ao: Convert 
> libxl_run_bootloader"):
>> Ian Campbell writes ("Re: [Xen-devel] [PATCH 20/20] libxl: ao: Convert 
>> libxl_run_bootloader"):
>> > Could this stuff be usefully pushed into libxl__openpty?
>>
>> Hmm. ÂThis is a bit of a can of worms I think. ÂWhen I wrote my patch
>> I left it well alone. ÂTBH I think the current code is arguably wrong.
>> I can see no reason why we would try to do termios operations on the
>> pty master at all. ÂI think these functions should be performed on the
>> slave on all platforms.
>>
>> But I don't want to introduce a perhaps-not-wonderfully-portable
>> change like that in this series.
>
> Just to be clear: I left all of this pty termios code alone in my
> series; it just shuffles about a bit but is otherwise unchanged as far
> as possible. ÂThe only intentional difference is that I replaced
> forkpty with our own fork + openpty + login_tty.
>
>> > login_tty is a new one to me. When I lookup up the manpage I happened to
>> > notice:
>> > Â Â Â ÂLink with -lutil.
>>
>> Hmm. ÂThe BSD manpage is more forthcoming and suggests we need
>> something different there. Â(This isn't a particularly portable
>> function.)
>>
>> I think we need some more autoconfery.
>
> Like the below. ÂPreviously libxl was linking with -lutil by virtue of
> using UTIL_LIBS which came from config/StdGNU.mk.
>
> Ian.
>
> From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Subject: [PATCH] autoconf: New test for openpty et al.
>
> We may need to #include <libutil.h>, and/or link with -lutil, to use
> openpty, login_tty, and the like. ÂProvide INCLUDE_LIBUTIL_H
> (preprocessor constant, not always defined) and PTYFUNCS_LIBS
> (makefile variable).
>
> We link libxl against PTYFUNCS_LIBS (which comes from autoconf) rather
> than UTIL_LIBS, and #include <libutil.h> where appropriate.
>
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>
> ---
> Âconfig/Tools.mk.in       |  Â2 +
> Âtools/configure        Â|  57 
> ++++++++++++++++++++++++++++++++++++++++
> Âtools/configure.ac       |  Â2 +
> Âtools/libxl/Makefile      |  Â2 +-
> Âtools/libxl/libxl_bootloader.c | Â Â4 +++
> Âtools/m4/ptyfuncs.m4 Â Â Â Â Â | Â 24 +++++++++++++++++
> Â6 files changed, 90 insertions(+), 1 deletions(-)
[...]
> diff --git a/tools/m4/ptyfuncs.m4 b/tools/m4/ptyfuncs.m4
> new file mode 100644
> index 0000000..2846a6d
> --- /dev/null
> +++ b/tools/m4/ptyfuncs.m4
> @@ -0,0 +1,24 @@
> +AC_DEFUN([AX_CHECK_PTYFUNCS], [
> + Â ÂAC_CHECK_HEADER([libutil.h],[
> + Â Â ÂAC_DEFINE([INCLUDE_LIBUTIL_H],[<libutil.h>],[libutil header file name])
> + Â Â])
> + Â ÂAC_CACHE_CHECK([for openpty et al], [ax_cv_ptyfuncs_libs], [
> + Â Â Â Âax_cv_ptyfuncs_libs=-lutil
> + Â Â Â ÂAX_SAVEVAR_SAVE(LIBS)
> + Â Â Â ÂLIBS="$LIBS $ax_cv_ptyfuncs_libs"
> + Â Â Â ÂAC_LINK_IFELSE([
> +#ifdef INCLUDE_LIBUTIL_H
> +#include INCLUDE_LIBUTIL_H
> +#endif

Shouldn't you add something like this (or perhaps add the necessary
AC_CHECK_HEADER for this headers, so we can get rid of the OS
conditional include dependency):

#if defined(__NetBSD__) || defined(__OpenBSD__)
#include <util.h>
#elif defined(__linux__)
#include <pty.h>
#elif defined(__sun__)
#include <stropts.h>
#endif
#include <utmp.h>

> +int main(void) {
> + Âopenpty(0,0,0,0,0);
> + Âlogin_tty(0);
> +}
> +])
> + Â Â Â ÂAX_SAVEVAR_RESTORE(LIBS)],
> + Â Â[],[
> + Â Â Â ÂAC_MSG_FAILURE([Unable to find -lutil for openpty and login_tty])
> + Â Â])
> + Â ÂPTYFUNCS_LIBS="$ax_cv_ptyfuncs_libs"
> + Â ÂAC_SUBST(PTYFUNCS_LIBS)
> +])
> --
> tg: (154e097..) t/xen/tools/ptyfuncs (depends on: t/xen/xl.event.children)
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

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