Richard W.M. Jones
2009-Sep-17 14:02 UTC
[Libguestfs] [PATCH] Remove explicit guestfs=10.0.2.4:6666 kernel command line parameter.
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. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 75 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -------------- next part -------------->From 45630d4ac47511c5c0dc55a22fadba4b9236bf61 Mon Sep 17 00:00:00 2001From: 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" int verbose = 0; @@ -67,18 +67,14 @@ int sysroot_len = 8; int main (int argc, char *argv[]) { - static const char *options = "fh:p:?"; + static const char *options = "f?"; static const struct option long_options[] = { { "foreground", 0, 0, 'f' }, { "help", 0, 0, '?' }, - { "host", 1, 0, 'h' }, - { "port", 1, 0, 'p' }, { 0, 0, 0, 0 } }; int c, n, r; int dont_fork = 0; - const char *host = NULL; - const char *port = NULL; FILE *fp; char buf[4096]; char *p, *p2; @@ -111,14 +107,6 @@ main (int argc, char *argv[]) dont_fork = 1; break; - case 'h': - host = optarg; - break; - - case 'p': - port = optarg; - break; - case '?': usage (); exit (0); @@ -134,47 +122,21 @@ main (int argc, char *argv[]) exit (1); } - /* If host and port aren't set yet, try /proc/cmdline. */ - if (!host || !port) { - fp = fopen ("/proc/cmdline", "r"); - if (fp == NULL) { - perror ("/proc/cmdline"); - goto next; - } - n = fread (buf, 1, sizeof buf - 1, fp); - fclose (fp); - buf[n] = '\0'; - - /* Set the verbose flag. Not quite right because this will only - * set the flag if host and port aren't set on the command line. - * Don't worry about this for now. (XXX) - */ - verbose = strstr (buf, "guestfs_verbose=1") != NULL; - if (verbose) - printf ("verbose daemon enabled\n"); - - p = strstr (buf, "guestfs="); - - if (p) { - p += 8; - p2 = strchr (p, ':'); - if (p2) { - *p2++ = '\0'; - host = p; - r = strcspn (p2, " \n"); - p2[r] = '\0'; - port = p2; - } - } + /* Set the verbose flag. */ + fp = fopen ("/proc/cmdline", "r"); + if (fp == NULL) { + perror ("/proc/cmdline"); + goto next; } + n = fread (buf, 1, sizeof buf - 1, fp); + fclose (fp); + buf[n] = '\0'; - next: - /* Can't parse /proc/cmdline, so use built-in defaults. */ - if (!host || !port) { - host = VMCHANNEL_ADDR; - port = VMCHANNEL_PORT; - } + verbose = strstr (buf, "guestfs_verbose=1") != NULL; + if (verbose) + printf ("verbose daemon enabled\n"); + next: /* Make sure SIGPIPE doesn't kill us. */ memset (&sa, 0, sizeof sa); sa.sa_handler = SIG_IGN; @@ -197,9 +159,10 @@ main (int argc, char *argv[]) memset (&hints, 0, sizeof hints); hints.ai_socktype = SOCK_STREAM; hints.ai_flags = AI_ADDRCONFIG; - r = getaddrinfo (host, port, &hints, &res); + r = getaddrinfo (GUESTFWD_ADDR, GUESTFWD_PORT, &hints, &res); if (r != 0) { - fprintf (stderr, "%s:%s: %s\n", host, port, gai_strerror (r)); + fprintf (stderr, "%s:%s: %s\n", + GUESTFWD_ADDR, GUESTFWD_PORT, gai_strerror (r)); exit (1); } @@ -219,7 +182,8 @@ main (int argc, char *argv[]) freeaddrinfo (res); if (sock == -1) { - fprintf (stderr, "connection to %s:%s failed\n", host, port); + fprintf (stderr, "connection to %s:%s failed\n", + GUESTFWD_ADDR, GUESTFWD_PORT); exit (1); } @@ -317,7 +281,7 @@ xread (int sock, void *v_buf, size_t len) static void usage (void) { - fprintf (stderr, "guestfsd [-f] [-h host -p port]\n"); + fprintf (stderr, "guestfsd [-f]\n"); } int diff --git a/src/guestfs.c b/src/guestfs.c index e9961d1..0c97cea 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -85,8 +85,8 @@ static void close_handles (void); #define UNIX_PATH_MAX 108 /* Also in guestfsd.c */ -#define VMCHANNEL_PORT 6666 -#define VMCHANNEL_ADDR "10.0.2.4" +#define GUESTFWD_PORT 6666 +//#define GUESTFWD_ADDR "10.0.2.4" /* GuestFS handle and connection. */ enum state { CONFIG, LAUNCHING, READY, BUSY, NO_HANDLE }; @@ -1003,11 +1003,9 @@ guestfs__launch (guestfs_h *g) /* Linux kernel command line. */ snprintf (append, sizeof append, LINUX_CMDLINE - "guestfs=%s:%d " "%s" /* (selinux) */ "%s" /* (verbose) */ "%s", /* (append) */ - VMCHANNEL_ADDR, VMCHANNEL_PORT, g->selinux ? "selinux=1 enforcing=0 " : "selinux=0 ", g->verbose ? "guestfs_verbose=1 " : " ", g->append ? g->append : ""); @@ -1038,7 +1036,7 @@ guestfs__launch (guestfs_h *g) */ snprintf (vmchannel, sizeof vmchannel, "user,vlan=0,net=10.0.2.0/8,guestfwd=tcp:%s:%d-unix:%s,server,nowait", - VMCHANNEL_ADDR, VMCHANNEL_PORT, unixsock); + GUESTFWD_ADDR, GUESTFWD_PORT, unixsock); add_cmdline (g, "-net"); add_cmdline (g, vmchannel); @@ -1049,7 +1047,7 @@ guestfs__launch (guestfs_h *g) */ snprintf (vmchannel, sizeof vmchannel, "channel,%d:unix:%s,server,nowait", - VMCHANNEL_PORT, unixsock); + GUESTFWD_PORT, unixsock); add_cmdline (g, "-net"); add_cmdline (g, vmchannel); -- 1.6.2.5
Jim Meyering
2009-Sep-17 14:44 UTC
[Libguestfs] [PATCH] Remove explicit guestfs=10.0.2.4:6666 kernel command line parameter.
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.
Reasonably Related Threads
- [PATCH] Enable new-style -chardev ... guestfwd command line
- [PATCH 0/2] Use link-local addresses when communicating between appliance and host (RHBZ#588763)
- [PATCH 0/3] RFC: Allow use of external QEMU process with libguestfs
- domain xml questions
- using vmchannel between 6.x host/guests