From: Richard Jones <rjones(a)trick.home.annexia.org&gt; --- src/guestfs.c | 59 +++++++++++++++++++++++++++++---------------------------- 1 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/guestfs.c b/src/guestfs.c index c735c1c..bc4fe4e 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -992,39 +992,22 @@ guestfs__launch (guestfs_h *g) */ g->cmdline[0] = g->qemu; -#define LINUX_CMDLINE \ - "panic=1 " /* force kernel to panic if daemon exits */ \ - "console=ttyS0 " /* serial console */ \ - "udevtimeout=300 " /* good for very slow systems (RHBZ#480319) */ \ - "noapic " /* workaround for RHBZ#502058 - ok if not SMP */ \ - "acpi=off " /* we don't need ACPI, turn it off */ \ - "cgroup_disable=memory " /* saves us about 5 MB of RAM */ - - /* Linux kernel command line. */ - snprintf (append, sizeof append, - LINUX_CMDLINE - "%s" /* (selinux) */ - "%s" /* (verbose) */ - "%s", /* (append) */ - g->selinux ? "selinux=1 enforcing=0 " : "selinux=0 ", - g->verbose ? "guestfs_verbose=1 " : " ", - g->append ? g->append : ""); - snprintf (memsize_str, sizeof memsize_str, "%d", g->memsize); add_cmdline (g, "-m"); add_cmdline (g, memsize_str); add_cmdline (g, "-no-reboot"); /* Force exit instead of reboot on panic */ - add_cmdline (g, "-kernel"); - add_cmdline (g, (char *) kernel); - add_cmdline (g, "-initrd"); - add_cmdline (g, (char *) initrd); - add_cmdline (g, "-append"); - add_cmdline (g, append); add_cmdline (g, "-nographic"); add_cmdline (g, "-serial"); add_cmdline (g, "stdio"); + /* These options recommended by KVM developers to improve reliability. */ + if (qemu_supports (g, "-no-hpet")) + add_cmdline (g, "-no-hpet"); + + if (qemu_supports (g, "-rtc-td-hack")) + add_cmdline (g, "-rtc-td-hack"); + if (qemu_supports (g, "-chardev") && qemu_supports (g, "guestfwd")) { /* New-style -net user,guestfwd=... syntax for guestfwd. See: * @@ -1065,12 +1048,30 @@ guestfs__launch (guestfs_h *g) add_cmdline (g, "-net"); add_cmdline (g, "nic,model=" NET_IF ",vlan=0"); - /* These options recommended by KVM developers to improve reliability. */ - if (qemu_supports (g, "-no-hpet")) - add_cmdline (g, "-no-hpet"); +#define LINUX_CMDLINE \ + "panic=1 " /* force kernel to panic if daemon exits */ \ + "console=ttyS0 " /* serial console */ \ + "udevtimeout=300 " /* good for very slow systems (RHBZ#480319) */ \ + "noapic " /* workaround for RHBZ#502058 - ok if not SMP */ \ + "acpi=off " /* we don't need ACPI, turn it off */ \ + "cgroup_disable=memory " /* saves us about 5 MB of RAM */ - if (qemu_supports (g, "-rtc-td-hack")) - add_cmdline (g, "-rtc-td-hack"); + /* Linux kernel command line. */ + snprintf (append, sizeof append, + LINUX_CMDLINE + "%s" /* (selinux) */ + "%s" /* (verbose) */ + "%s", /* (append) */ + g->selinux ? "selinux=1 enforcing=0 " : "selinux=0 ", + g->verbose ? "guestfs_verbose=1 " : " ", + g->append ? g->append : ""); + + add_cmdline (g, "-kernel"); + add_cmdline (g, (char *) kernel); + add_cmdline (g, "-initrd"); + add_cmdline (g, (char *) initrd); + add_cmdline (g, "-append"); + add_cmdline (g, append); /* Finish off the command line. */ incr_cmdline_size (g); -- 1.6.2.5

From: Richard Jones <rjones(a)trick.home.annexia.org&gt; This commit changes guestfs_launch so that it both launches the appliance and waits until it is ready (ie. the daemon communicates back to us). Since we removed the pretence that we could implement a low-level asynchronous API, the need to call launch() followed by wait_ready() has looked a bit silly. Now guestfs_wait_ready() is basically a no-op. It is left in the API for backwards compatibility. Any calls to guestfs_wait_ready() can be removed from client code. --- TODO | 3 +- cat/virt-cat.pl | 1 - df/virt-df.pl | 1 - examples/hello.c | 1 - examples/to-xml.c | 1 - fish/fish.c | 2 - guestfs.pod | 24 +++++---------- haskell/Guestfs010Launch.hs | 1 - haskell/Guestfs050LVCreate.hs | 1 - inspector/virt-inspector.pl | 1 - java/t/GuestFS010Launch.java | 1 - java/t/GuestFS050LVCreate.java | 1 - ocaml/examples/lvs.ml | 1 - ocaml/t/guestfs_010_launch.ml | 1 - ocaml/t/guestfs_050_lvcreate.ml | 1 - ocaml/t/guestfs_060_readdir.ml | 1 - perl/examples/lvs.pl | 1 - perl/lib/Sys/Guestfs/Lib.pm | 2 +- perl/t/050-lvcreate.t | 4 +-- perl/t/060-readdir.t | 4 +-- perl/t/510-lib-file-arch.t | 4 +-- python/t/010-launch.py | 1 - python/t/050-lvcreate.py | 1 - ruby/tests/tc_050_lvcreate.rb | 1 - ruby/tests/tc_rhbz507346.rb | 1 - src/generator.ml | 28 +++++++---------- src/guestfs.c | 62 +++++++++++++++++++-------------------- test-tool/test-tool.c | 10 +----- 28 files changed, 56 insertions(+), 105 deletions(-) diff --git a/TODO b/TODO index bfcc477..12b1ce6 100644 --- a/TODO +++ b/TODO @@ -214,7 +214,7 @@ Quick Perl scripts Currently we can't do Perl "one-liners". ie. The current syntax for any short Perl one-liner would be: - perl -MSys::Guestfs -e '$g = Sys::Guestfs->new(); $g->add_drive ("foo"); $g->launch; $g->wait_ready; $g->mount ("/dev/sda1", "/"); ....' + perl -MSys::Guestfs -e '$g = Sys::Guestfs->new(); $g->add_drive ("foo"); $g->launch; $g->mount ("/dev/sda1", "/"); ....' You can see we're well beyond a single line just getting to the point of adding drives and mounting. @@ -235,7 +235,6 @@ which is equivalent to the following sequence of calls: $h->set_autosync (1); $h->add_drive_ro ($filename); $h->launch (); - $h->wait_ready (); $h->mount_ro (\"/dev/sda1\", \"/\"); Command-line form would be: diff --git a/cat/virt-cat.pl b/cat/virt-cat.pl index 1017876..8db5348 100755 --- a/cat/virt-cat.pl +++ b/cat/virt-cat.pl @@ -140,7 +140,6 @@ if ($uri) { } $g->launch (); -$g->wait_ready (); # List of possible filesystems. my @partitions = get_partitions ($g); diff --git a/df/virt-df.pl b/df/virt-df.pl index a1b2228..21ba791 100755 --- a/df/virt-df.pl +++ b/df/virt-df.pl @@ -183,7 +183,6 @@ sub do_df } $g->launch (); - $g->wait_ready (); my @partitions = get_partitions ($g); diff --git a/examples/hello.c b/examples/hello.c index 1c12f8e..8b36ed4 100644 --- a/examples/hello.c +++ b/examples/hello.c @@ -27,7 +27,6 @@ main (int argc, char *argv[]) if (guestfs_add_drive (g, argv[1]) == -1) exit (1); if (guestfs_launch (g) == -1) exit (1); - if (guestfs_wait_ready (g) == -1) exit (1); if (guestfs_mount (g, argv[2], "/") == -1) exit (1); diff --git a/examples/to-xml.c b/examples/to-xml.c index 2cf3de8..6d0a1df 100644 --- a/examples/to-xml.c +++ b/examples/to-xml.c @@ -51,7 +51,6 @@ main (int argc, char *argv[]) CALL (guestfs_add_drive (g, argv[i]), -1); CALL (guestfs_launch (g), -1); - CALL (guestfs_wait_ready (g), -1); printf ("<guestfs-system>\n"); diff --git a/fish/fish.c b/fish/fish.c index 9215f4e..f8c0483 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -81,8 +81,6 @@ launch (guestfs_h *_g) if (guestfs_is_config (g)) { if (guestfs_launch (g) == -1) return -1; - if (guestfs_wait_ready (g) == -1) - return -1; } return 0; } diff --git a/guestfs.pod b/guestfs.pod index c5b756e..33b84d7 100644 --- a/guestfs.pod +++ b/guestfs.pod @@ -11,7 +11,6 @@ guestfs - Library for accessing and modifying virtual machine images guestfs_h *handle = guestfs_create (); guestfs_add_drive (handle, "guest.img"); guestfs_launch (handle); - guestfs_wait_ready (handle); guestfs_mount (handle, "/dev/sda1", "/"); guestfs_touch (handle, "/hello"); guestfs_sync (handle); @@ -56,7 +55,6 @@ functions in the following order: */ guestfs_launch (handle); - guestfs_wait_ready (handle); /* now you can examine what partitions, LVs etc are available * you have to mount / at least @@ -73,7 +71,7 @@ functions in the following order: guestfs_close (handle); -C<guestfs_wait_ready> and all of the actions including C<guestfs_sync> +C<guestfs_launch> and all of the actions including C<guestfs_sync> are blocking calls. You can use the low-level event API to do non-blocking operations instead. @@ -100,8 +98,7 @@ You have to call C<guestfs_add_drive> on the handle at least once. This function returns a non-NULL pointer to a handle on success or NULL on error. -After configuring the handle, you have to call C<guestfs_launch> and -C<guestfs_wait_ready>. +After configuring the handle, you have to call C<guestfs_launch>. You may also want to configure error handling for the handle. See L</ERROR HANDLING> section below. @@ -443,7 +440,7 @@ libguestfs uses a state machine to model the child process: / | | LAUNCHING | / | \___________/ / | / - / | guestfs_wait_ready + / | guestfs_launch / | / ______ / __|____V / \ ------> / \ @@ -463,12 +460,10 @@ Configuration commands for qemu such as C<guestfs_add_drive> can only be issued when in the CONFIG state. The high-level API offers two calls that go from CONFIG through -LAUNCHING to READY. C<guestfs_launch> is a non-blocking call that -starts up the child process, immediately moving from CONFIG to -LAUNCHING. C<guestfs_wait_ready> blocks until the child process is -READY to accept commands (or until some failure or timeout). The -low-level event API described below provides a non-blocking way to -replace C<guestfs_wait_ready>. +LAUNCHING to READY. C<guestfs_launch> blocks until the child process +is READY to accept commands (or until some failure or timeout). +C<guestfs_launch> internally moves the state from CONFIG to LAUNCHING +while it is running. High-level API actions such as C<guestfs_mount> can only be issued when in the READY state. These high-level API calls block waiting for @@ -535,9 +530,6 @@ The callback function C<cb> will be called when the child process becomes ready first time after it has been launched. (This corresponds to a transition from LAUNCHING to the READY state). -You can use this instead of C<guestfs_wait_ready> to implement a -non-blocking wait for the child process to finish booting up. - =head1 BLOCK DEVICE NAMING In the kernel there is now quite a profusion of schemata for naming @@ -780,7 +772,7 @@ parameters, but with the roles of daemon and library reversed. Because the underlying channel (QEmu -net channel) doesn't have any sort of connection control, when the daemon launches it sends an initial word (C<GUESTFS_LAUNCH_FLAG>) which indicates that the guest -and daemon is alive. This is what C<guestfs_wait_ready> waits for. +and daemon is alive. This is what C<guestfs_launch> waits for. =head1 QEMU WRAPPERS diff --git a/haskell/Guestfs010Launch.hs b/haskell/Guestfs010Launch.hs index 27e49f7..930d666 100644 --- a/haskell/Guestfs010Launch.hs +++ b/haskell/Guestfs010Launch.hs @@ -28,5 +28,4 @@ main = do hClose fd Guestfs.add_drive g "test.img" Guestfs.launch g - Guestfs.wait_ready g removeLink "test.img" diff --git a/haskell/Guestfs050LVCreate.hs b/haskell/Guestfs050LVCreate.hs index b82bf0d..502f6b2 100644 --- a/haskell/Guestfs050LVCreate.hs +++ b/haskell/Guestfs050LVCreate.hs @@ -28,7 +28,6 @@ main = do hClose fd Guestfs.add_drive g "test.img" Guestfs.launch g - Guestfs.wait_ready g Guestfs.pvcreate g "/dev/sda" Guestfs.vgcreate g "VG" ["/dev/sda"] diff --git a/inspector/virt-inspector.pl b/inspector/virt-inspector.pl index 943f32e..86b1795 100755 --- a/inspector/virt-inspector.pl +++ b/inspector/virt-inspector.pl @@ -219,7 +219,6 @@ if ($uri) { } $g->launch (); -$g->wait_ready (); =head1 OUTPUT FORMAT diff --git a/java/t/GuestFS010Launch.java b/java/t/GuestFS010Launch.java index 198ea85..ad0665b 100644 --- a/java/t/GuestFS010Launch.java +++ b/java/t/GuestFS010Launch.java @@ -29,7 +29,6 @@ public class GuestFS010Launch { f.close (); g.add_drive ("test.img"); g.launch (); - g.wait_ready (); g.close (); File f2 = new File ("test.img"); f2.delete (); diff --git a/java/t/GuestFS050LVCreate.java b/java/t/GuestFS050LVCreate.java index 012d68b..940239a 100644 --- a/java/t/GuestFS050LVCreate.java +++ b/java/t/GuestFS050LVCreate.java @@ -31,7 +31,6 @@ public class GuestFS050LVCreate { g.add_drive ("test.img"); g.launch (); - g.wait_ready (); g.pvcreate ("/dev/sda"); g.vgcreate ("VG", new String[] {"/dev/sda"}); diff --git a/ocaml/examples/lvs.ml b/ocaml/examples/lvs.ml index 12c7c15..74998b9 100644 --- a/ocaml/examples/lvs.ml +++ b/ocaml/examples/lvs.ml @@ -11,7 +11,6 @@ let () = let h = Guestfs.create () in Guestfs.add_drive h Sys.argv.(1); Guestfs.launch h; - Guestfs.wait_ready h; let pvs = Guestfs.pvs h in printf "PVs found: [ %s ]\n" (String.concat "; " (Array.to_list pvs)); diff --git a/ocaml/t/guestfs_010_launch.ml b/ocaml/t/guestfs_010_launch.ml index b253e0c..ae0f9f1 100644 --- a/ocaml/t/guestfs_010_launch.ml +++ b/ocaml/t/guestfs_010_launch.ml @@ -27,6 +27,5 @@ let () = Guestfs.add_drive g "test.img"; Guestfs.launch g; - Guestfs.wait_ready g; unlink "test.img" diff --git a/ocaml/t/guestfs_050_lvcreate.ml b/ocaml/t/guestfs_050_lvcreate.ml index 2d5bb1f..ad98c06 100644 --- a/ocaml/t/guestfs_050_lvcreate.ml +++ b/ocaml/t/guestfs_050_lvcreate.ml @@ -27,7 +27,6 @@ let () = Guestfs.add_drive g "test.img"; Guestfs.launch g; - Guestfs.wait_ready g; Guestfs.pvcreate g "/dev/sda"; Guestfs.vgcreate g "VG" [|"/dev/sda"|]; diff --git a/ocaml/t/guestfs_060_readdir.ml b/ocaml/t/guestfs_060_readdir.ml index e9368a8..8035a09 100644 --- a/ocaml/t/guestfs_060_readdir.ml +++ b/ocaml/t/guestfs_060_readdir.ml @@ -27,7 +27,6 @@ let () = Guestfs.add_drive g "test.img"; Guestfs.launch g; - Guestfs.wait_ready g; Guestfs.sfdisk g "/dev/sda" 0 0 0 [|","|]; Guestfs.mkfs g "ext2" "/dev/sda1"; diff --git a/perl/examples/lvs.pl b/perl/examples/lvs.pl index 152db08..1a84017 100755 --- a/perl/examples/lvs.pl +++ b/perl/examples/lvs.pl @@ -14,7 +14,6 @@ $h->add_drive ($ARGV[0]); print "Launching, this can take a few seconds\n"; $h->launch (); -$h->wait_ready (); print "Looking for PVs on the disk image\n"; my @pvs = $h->pvs (); diff --git a/perl/lib/Sys/Guestfs/Lib.pm b/perl/lib/Sys/Guestfs/Lib.pm index 96ada0d..a9868e2 100644 --- a/perl/lib/Sys/Guestfs/Lib.pm +++ b/perl/lib/Sys/Guestfs/Lib.pm @@ -100,7 +100,7 @@ read-write handle, this function will refuse to use active libvirt domains. The handle is still in the config state when it is returned, so you -have to call C<$g-E<gt>launch ()> and C<$g-E<gt>wait_ready>. +have to call C<$g-E<gt>launch ()>. The optional C<address> parameter can be added to specify the libvirt URI. In addition, L<Sys::Virt(3)> lists other parameters which are diff --git a/perl/t/050-lvcreate.t b/perl/t/050-lvcreate.t index 1b3e313..a8f3e7b 100644 --- a/perl/t/050-lvcreate.t +++ b/perl/t/050-lvcreate.t @@ -17,7 +17,7 @@ use strict; use warnings; -use Test::More tests => 12; +use Test::More tests => 11; use Sys::Guestfs; @@ -33,8 +33,6 @@ ok (1); $h->launch (); ok (1); -$h->wait_ready (); -ok (1); $h->pvcreate ("/dev/sda"); ok (1); diff --git a/perl/t/060-readdir.t b/perl/t/060-readdir.t index 17dfd24..898b44f 100644 --- a/perl/t/060-readdir.t +++ b/perl/t/060-readdir.t @@ -17,7 +17,7 @@ use strict; use warnings; -use Test::More tests => 13; +use Test::More tests => 12; use Sys::Guestfs; @@ -33,8 +33,6 @@ ok (1); $h->launch (); ok (1); -$h->wait_ready (); -ok (1); $h->sfdisk ("/dev/sda", 0, 0, 0, [","]); ok (1); diff --git a/perl/t/510-lib-file-arch.t b/perl/t/510-lib-file-arch.t index 5c70449..dfe32bc 100644 --- a/perl/t/510-lib-file-arch.t +++ b/perl/t/510-lib-file-arch.t @@ -22,7 +22,7 @@ BEGIN { use Test::More; eval "use Locale::TextDomain";; if (exists $INC{"Locale/TextDomain.pm"}) { - plan tests => 17; + plan tests => 16; } else { plan skip_all => "no perl-libintl module"; exit 0; @@ -40,8 +40,6 @@ ok (1); $h->launch (); ok (1); -$h->wait_ready (); -ok (1); $h->mount_ro ("/dev/sda", "/"); ok (1); diff --git a/python/t/010-launch.py b/python/t/010-launch.py index c1316d2..93a8154 100644 --- a/python/t/010-launch.py +++ b/python/t/010-launch.py @@ -24,6 +24,5 @@ f.truncate (500 * 1024 * 1024) f.close () g.add_drive ("test.img") g.launch () -g.wait_ready () os.unlink ("test.img") diff --git a/python/t/050-lvcreate.py b/python/t/050-lvcreate.py index 8503023..f98fe29 100644 --- a/python/t/050-lvcreate.py +++ b/python/t/050-lvcreate.py @@ -24,7 +24,6 @@ f.truncate (500 * 1024 * 1024) f.close () g.add_drive ("test.img") g.launch () -g.wait_ready () g.pvcreate ("/dev/sda") g.vgcreate ("VG", ["/dev/sda"]) g.lvcreate ("LV1", "VG", 200) diff --git a/ruby/tests/tc_050_lvcreate.rb b/ruby/tests/tc_050_lvcreate.rb index ea87e39..37ae94d 100644 --- a/ruby/tests/tc_050_lvcreate.rb +++ b/ruby/tests/tc_050_lvcreate.rb @@ -30,7 +30,6 @@ class TestLoad < Test::Unit::TestCase g.add_drive("test.img") g.launch() - g.wait_ready() g.pvcreate("/dev/sda") g.vgcreate("VG", ["/dev/sda"]); diff --git a/ruby/tests/tc_rhbz507346.rb b/ruby/tests/tc_rhbz507346.rb index 3bd4bb6..8c5b34f 100644 --- a/ruby/tests/tc_rhbz507346.rb +++ b/ruby/tests/tc_rhbz507346.rb @@ -30,7 +30,6 @@ class TestLoad < Test::Unit::TestCase g.add_drive("test.img") g.launch() - g.wait_ready() exception = assert_raise TypeError do g.command(1) diff --git a/src/generator.ml b/src/generator.ml index 179665b..e5e2681 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -440,13 +440,18 @@ You should call this after configuring the handle ("wait_ready", (RErr, []), -1, [NotInFish], [], - "wait until the qemu subprocess launches", + "wait until the qemu subprocess launches (no op)", "\ -Internally libguestfs is implemented by running a virtual machine -using L<qemu(1)>. +This function is a no op. -You should call this after C<guestfs_launch> to wait for the launch -to complete."); +In versions of the API E<lt> 1.0.71 you had to call this function +just after calling C<guestfs_launch> to wait for the launch +to complete. However this is no longer necessary because +C<guestfs_launch> now does the waiting. + +If you see any calls to this function in code then you can just +remove them, unless you want to retain compatibility with older +versions of the API."); ("kill_subprocess", (RErr, []), -1, [], [], @@ -4625,12 +4630,9 @@ static int check_state (guestfs_h *g, const char *caller) { if (!guestfs__is_ready (g)) { - if (guestfs__is_config (g)) + if (guestfs__is_config (g) || guestfs__is_launching (g)) error (g, \"%%s: call launch before using this function\\n(in guestfish, don't forget to use the 'run' command)\", caller); - else if (guestfs__is_launching (g)) - error (g, \"%%s: call wait_ready() before using this function\", - caller); else error (g, \"%%s called from the wrong state, %%d != READY\", caller, guestfs__get_state (g)); @@ -5522,11 +5524,6 @@ int main (int argc, char *argv[]) /* Set a timeout in case qemu hangs during launch (RHBZ#505329). */ alarm (600); - if (guestfs_wait_ready (g) == -1) { - printf (\"guestfs_wait_ready FAILED\\n\"); - exit (1); - } - /* Cancel previous alarm. */ alarm (0); @@ -7390,7 +7387,6 @@ Sys::Guestfs - Perl bindings for libguestfs my $h = Sys::Guestfs->new (); $h->add_drive ('guest.img'); $h->launch (); - $h->wait_ready (); $h->mount ('/dev/sda1', '/'); $h->touch ('/hello'); $h->sync (); @@ -7928,7 +7924,6 @@ import guestfs g = guestfs.GuestFS () g.add_drive (\"guest.img\") g.launch () -g.wait_ready () parts = g.list_partitions () The guestfs module provides a Python binding to the libguestfs API @@ -7963,7 +7958,6 @@ g.add_drive (\"guest.img\") # Launch the qemu subprocess and wait for it to become ready: g.launch () -g.wait_ready () # Now you can issue commands, for example: logvols = g.lvs () diff --git a/src/guestfs.c b/src/guestfs.c index 17d812a..417d17f 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -1208,6 +1208,32 @@ guestfs__launch (guestfs_h *g) connected: g->state = LAUNCHING; + + /* Wait for qemu to start and to connect back to us via vmchannel and + * send the GUESTFS_LAUNCH_FLAG message. + */ + uint32_t size; + void *buf = NULL; + r = recv_from_daemon (g, &size, &buf); + free (buf); + + if (r == -1) return -1; + + if (size != GUESTFS_LAUNCH_FLAG) { + error (g, _("guestfs_launch failed, see earlier error messages")); + goto cleanup2; + } + + /* This is possible in some really strange situations, such as + * guestfsd starts up OK but then qemu immediately exits. Check for + * it because the caller is probably expecting to be able to send + * commands after this function returns. + */ + if (g->state != READY) { + error (g, _("qemu launched and contacted daemon, but state != READY")); + goto cleanup2; + } + return 0; cleanup2: @@ -1381,45 +1407,17 @@ qemu_supports (guestfs_h *g, const char *option) return g->qemu_help && strstr (g->qemu_help, option) != NULL; } +/* You had to call this function after launch in versions <= 1.0.70, + * but it is now a no-op. + */ int guestfs__wait_ready (guestfs_h *g) { - int r; - uint32_t size; - void *buf = NULL; - - if (g->state == READY) return 0; - - if (g->state == BUSY) { - error (g, _("qemu has finished launching already")); - return -1; - } - - if (g->state != LAUNCHING) { + if (g->state != READY) { error (g, _("qemu has not been launched yet")); return -1; } - r = recv_from_daemon (g, &size, &buf); - free (buf); - - if (r == -1) return -1; - - if (size != GUESTFS_LAUNCH_FLAG) { - error (g, _("guestfs_wait_ready failed, see earlier error messages")); - return -1; - } - - /* This is possible in some really strange situations, such as - * guestfsd starts up OK but then qemu immediately exits. Check for - * it because the caller is probably expecting to be able to send - * commands after this function returns. - */ - if (g->state != READY) { - error (g, _("qemu launched and contacted daemon, but state != READY")); - return -1; - } - return 0; } diff --git a/test-tool/test-tool.c b/test-tool/test-tool.c index 508c31a..c1a3de7 100644 --- a/test-tool/test-tool.c +++ b/test-tool/test-tool.c @@ -186,20 +186,14 @@ main (int argc, char *argv[]) printf ("guestfs_get_verbose: %d\n", guestfs_get_verbose (g)); /* Launch the guest handle. */ - if (guestfs_launch (g) == -1) { - fprintf (stderr, - _("libguestfs-test-tool: failed to launch appliance\n")); - exit (1); - } - printf ("Launching appliance, timeout set to %d seconds.\n", timeout); fflush (stdout); alarm (timeout); - if (guestfs_wait_ready (g) == -1) { + if (guestfs_launch (g) == -1) { fprintf (stderr, - _("libguestfs-test-tool: failed or timed out in 'wait_ready'\n")); + _("libguestfs-test-tool: failed to launch appliance\n")); exit (1); } -- 1.6.2.5

From: Richard Jones <rjones(a)trick.home.annexia.org&gt; --- src/guestfs.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/guestfs.c b/src/guestfs.c index bc34f60..55732f9 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 GUESTFWD_PORT 6666 #define GUESTFWD_ADDR "10.0.2.4" +#define GUESTFWD_PORT "6666" /* GuestFS handle and connection. */ enum state { CONFIG, LAUNCHING, READY, BUSY, NO_HANDLE }; @@ -1026,8 +1026,8 @@ guestfs__launch (guestfs_h *g) snprintf (buf, sizeof buf, "user,vlan=0,net=10.0.2.0/8," - "guestfwd=tcp:%s:%d-chardev:guestfsvmc", - GUESTFWD_ADDR, GUESTFWD_PORT); + "guestfwd=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT + "-chardev:guestfsvmc"); add_cmdline (g, "-net"); add_cmdline (g, buf); @@ -1036,7 +1036,7 @@ guestfs__launch (guestfs_h *g) * syntax, or if not then we'll get a quick failure. */ snprintf (buf, sizeof buf, - "channel,%d:unix:%s,server,nowait", GUESTFWD_PORT, unixsock); + "channel," GUESTFWD_PORT ":unix:%s,server,nowait", unixsock); add_cmdline (g, "-net"); add_cmdline (g, buf); -- 1.6.2.5

From: Richard Jones <rjones(a)trick.home.annexia.org&gt; This reimplements parts of commit da0a4f8d1f6ddd302ceba028d87c6e009589e503 in a different, but compatible way. We pass guestfs_vmchannel=tcp:<ip>:<port> on the command line. This is intended to be used as follows (now and in future versions): tcp:10.0.2.4:6666 for guestfwd vmchannel tcp:10.0.2.2:<port> for future "no vmchannel" implementation /dev/vcon4 for future virtio-console vmchannel* It also accepts the old-style guestfs=10.0.2.4:6666 parameter which is sent by older libraries, and turns this transparently into the correct format above. If no guestfs_vmchannel is passed, then this defaults to the guestfwd vmchannel which older libraries would expect. * Maybe this last one should be dev:/dev/vcon4 or file:/dev/vcon4, but we don't need to decide that now. --- daemon/guestfsd.c | 147 +++++++++++++++++++++++++++++++++++++++++++--------- src/guestfs.c | 4 ++ 2 files changed, 125 insertions(+), 26 deletions(-) diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index a38452c..ce14dbf 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -68,8 +68,9 @@ int sysroot_len = 8; int main (int argc, char *argv[]) { - static const char *options = "f?"; + static const char *options = "fc:?"; static const struct option long_options[] = { + { "channel", 0, 0, 'c' }, { "foreground", 0, 0, 'f' }, { "help", 0, 0, '?' }, { 0, 0, 0, 0 } @@ -77,6 +78,7 @@ main (int argc, char *argv[]) int c; int dont_fork = 0; char *cmdline; + char *vmchannel = NULL; #ifdef HAVE_REGISTER_PRINTF_SPECIFIER /* http://udrepper.livejournal.com/20948.html */ @@ -96,6 +98,10 @@ main (int argc, char *argv[]) if (c == -1) break; switch (c) { + case 'c': + vmchannel = optarg; + break; + case 'f': dont_fork = 1; break; @@ -148,38 +154,127 @@ main (int argc, char *argv[]) /* We document that umask defaults to 022 (it should be this anyway). */ umask (022); - /* Resolve the hostname. */ - struct addrinfo *res, *rr; - struct addrinfo hints; - int r; - memset (&hints, 0, sizeof hints); - hints.ai_socktype = SOCK_STREAM; - hints.ai_flags = AI_ADDRCONFIG; - r = getaddrinfo (GUESTFWD_ADDR, GUESTFWD_PORT, &hints, &res); - if (r != 0) { - fprintf (stderr, "%s:%s: %s\n", - GUESTFWD_ADDR, GUESTFWD_PORT, gai_strerror (r)); - exit (1); + /* Get the vmchannel string. + * + * Sources: + * --channel/-c option on the command line + * guestfs_vmchannel=... from the kernel command line + * guestfs=... from the kernel command line + * built-in default + * + * At the moment we expect this to contain "tcp:ip:port" but in + * future it might contain a device name, eg. "/dev/vcon4" for + * virtio-console vmchannel. + */ + if (vmchannel == NULL && cmdline) { + char *p; + int len; + + p = strstr (cmdline, "guestfs_vmchannel="); + if (p) { + len = strcspn (p + 18, " \t\n"); + vmchannel = strndup (p + 18, len); + if (!vmchannel) { + perror ("strndup"); + exit (1); + } + } + + /* Old libraries passed guestfs=host:port. Rewrite it as tcp:host:port. */ + if (vmchannel == NULL) { + p = strstr (cmdline, "guestfs="); + if (p) { + len = strcspn (p + 4, " \t\n"); + vmchannel = strndup (p + 4, len); + if (!vmchannel) { + perror ("strndup"); + exit (1); + } + memcpy (vmchannel, "tcp:", 4); + } + } } - /* Connect to the given TCP socket. */ + /* Default vmchannel. */ + if (vmchannel == NULL) { + vmchannel = strdup ("tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT); + if (!vmchannel) { + perror ("strdup"); + exit (1); + } + } + + if (verbose) + printf ("vmchannel: %s\n", vmchannel); + + /* Connect to vmchannel. */ int sock = -1; - for (rr = res; rr != NULL; rr = rr->ai_next) { - sock = socket (rr->ai_family, rr->ai_socktype, rr->ai_protocol); - if (sock != -1) { - if (connect (sock, rr->ai_addr, rr->ai_addrlen) == 0) - break; - perror ("connect"); - - close (sock); - sock = -1; + + if (strncmp (vmchannel, "tcp:", 4) == 0) { + /* Resolve the hostname. */ + struct addrinfo *res, *rr; + struct addrinfo hints; + int r; + char *host, *port; + + host = vmchannel+4; + port = strchr (host, ':'); + if (port) { + port[0] = '\0'; + port++; + } else { + fprintf (stderr, "vmchannel: expecting \"tcp:<ip>:<port>\": %s\n", + vmchannel); + exit (1); } + + memset (&hints, 0, sizeof hints); + hints.ai_socktype = SOCK_STREAM; + hints.ai_flags = AI_ADDRCONFIG; + r = getaddrinfo (host, port, &hints, &res); + if (r != 0) { + fprintf (stderr, "%s:%s: %s\n", + host, port, gai_strerror (r)); + exit (1); + } + + /* Connect to the given TCP socket. */ + for (rr = res; rr != NULL; rr = rr->ai_next) { + sock = socket (rr->ai_family, rr->ai_socktype, rr->ai_protocol); + if (sock != -1) { + if (connect (sock, rr->ai_addr, rr->ai_addrlen) == 0) + break; + perror ("connect"); + + close (sock); + sock = -1; + } + } + freeaddrinfo (res); + } else { + fprintf (stderr, + "unknown vmchannel connection type: %s\n" + "expecting \"tcp:<ip>:<port>\"\n", + vmchannel); + exit (1); } - freeaddrinfo (res); if (sock == -1) { - fprintf (stderr, "connection to %s:%s failed\n", - GUESTFWD_ADDR, GUESTFWD_PORT); + fprintf (stderr, + "\n" + "Failed to connect to any vmchannel implementation.\n" + "vmchannel: %s\n" + "\n" + "This is a fatal error and the appliance will now exit.\n" + "\n" + "Usually this error is caused by either QEMU or the appliance\n" + "kernel not supporting the vmchannel method that the\n" + "libguestfs library chose to use. Please run\n" + "'libguestfs-test-tool' and provide the complete, unedited\n" + "output to the libguestfs developers, either in a bug report\n" + "or on the libguestfs redhat com mailing list.\n" + "\n", + vmchannel); exit (1); } diff --git a/src/guestfs.c b/src/guestfs.c index 55732f9..ec7473e 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -984,6 +984,7 @@ guestfs__launch (guestfs_h *g) if (r == 0) { /* Child (qemu). */ char buf[256]; + const char *vmchannel = NULL; /* Set up the full command line. Do this in the subprocess so we * don't need to worry about cleaning up. @@ -1045,6 +1046,7 @@ guestfs__launch (guestfs_h *g) } add_cmdline (g, "-net"); add_cmdline (g, "nic,model=" NET_IF ",vlan=0"); + vmchannel = "guestfs_vmchannel=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT " "; #define LINUX_CMDLINE \ "panic=1 " /* force kernel to panic if daemon exits */ \ @@ -1058,9 +1060,11 @@ guestfs__launch (guestfs_h *g) snprintf (buf, sizeof buf, LINUX_CMDLINE "%s" /* (selinux) */ + "%s" /* (vmchannel) */ "%s" /* (verbose) */ "%s", /* (append) */ g->selinux ? "selinux=1 enforcing=0 " : "selinux=0 ", + vmchannel ? vmchannel : "", g->verbose ? "guestfs_verbose=1 " : "", g->append ? g->append : ""); -- 1.6.2.5

From: Richard Jones <rjones(a)trick.home.annexia.org&gt; This commit removes the requirement for vmchannel, although retaining support for it. In this configuration, known as "null vmchannel", the library listens on a random loopback port. It passes the number of this port to the appliance (guestfs_vmchannel=tcp:10.0.2.2:12345), and the daemon then connects back. The library, instead of connecting, listens and accepts the connection during guestfs_launch. QEMU SLIRP (user mode networking) is still required to make this work: SLIRP forwards the TCP connection transparently (instead of explicitly as with guestfwd) to 127.0.0.1:<port> There is a window in which any other local process on the machine could see the port number in the qemu command line and try to connect to it. This would be a devastating security hole, because any local process could pretend to be the daemon and feed back malicious replies to our remote procedure calls. To prevent this, we check the UID of the other side of the TCP connection. If the UID is different from the library's EUID, then we reject the connection. To do this we have to parse /proc/net/tcp. (On Solaris we could use getsockopt (SO_PEERCRED), but this doesn't work on Linux TCP sockets). Other vmchannel(s) are still supported. This is important, because we can't in general be sure the qemu will always support SLIRP. In particular, in recent versions of qemu it is possible to compile out SLIRP. --- src/guestfs.c | 366 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 302 insertions(+), 64 deletions(-) diff --git a/src/guestfs.c b/src/guestfs.c index ec7473e..60be453 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -56,6 +56,14 @@ #include <sys/un.h> #endif +#ifdef HAVE_NETINET_IN_H +#include <netinet/in.h> +#endif + +#ifdef HAVE_ARPA_INET_H +#include <arpa/inet.h> +#endif + #include "guestfs.h" #include "guestfs-internal-actions.h" #include "guestfs_protocol.h" @@ -80,6 +88,8 @@ static void default_error_cb (guestfs_h *g, void *data, const char *msg); static int send_to_daemon (guestfs_h *g, const void *v_buf, size_t n); static int recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn); +static int accept_from_daemon (guestfs_h *g); +static int check_peer_euid (guestfs_h *g, int sock, uid_t *rtn); static void close_handles (void); #define UNIX_PATH_MAX 108 @@ -837,6 +847,7 @@ guestfs__launch (guestfs_h *g) int tries; char *path, *pelem, *pend; char *kernel = NULL, *initrd = NULL; + int null_vmchannel_sock; char unixsock[256]; struct sockaddr_un addr; @@ -963,9 +974,54 @@ guestfs__launch (guestfs_h *g) if (test_qemu (g) == -1) goto cleanup0; - /* Make the vmchannel socket. */ - snprintf (unixsock, sizeof unixsock, "%s/sock", g->tmpdir); - unlink (unixsock); + /* Choose which vmchannel implementation to use. */ + if (qemu_supports (g, "-net user")) { + /* The "null vmchannel" implementation. Requires SLIRP (user mode + * networking in qemu) but no other vmchannel support. The daemon + * will connect back to a random port number on localhost. + */ + struct sockaddr_in addr; + socklen_t addrlen = sizeof addr; + + g->sock = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP); + if (g->sock == -1) { + perrorf (g, "socket"); + goto cleanup0; + } + addr.sin_family = AF_INET; + addr.sin_port = htons (0); + addr.sin_addr.s_addr = htonl (INADDR_LOOPBACK); + if (bind (g->sock, (struct sockaddr *) &addr, addrlen) == -1) { + perrorf (g, "bind"); + goto cleanup0; + } + + if (listen (g->sock, 256) == -1) { + perrorf (g, "listen"); + goto cleanup0; + } + + if (getsockname (g->sock, (struct sockaddr *) &addr, &addrlen) == -1) { + perrorf (g, "getsockname"); + goto cleanup0; + } + + if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) { + perrorf (g, "fcntl"); + goto cleanup0; + } + + null_vmchannel_sock = ntohs (addr.sin_port); + if (g->verbose) + fprintf (stderr, "null_vmchannel_sock = %d\n", null_vmchannel_sock); + } else { + /* Using some vmchannel impl. We need to create a local Unix + * domain socket for qemu to use. + */ + snprintf (unixsock, sizeof unixsock, "%s/sock", g->tmpdir); + unlink (unixsock); + null_vmchannel_sock = 0; + } if (pipe (wfd) == -1 || pipe (rfd) == -1) { perrorf (g, "pipe"); @@ -1007,18 +1063,31 @@ guestfs__launch (guestfs_h *g) if (qemu_supports (g, "-rtc-td-hack")) add_cmdline (g, "-rtc-td-hack"); - if (qemu_supports (g, "-chardev") && qemu_supports (g, "guestfwd")) { - /* New-style -net user,guestfwd=... syntax for guestfwd. See: - * - * http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=c92ef6a22d3c71538fcc... - * - * The original suggested format doesn't work, see: - * - * http://lists.gnu.org/archive/html/qemu-devel/2009-07/msg01654.html - * - * However Gerd Hoffman privately suggested to me using -chardev - * instead, which does work. - */ + /* If qemu has SLIRP (user mode network) enabled then we can get + * away with "no vmchannel", where we just connect back to a random + * host port. + */ + if (null_vmchannel_sock) { + add_cmdline (g, "-net"); + add_cmdline (g, "user,vlan=0,net=10.0.2.0/8"); + + snprintf (buf, sizeof buf, + "guestfs_vmchannel=tcp:10.0.2.2:%d ", null_vmchannel_sock); + vmchannel = strdup (buf); + } + + /* New-style -net user,guestfwd=... syntax for guestfwd. See: + * + * http://git.savannah.gnu.org/cgit/qemu.git/commit/?id=c92ef6a22d3c71538fcc... + * + * The original suggested format doesn't work, see: + * + * http://lists.gnu.org/archive/html/qemu-devel/2009-07/msg01654.html + * + * However Gerd Hoffman privately suggested to me using -chardev + * instead, which does work. + */ + else if (qemu_supports (g, "-chardev") && qemu_supports (g, "guestfwd")) { snprintf (buf, sizeof buf, "socket,id=guestfsvmc,path=%s,server,nowait", unixsock); @@ -1032,10 +1101,14 @@ guestfs__launch (guestfs_h *g) add_cmdline (g, "-net"); add_cmdline (g, buf); - } else { - /* Not guestfwd. HOPEFULLY this qemu uses the older -net channel - * syntax, or if not then we'll get a quick failure. - */ + + vmchannel = "guestfs_vmchannel=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT " "; + } + + /* Not guestfwd. HOPEFULLY this qemu uses the older -net channel + * syntax, or if not then we'll get a quick failure. + */ + else { snprintf (buf, sizeof buf, "channel," GUESTFWD_PORT ":unix:%s,server,nowait", unixsock); @@ -1043,10 +1116,11 @@ guestfs__launch (guestfs_h *g) add_cmdline (g, buf); add_cmdline (g, "-net"); add_cmdline (g, "user,vlan=0,net=10.0.2.0/8"); + + vmchannel = "guestfs_vmchannel=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT " "; } add_cmdline (g, "-net"); add_cmdline (g, "nic,model=" NET_IF ",vlan=0"); - vmchannel = "guestfs_vmchannel=tcp:" GUESTFWD_ADDR ":" GUESTFWD_PORT " "; #define LINUX_CMDLINE \ "panic=1 " /* force kernel to panic if daemon exits */ \ @@ -1169,48 +1243,84 @@ guestfs__launch (guestfs_h *g) g->fd[0] = wfd[1]; /* stdin of child */ g->fd[1] = rfd[0]; /* stdout of child */ - /* Open the Unix socket. The vmchannel implementation that got - * merged with qemu sucks in a number of ways. Both ends do - * connect(2), which means that no one knows what, if anything, is - * connected to the other end, or if it becomes disconnected. Even - * worse, we have to wait some indeterminate time for qemu to create - * the socket and connect to it (which happens very early in qemu's - * start-up), so any code that uses vmchannel is inherently racy. - * Hence this silly loop. - */ - g->sock = socket (AF_UNIX, SOCK_STREAM, 0); - if (g->sock == -1) { - perrorf (g, "socket"); - goto cleanup1; - } + if (null_vmchannel_sock) { + int sock = -1; + uid_t uid; - if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) { - perrorf (g, "fcntl"); - goto cleanup2; - } + /* Null vmchannel implementation: We listen on g->sock for a + * connection. The connection could come from any local process + * so we must check it comes from the appliance (or at least + * from our UID) for security reasons. + */ + while (sock == -1) { + sock = accept_from_daemon (g); + if (sock == -1) + goto cleanup1; + + if (check_peer_euid (g, sock, &uid) == -1) + goto cleanup1; + if (uid != geteuid ()) { + fprintf (stderr, + "libguestfs: warning: unexpected connection from UID %d to port %d\n", + uid, null_vmchannel_sock); + close (sock); + continue; + } + } - addr.sun_family = AF_UNIX; - strncpy (addr.sun_path, unixsock, UNIX_PATH_MAX); - addr.sun_path[UNIX_PATH_MAX-1] = '\0'; + if (fcntl (sock, F_SETFL, O_NONBLOCK) == -1) { + perrorf (g, "fcntl"); + goto cleanup1; + } - tries = 100; - /* Always sleep at least once to give qemu a small chance to start up. */ - usleep (10000); - while (tries > 0) { - r = connect (g->sock, (struct sockaddr *) &addr, sizeof addr); - if ((r == -1 && errno == EINPROGRESS) || r == 0) - goto connected; + close (g->sock); + g->sock = sock; + } else { + /* Other vmchannel. Open the Unix socket. + * + * The vmchannel implementation that got merged with qemu sucks in + * a number of ways. Both ends do connect(2), which means that no + * one knows what, if anything, is connected to the other end, or + * if it becomes disconnected. Even worse, we have to wait some + * indeterminate time for qemu to create the socket and connect to + * it (which happens very early in qemu's start-up), so any code + * that uses vmchannel is inherently racy. Hence this silly loop. + */ + g->sock = socket (AF_UNIX, SOCK_STREAM, 0); + if (g->sock == -1) { + perrorf (g, "socket"); + goto cleanup1; + } - if (errno != ENOENT) - perrorf (g, "connect"); - tries--; - usleep (100000); - } + if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) { + perrorf (g, "fcntl"); + goto cleanup1; + } - error (g, _("failed to connect to vmchannel socket")); - goto cleanup2; + addr.sun_family = AF_UNIX; + strncpy (addr.sun_path, unixsock, UNIX_PATH_MAX); + addr.sun_path[UNIX_PATH_MAX-1] = '\0'; + + tries = 100; + /* Always sleep at least once to give qemu a small chance to start up. */ + usleep (10000); + while (tries > 0) { + r = connect (g->sock, (struct sockaddr *) &addr, sizeof addr); + if ((r == -1 && errno == EINPROGRESS) || r == 0) + goto connected; + + if (errno != ENOENT) + perrorf (g, "connect"); + tries--; + usleep (100000); + } + + error (g, _("failed to connect to vmchannel socket")); + goto cleanup1; + + connected: ; + } - connected: g->state = LAUNCHING; /* Wait for qemu to start and to connect back to us via vmchannel and @@ -1225,7 +1335,7 @@ guestfs__launch (guestfs_h *g) if (size != GUESTFS_LAUNCH_FLAG) { error (g, _("guestfs_launch failed, see earlier error messages")); - goto cleanup2; + goto cleanup1; } /* This is possible in some really strange situations, such as @@ -1235,14 +1345,11 @@ guestfs__launch (guestfs_h *g) */ if (g->state != READY) { error (g, _("qemu launched and contacted daemon, but state != READY")); - goto cleanup2; + goto cleanup1; } return 0; - cleanup2: - close (g->sock); - cleanup1: close (wfd[1]); close (rfd[0]); @@ -1252,12 +1359,16 @@ guestfs__launch (guestfs_h *g) if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; - g->sock = -1; g->pid = 0; g->recoverypid = 0; g->start_t = 0; cleanup0: + if (g->sock >= 0) { + close (g->sock); + g->sock = -1; + } + g->state = CONFIG; free (kernel); free (initrd); return -1; @@ -1411,6 +1522,83 @@ qemu_supports (guestfs_h *g, const char *option) return g->qemu_help && strstr (g->qemu_help, option) != NULL; } +/* Check the peer effective UID for a TCP socket. Ideally we'd like + * SO_PEERCRED for a loopback TCP socket. This isn't possible on + * Linux (but it is on Solaris!) so we read /proc/net/tcp instead. + */ +static int +check_peer_euid (guestfs_h *g, int sock, uid_t *rtn) +{ + struct sockaddr_in peer; + socklen_t addrlen = sizeof peer; + + if (getpeername (sock, (struct sockaddr *) &peer, &addrlen) == -1) { + perrorf (g, "getpeername"); + return -1; + } + + if (peer.sin_family != AF_INET || + ntohl (peer.sin_addr.s_addr) != INADDR_LOOPBACK) { + error (g, "check_peer_euid: unexpected connection from non-IPv4, non-loopback peer (family = %d, addr = %s)", + peer.sin_family, inet_ntoa (peer.sin_addr)); + return -1; + } + + struct sockaddr_in our; + addrlen = sizeof our; + if (getsockname (sock, (struct sockaddr *) &our, &addrlen) == -1) { + perrorf (g, "getsockname"); + return -1; + } + + FILE *fp = fopen ("/proc/net/tcp", "r"); + if (fp == NULL) { + perrorf (g, "/proc/net/tcp"); + return -1; + } + + char line[256]; + if (fgets (line, sizeof line, fp) == NULL) { /* Drop first line. */ + error (g, "unexpected end of file in /proc/net/tcp"); + fclose (fp); + return -1; + } + + while (fgets (line, sizeof line, fp) != NULL) { + unsigned line_our_addr, line_our_port, line_peer_addr, line_peer_port; + int dummy0, dummy1, dummy2, dummy3, dummy4, dummy5, dummy6; + int line_uid; + + if (sscanf (line, "%d:%08X:%04X %08X:%04X %02X %08X:%08X %02X:%08X %08X %d", + &dummy0, + &line_our_addr, &line_our_port, + &line_peer_addr, &line_peer_port, + &dummy1, &dummy2, &dummy3, &dummy4, &dummy5, &dummy6, + &line_uid) == 12) { + /* Note about /proc/net/tcp: local_address and rem_address are + * always in network byte order. However the port part is + * always in host byte order. + * + * The sockname and peername that we got above are in network + * byte order. So we have to byte swap the port but not the + * address part. + */ + if (line_our_addr == our.sin_addr.s_addr && + line_our_port == ntohs (our.sin_port) && + line_peer_addr == peer.sin_addr.s_addr && + line_peer_port == ntohs (peer.sin_port)) { + *rtn = line_uid; + fclose (fp); + return 0; + } + } + } + + error (g, "check_peer_euid: no matching TCP connection found in /proc/net/tcp"); + fclose (fp); + return -1; +} + /* You had to call this function after launch in versions <= 1.0.70, * but it is now a no-op. */ @@ -1931,6 +2119,56 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void **buf_rtn) return 0; } +/* This is very much like recv_from_daemon above, but g->sock is + * a listening socket and we are accepting a new connection on + * that socket instead of reading anything. Returns the newly + * accepted socket. + */ +static int +accept_from_daemon (guestfs_h *g) +{ + fd_set rset, rset2; + + if (g->verbose) + fprintf (stderr, + "accept_from_daemon: %p g->state = %d\n", g, g->state); + + FD_ZERO (&rset); + + FD_SET (g->fd[1], &rset); /* Read qemu stdout for log messages & EOF. */ + FD_SET (g->sock, &rset); /* Read socket for accept. */ + + int max_fd = g->sock > g->fd[1] ? g->sock : g->fd[1]; + int sock = -1; + + while (sock == -1) { + rset2 = rset; + int r = select (max_fd+1, &rset2, NULL, NULL, NULL); + if (r == -1) { + if (errno == EINTR || errno == EAGAIN) + continue; + perrorf (g, "select"); + return -1; + } + + if (FD_ISSET (g->fd[1], &rset2)) { + if (read_log_message_or_eof (g, g->fd[1]) == -1) + return -1; + } + if (FD_ISSET (g->sock, &rset2)) { + sock = accept (g->sock, NULL, NULL); + if (sock == -1) { + if (errno == EINTR || errno == EAGAIN) + continue; + perrorf (g, "accept"); + return -1; + } + } + } + + return sock; +} + int guestfs___send (guestfs_h *g, int proc_nr, xdrproc_t xdrp, char *args) { -- 1.6.2.5

Richard W.M. Jones wrote: ... Similarly here: if you use gnulib's arpa_inet module, then you can drop the #ifdef

Richard W.M. Jones wrote: ACK. I confirmed that this series converts all uses, (including the ones in daemon/, later)