[Libguestfs] [PATCH] Remove explicit guestfs=10.0.2.4:6666 kernel command line parameter.

[Jim Meyering]

Richard W.M. Jones wrote:
> This patch is in preparation for allowing libguestfs to use alternate
> "vmchannel" implementations.
>
> Although it's not a functional change, I think it is worthwhile on its
> own because use of the term "vmchannel" in the names of constants is
> inappropriate since (a) the "-net channel" option is now properly
> known upstream as "guestfwd", and (b) no one can agree on what
> "vmchannel" really means.
...
>>From 45630d4ac47511c5c0dc55a22fadba4b9236bf61 Mon Sep 17 00:00:00 2001
> From: Richard Jones <rjones at trick.home.annexia.org>
> Date: Tue, 15 Sep 2009 17:14:44 +0100
> Subject: [PATCH 4/5] Remove explicit guestfs=10.0.2.4:6666 kernel command line parameter.
>
> Since we control the appliance tightly, we can just specify
> that it will always use a particular host and port, and we
> don't need to pass it on the command line each time.
>
> Also the VMCHANNEL_* constants are only relevant to the
> particular guestfwd vmchannel implementation, so we rename
> them as GUESTFWD_*.
> ---
>  daemon/guestfsd.c |   78 ++++++++++++++--------------------------------------
>  src/guestfs.c     |   10 +++----
>  2 files changed, 25 insertions(+), 63 deletions(-)
>
> diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
> index af554bf..ebaf960 100644
> --- a/daemon/guestfsd.c
> +++ b/daemon/guestfsd.c
> @@ -43,8 +43,8 @@
>  static void usage (void);
>
>  /* Also in guestfs.c */
> -#define VMCHANNEL_PORT "6666"
> -#define VMCHANNEL_ADDR "10.0.2.4"
> +#define GUESTFWD_PORT "6666"
> +#define GUESTFWD_ADDR "10.0.2.4"

All looks good, though this seems like it *is* a functional
change, since it removes the -h and -p options below.

Two questions:

First, note the definition of GUESTFWD_ADDR above
Why the commented-out one below (in src/guestfs.c):

  //#define GUESTFWD_ADDR "10.0.2.4"


Second, I saw this in the context:

>    /* Make sure SIGPIPE doesn't kill us. */
>    memset (&sa, 0, sizeof sa);
>    sa.sa_handler = SIG_IGN;

Have you considered alternate ways of ignoring SIGPIPE?
Ignoringn SIGPIPE across the board can cause problems (albeit subtle)
in unsuspecting child processes.