vm_tools: sommelier: WaylandChannel early-init with fallback

While migrating from VirtWaylandChannel to VirtGpuChannel
(cross-domain), sommelier should attempt to fallback to
VirtWaylandChannel in case the host VMM doesn't provide a Virtio-Gpu
cross-domain context-type implementation.

This way, VMs may opt-in to VirtGpuChannel when it is possible, and
continue to operate normally on VirtWaylandChannel when it is not.

Also refactors the event loop initialization flow that depends on the
wayland channel.

BUG=b:314788008
TEST=`sommelier --virtgpu-channel ...` falls back to VirtWaylandChannel
when Virtio-Gpu doesn't advertise its "cross-domain" context-type.
TEST=USE=fuzzer cros_sdk emerge-tatl sommelier
TEST=cros_sdk cros_workon_make --verbose --board tatl --test sommelier

Change-Id: I925bef4af41dc5fa76a6816191b15b34b27384f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/5523855
Reviewed-by: Chloe Pelling <[email protected]>
Commit-Queue: Ryan Neph <[email protected]>
Tested-by: Ryan Neph <[email protected]>

sommelier-ctx.cc

@@ -387,46 +387,60 @@ static int sl_handle_virtwl_socket_event(int fd, uint32_t mask, void* data) {
   return 1;
 }
 
-bool sl_context_init_wayland_channel(struct sl_context* ctx,
-                                     struct wl_event_loop* event_loop,
-                                     bool display) {
-  if (ctx->channel == nullptr) {
-    // Running in noop mode, without virtualization.
-    return true;
+wl_event_loop* sl_context_configure_event_loop(sl_context* ctx,
+                                               WaylandChannel* channel,
+                                               bool use_virtual_context) {
+  assert(ctx);
+  // caller must provide a wayland channel to use virtual context
+  assert(!use_virtual_context || channel);
+
+  wl_display* host_display = wl_display_create();
+  if (!host_display) {
+    LOG(ERROR) << "failed to create host wayland display.";
+    return nullptr;
   }
-  int rv = ctx->channel->init();
-  if (rv) {
-    LOG(ERROR) << "could not initialize wayland channel: " << strerror(-rv);
-    return false;
-  }
-  if (!display) {
-    // We use a wayland virtual context unless display was explicitly specified.
+  // libwayland ensures event_loop can be retrieved from a valid display, but
+  // assert such in case that assumption ever changes.
+  wl_event_loop* event_loop = wl_display_get_event_loop(host_display);
+  assert(event_loop);
+
+  int wayland_channel_fd = -1;
+  if (channel && use_virtual_context) {
     // WARNING: It's critical that we never call wl_display_roundtrip
     // as we're not spawning a new thread to handle forwarding. Calling
     // wl_display_roundtrip will cause a deadlock.
-    int vws[2];
+    int ret;
+
+    ret = channel->create_context(wayland_channel_fd);
+    if (ret) {
+      LOG(ERROR) << "failed to create virtwl context: " << strerror(-ret);
+      return nullptr;
+    }
 
     // Connection to virtwl channel.
-    rv = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, vws);
-    errno_assert(!rv);
+    int vws[2];
+    ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, vws);
+    if (ret) {
+      LOG(ERROR) << "failed to create wayland channel socket pair: "
+                 << strerror(errno);
+      return nullptr;
+    }
 
     ctx->virtwl_socket_fd = vws[0];
     ctx->virtwl_display_fd = vws[1];
 
-    rv = ctx->channel->create_context(ctx->wayland_channel_fd);
-    if (rv) {
-      LOG(ERROR) << "failed to create virtwl context: " << strerror(-rv);
-      return false;
-    }
-
     ctx->virtwl_socket_event_source.reset(wl_event_loop_add_fd(
         event_loop, ctx->virtwl_socket_fd, WL_EVENT_READABLE,
         sl_handle_virtwl_socket_event, ctx));
-    ctx->wayland_channel_event_source.reset(wl_event_loop_add_fd(
-        event_loop, ctx->wayland_channel_fd, WL_EVENT_READABLE,
-        sl_handle_wayland_channel_event, ctx));
+    ctx->wayland_channel_event_source.reset(
+        wl_event_loop_add_fd(event_loop, wayland_channel_fd, WL_EVENT_READABLE,
+                             sl_handle_wayland_channel_event, ctx));
   }
-  return true;
+
+  ctx->host_display = host_display;
+  ctx->channel = channel;
+  ctx->wayland_channel_fd = wayland_channel_fd;
+  return event_loop;
 }
 
 sl_window* sl_context_lookup_window_for_surface(struct sl_context* ctx,

sommelier-ctx.h

@@ -222,7 +222,6 @@ struct sl_context {
   // Command-line configurable options.
   bool trace_system;
   bool use_explicit_fence;
-  bool use_virtgpu_channel;
   bool use_direct_scale;
   bool viewport_resize;
   bool allow_xwayland_emulate_screen_pos_size;
@@ -252,9 +251,9 @@ const char* sl_context_atom_name(int atom_enum);
 
 void sl_context_init_default(struct sl_context* ctx);
 
-bool sl_context_init_wayland_channel(struct sl_context* ctx,
-                                     struct wl_event_loop* event_loop,
-                                     bool display);
+wl_event_loop* sl_context_configure_event_loop(sl_context* ctx,
+                                               WaylandChannel* channel,
+                                               bool use_virtual_context);
 
 sl_window* sl_context_lookup_window_for_surface(struct sl_context* ctx,
                                                 wl_resource* resource);

sommelier-wayland-fuzzer.cc

@@ -152,6 +152,7 @@ int count_fds() {
 }
 
 int LLVMFuzzerTestOneInput_real(const uint8_t* data, size_t size) {
+  int ret;
   static Environment env;
   FuzzedDataProvider source(data, size);
 
@@ -166,11 +167,13 @@ int LLVMFuzzerTestOneInput_real(const uint8_t* data, size_t size) {
   // goes to sommelier to listen on/write to, the other end is kept by us. The
   // channel implements send with a no-op, so we don't ever have to read from
   // our end.
-  ctx.host_display = wl_display_create();
+  ret = channel.init();
+  assert(!ret);
+
   struct wl_event_loop* event_loop =
-      wl_display_get_event_loop(ctx.host_display);
-  ctx.channel = &channel;
-  sl_context_init_wayland_channel(&ctx, event_loop, /*display=*/false);
+      sl_context_configure_event_loop(&ctx, &channel,
+                                      /*use_virtual_context=*/true);
+
   // `display` takes ownership of `virtwl_display_fd`
   ctx.display = wl_display_connect_to_fd(ctx.virtwl_display_fd);
 
@@ -179,7 +182,7 @@ int LLVMFuzzerTestOneInput_real(const uint8_t* data, size_t size) {
   // fuzzer. We set up the event loop to drain any data send by sommelier to our
   // end, and write fuzz data to our end in the main loop.
   int sv[2];
-  int ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sv);
+  ret = socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sv);
   assert(!ret);
   // wl_client takes ownership of its file descriptor
   ctx.client = wl_client_create(ctx.host_display, sv[0]);

sommelier.cc

@@ -3636,7 +3636,9 @@ static void sl_print_usage() {
       "  --stats-timer=SECS\t\tNumber of seconds between stats dumps\n"
       "  --fullscreen-mode=MODE\tDefault fullscreen behavior (immersive,"
       " plain)\n"
-      "  --noop-driver\t\tPass through to existing Wayland server"
+      "  --virtgpu-channel\t\tUse virtgpu cross-domain context as virtual"
+      " wayland channel\n"
+      "  --noop-driver\t\t\tPass through to existing Wayland server"
       " without virtualization\n");
 }
 
@@ -4004,6 +4006,40 @@ void create_shims() {
 #endif
 }
 
+static WaylandChannel* try_wayland_channel_init(
+    WaylandChannel* channel, const char* channel_description) {
+  if (channel) {
+    int ret = channel->init();
+    if (ret) {
+      LOG(VERBOSE) << "WaylandChannel init for candidate '"
+                   << channel_description << "' failed: " << strerror(-ret);
+      delete channel;
+      channel = nullptr;
+    } else {
+      LOG(VERBOSE) << "WaylandChannel init for candidate '"
+                   << channel_description << "' succeeded.";
+    }
+  }
+  return channel;
+}
+
+WaylandChannel* sl_create_wayland_channel(bool try_virtgpu_channel) {
+  // defines a preferred channel ordering and attempt early init so we can
+  // fallback if necessary.
+  WaylandChannel* channel = nullptr;
+  if (try_virtgpu_channel) {
+    channel = try_wayland_channel_init(new VirtGpuChannel(), "VirtGpuChannel");
+  }
+  if (!channel) {
+    channel = try_wayland_channel_init(new VirtWaylandChannel(),
+                                       "VirtWaylandChannel");
+  }
+  if (!channel) {
+    LOG(ERROR) << "WaylandChannel init failed for all candidate backends";
+  }
+  return channel;
+}
+
 int real_main(int argc, char** argv) {
   struct sl_context ctx;
   sl_context_init_default(&ctx);
@@ -4035,7 +4071,6 @@ int real_main(int argc, char** argv) {
 #endif
   const char* socket_name = "wayland-0";
   bool noop_driver = false;
-  struct wl_event_loop* event_loop;
   struct wl_listener client_destroy_listener = {};
   client_destroy_listener.notify = sl_client_destroy_notify;
   int sv[2];
@@ -4046,6 +4081,7 @@ int real_main(int argc, char** argv) {
   int i;
   const char* stats_summary = nullptr;
   const char* stats_log = nullptr;
+  bool use_virtgpu_channel = false;
 
   // Ignore SIGUSR1 (used for trace dumping) in all child processes.
   signal(SIGUSR1, SIG_IGN);
@@ -4159,7 +4195,7 @@ int real_main(int argc, char** argv) {
     } else if (strstr(arg, "--enable-x11-move-windows") == arg) {
       ctx.enable_x11_move_windows = true;
     } else if (strstr(arg, "--virtgpu-channel") == arg) {
-      ctx.use_virtgpu_channel = true;
+      use_virtgpu_channel = true;
     } else if (strstr(arg, "--noop-driver") == arg) {
       noop_driver = true;
     } else if (strstr(arg, "--stable-scaling") == arg) {
@@ -4290,21 +4326,29 @@ int real_main(int argc, char** argv) {
   // Handle broken pipes without signals that kill the entire process.
   signal(SIGPIPE, SIG_IGN);
 
-  ctx.host_display = wl_display_create();
-  assert(ctx.host_display);
-
-  if (noop_driver) {
-    ctx.channel = nullptr;
-  } else if (ctx.use_virtgpu_channel) {
-    ctx.channel = new VirtGpuChannel();
-  } else {
-    ctx.channel = new VirtWaylandChannel();
-  }
-
+  wl_event_loop* event_loop = nullptr;
   {
-    ScopeTimer timer("init wayland channel");
-    event_loop = wl_display_get_event_loop(ctx.host_display);
-    if (!sl_context_init_wayland_channel(&ctx, event_loop, display)) {
+    ScopeTimer timer("configure event loop");
+    // We use a wayland virtual context unless display name was explicitly
+    // specified.
+    const bool use_virtual_context = !display;
+
+    WaylandChannel* wayland_channel = nullptr;
+    if (!noop_driver) {
+      wayland_channel = sl_create_wayland_channel(use_virtgpu_channel);
+      if (!wayland_channel) {
+        LOG(FATAL) << "failed to initialize wayland channel.";
+        return EXIT_FAILURE;
+      }
+    } else if (use_virtual_context) {
+      LOG(FATAL) << "Must either provide a wayland display (--display=<name>) "
+                    "or use a virtual wayland channel (remove --noop-driver).";
+      return EXIT_FAILURE;
+    }
+
+    event_loop = sl_context_configure_event_loop(&ctx, wayland_channel,
+                                                 use_virtual_context);
+    if (!event_loop) {
       return EXIT_FAILURE;
     }
   }

testing/wayland-test-base.h

@@ -180,14 +180,15 @@ class WaylandTestBase : public ::testing::Test {
     ON_CALL(mock_wayland_channel_, create_context(_)).WillByDefault(Return(0));
     ON_CALL(mock_wayland_channel_, max_send_size())
         .WillByDefault(Return(DEFAULT_BUFFER_SIZE));
-    EXPECT_CALL(mock_wayland_channel_, init).Times(1);
+
     sl_context_init_default(&ctx);
-    ctx.host_display = wl_display_create();
-    assert(ctx.host_display);
 
-    ctx.channel = &mock_wayland_channel_;
-    EXPECT_TRUE(sl_context_init_wayland_channel(
-        &ctx, wl_display_get_event_loop(ctx.host_display), false));
+    int ret = mock_wayland_channel_.init();
+    EXPECT_EQ(ret, 0);
+
+    event_loop = sl_context_configure_event_loop(&ctx, &mock_wayland_channel_,
+                                                 /*use_virtual_context=*/true);
+    EXPECT_NE(event_loop, nullptr);
 
     InitContext();
     Connect();
@@ -213,7 +214,7 @@ class WaylandTestBase : public ::testing::Test {
   // init messages not relevant to your test case.
   void Pump() {
     wl_display_flush(ctx.display);
-    wl_event_loop_dispatch(wl_display_get_event_loop(ctx.host_display), 0);
+    wl_event_loop_dispatch(event_loop, 0);
   }
 
  protected:
@@ -323,6 +324,7 @@ class WaylandTestBase : public ::testing::Test {
 
   NiceMock<MockWaylandChannel> mock_wayland_channel_;
   sl_context ctx;
+  wl_event_loop* event_loop;
 
   // IDs allocated by the server are in the range [0xff000000, 0xffffffff].
   uint32_t next_server_id = 0xff000000;