These are mostly places where a field in a struct is only kept around
for lifetime reasons.
Annotating the individual instances means that we don't have to turn off
the warning for new code.
BUG=b:365852007
Change-Id: I9172ea28cc12779331cd48c2c6ad1003d7ca02f8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5966505
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
For ioctl numbers that do not require any parameters, make ioctl_io_nr
and related macros generate a constant rather than a function with no
parameters. This makes the code that uses these constants more idiomatic
and also allows using the constants in match statements (see an example
in virtio/fs/passthrough.rs).
BUG=None
TEST=tools/dev_container tools/presubmit
Change-Id: Id52817528d770c5dbbe2ce7928c9f31a15c83d83
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5648647
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This API directly depends on Minijail, so it does not make sense for it
to be in the base crate that should be cross platform. Move it into the
jail crate, which already wraps other Linux-specific Minijail
functionality.
If we need to add a fork API that works on all unix platforms (including
non-Linux), that could be re-added in base, but for now, this is only
needed for Linux.
This removes the `base` crate's dependency on minijail, as well as the
special case test harness (which is moved to `jail` instead).
Change-Id: Iabefff03ab18222b71b82571bd815d5f028236c8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5556681
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Ensure that every Cargo.toml dependency on a third-party crates.io crate
specifies at least a major version, or a minor version for 0.x crates,
to ensure that if a new major version is published, it cannot cause API
breaks.
The versions are selected to match the ones already in Cargo.lock, so
this should have no functional change, but it will help prevent new "*"
versions from being introduced via copy-and-paste.
For rationale, see the Cargo FAQ:
<https://doc.rust-lang.org/cargo/faq.html#can-libraries-use--as-a-version-for-their-dependencies>
`minijail`, `audio_streams`, and `cras` are left as "*" for now, since
they have unusual situations (imported from a submodule and/or replaced
at build time with ebuild magic).
BUG=None
TEST=tools/dev_container tools/presubmit
TEST=verify Cargo.lock is unchanged
Change-Id: Ifa18199f812f01d2d10bfb4146b3353c1a76527c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5555656
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The tempfile crate is only used in tests, so it does not need to be in
the main [dependencies] section of any of our first-party crates.
Also fix a couple of [dev_dependencies] instances to the officially
documented [dev-dependencies] name for consistency.
BUG=None
TEST=cargo tree
Change-Id: I1dcb6be10c302baec4e8ebe6f72480f76e4e3760
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5521511
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
See prior comment thread:
https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5454793/6..9//COMMIT_MSG#b12
`TimerTrait::reset` offers a very similar API to the `timerfd_settime`
system call, but in practice is almost used to set either a one-shot
timer, or a repeating timer with an initial timeout the same as the
repeating timeout.
`kqueue`'s timer functionality only supports the cases that are actually
used in practice. This change aligns the API with the way it is used,
and makes the distinction between the behavior of the arguments more
obvious.
Test: tools/presubmit
Test: cargo test -p base timer -- --ignored
Bug: 335486579
Change-Id: I1873ab9cccaf7a4b988431839964b1c253f76a34
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5463100
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The previous attempt to fix this was broken in that it only waited for
at least one task in a process to stop. This change guarantees that all
tasks stop.
BUG=b:331740397
TEST=ui.TaskSwitchCUJ with ArcVmmSwapPolicy turned up to 11
Change-Id: If56648c5c061eb8a33828638fd80867a4fb6c4c7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5445906
Commit-Queue: Shin Kawamura <kawasin@google.com>
Auto-Submit: David Stevens <stevensd@chromium.org>
Reviewed-by: Shin Kawamura <kawasin@google.com>
There can be race when a new process is forked while freezing crosvm
processes.
By checking processes before/after freezing, retry freeze all processes
if the race is detected.
BUG=b:324158729
BUG=b:331740397
TEST=tast run $DUT arc.Swap
Change-Id: Iaad8610b0fd708078e806d47611adeddc2fca612
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5437255
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: David Stevens <stevensd@chromium.org>
After pausing a process with SIGSTOP, wait for all tasks (i.e. threads)
in a process to stop before declaring the process quiesced.
This fixes an ARCVM guest kernel crash where the virtio-blk worker
thread would complete a read after guest memory was copied to staging
memory but before guest memory was MADV_REMOVE'ed, leading to erofs
getting all-zero pages to decompress.
BUG=b:331740397
TEST=ui.TaskSwitchCUJ with ArcVmmSwapPolicy turned up to 11
Change-Id: Iaa57516474c9c897c039c9082e65f60b064e741b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5437254
Reviewed-by: Shin Kawamura <kawasin@google.com>
Commit-Queue: David Stevens <stevensd@chromium.org>
Initialize the metrics tube and make sure its kept in all forked
children.
BUG=b:332466813
TEST=tast run DUT arc.Boot.vm
Change-Id: Ide7a9e4ba5a3d8a52e69ac65b9aaaefcf21735a6
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5400363
Commit-Queue: David Stevens <stevensd@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
SafeDescriptor can be converted to a RawDescriptor, so there is no less
flexibility.
Change-Id: Ifdbca63370d56a833473823ea7032ffabda4c521
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5425828
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Several crates still declared a dependency on data_model despite not
using any imports from it.
BUG=b:312312646
TEST=tools/dev_container tools/presubmit
Change-Id: I63a67696c205f684b1e4ac8bdad4de5f294b63dc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5370964
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
100ms timeout on wait_for_state() was too aggressive. It have flaky
timed out some times on system load.
Thins makes the timeout duration 2s.
BUG=none
TEST=cargo test --features=swap/enable -p swap
Change-Id: Ie5d3ef61067d81c643793f6b853b5535eccd2762
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5321374
Auto-Submit: Shin Kawamura <kawasin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
There don't appear to be any new clippy warnings, and this is the
version of Rust that Android is now using: b/303252546. Work for the
next version is tracked in b/310977762.
This doesn't have any particular features I'm looking for (async traits
only come in 1.75.0). I'm not particularly attached to this change. If
it's easy to upgrade the container though, we could ensure no new clippy
warnings appear.
Android rust versions appear here:
https://cs.android.com/android/platform/superproject/main/+/main:prebuilts/rust/linux-x86/
Also not sure what the ChromeOS release schedule is.
Release docs: https://releases.rs/docs/1.73.0/
Test: tools/dev_container --clean bash -c "rustc --version && tools/presubmit clippy"
Change-Id: I2d3010c3032053709f0c0beb7dcbe2b80d782415
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5201659
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
https://crrev.com/c/5273351 fails with original implementation because
a page which is swapped out on the second swap out round is not swapped
in when vmm-swap is disabled, leading to the page unexpectedly being
zeroed.
This bug was introduced by https://crrev.com/c/5040208 which wrongly
handles the idx_range_in_file.start and region_tail_idx_in_file
boundary.
BUG=b:324344124
TEST=https://crrev.com/c/5273351
Change-Id: I31b8156bae5928f70cf85117df18235630b3f29d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5275254
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
If the data iterator hits an error, then we probably shouldn't continue
as if there was no remaining data.
BUG=b:307603892
Change-Id: Iad75974c3801d00acc95201d0686aa5f58cb6ea8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5214579
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Shin Kawamura <kawasin@google.com>
userfaultfd 0.8.1 contains an update on IoctlFlags which now does not
cause errors if kernel output contains unknown flags.
This fix an issue that it fails to enable vmm-swap on kernel 6.6 which
adds UFFDIO_POISON flag to its output.
Corresponding rust crate update is https://crrev.com/c/5196274
BUG=b:315578762
TEST=cros build-packages --board=dedede crosvm
TEST=tast run $DUT arc.Swap
Change-Id: If70863c309afaa93beb4c62bb975aa8c87ce7f5d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5196097
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: David Stevens <stevensd@chromium.org>
There was crashes due to EINVAL from UFFDIO_COPY while swapping in. This
was caused because during shutdown of the main process, guest memory was
being torn down before the swap monitor process was shut down.
GuestMemory consists of `Arc` of a guest memory. Holding the cloned
GuestMemory in SwapController in the main process prevents the guest
memory from being freed.
BUG=b:318790972
TEST=manual
Change-Id: I2f0deee3a51ebf3980d202e0005cd91d736bd91a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5188833
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
UffdError::UffdClosed from the userfaultfd of the main process while
swapping in is because the main process dies without waiting for swap
monitor process terminates. Since it is not a bug of the swap monitor
but other feature on the main process, swap monitor should not panic.
BUG=b:318790972
TEST=manual
Change-Id: I716d666165c28c0b15add918692435eca1817846
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5188832
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
The tube error on reading is because the main process dies without
sending Exit command to the swap monitor. Since it is not a bug of the
swap monitor but other feature on the main process, swap monitor should
not panic.
BUG=b:318790972
TEST=manual
Change-Id: I54863c13bc8c463bd9d4041783f153808e71bc4b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5188021
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Option<(usize, bool)> is 16 bytes per item and FilePageState was 16
bytes.
Since we can assume that guest memory size is not that big, page id can
fit in 31 bits.
The metadata size for the swap file is 1MB per 1GB of maximum guest
memory, plus 1MB per 1GB of swapped data.
BUG=b:312072150
TEST=tast run -buildbundle=crosint $DUT arc.SwapAppLatency
Change-Id: I35937fca7800adc14a18720d2722b738bafe2290
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5048213
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: David Stevens <stevensd@chromium.org>
This commit fixes a bug that causes a significant delay when logging out
of an account. The bug occurs because:
* Simple file formats can have many holes, which results in a large
number of discard commands being issued when the file is deleted.
* crosvm implements a mitigation to slow down file size reduction, but
the crosvm process terminates when logging out, causing the swap file
to be deleted simultaneously.
To fix this issue, we compact the data and store it at the beginning of
the file.
BUG=b:312072150
TEST=tast run -buildbundle=crosint $DUT arc.SwapAppLatency
Change-Id: If29611daa1a1eda49a87fdad13541a23a8dcdcf7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5043882
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Calculate the page index in a file at page handler layer instead of swap
file layer.
This helps to migrate the swap file to be compacted in the next CL.
BUG=b:312072150
TEST=tast run -buildbundle=crosint $DUT arc.SwapAppLatency
Change-Id: If895f943e2bfe8072e46c3172974ea3edb0f7853
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5040208
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: David Stevens <stevensd@chromium.org>
IoBuf is defined in a platform-specific way (it is either iovec on unix
or WSABUF for windows), so it fits into the mission statement of the
base crate, which is meant to be *the* platform abstraction layer in
crosvm.
IoBufMut and VolatileMemory/VolatileSlice are defined in terms of IoBuf,
so they are also moved into base for consistency. Every crate that uses
these types already depended on base, so no extra dependencies are
added, and a few can be removed.
BUG=b:312312646
TEST=tools/dev_container tools/presubmit
Change-Id: I4dddc55d46906dfc55b88e8e6a967d7e1c1922dd
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5046605
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Also adds lld, which will be used in a follow-up to improve
linking performance.
Since debian has had a stable release since then, we also
need to upgrade a few library versions in the testvm.
Swap tests that require userfaultfd were able to run without
root before. In the latest version of debian we need to configure
for unprivileged access.
Since this is the case for glinux workstations as well, let's
mark those tests as root-only and let them execute via sudo.
BUG=b:304875018,b:256905223
TEST=presubmit
Change-Id: Ied0b34e248990378a7864b4a394aa21d8f824f71
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5013631
Reviewed-by: Zihan Chen <zihanchen@google.com>
Reviewed-by: Shin Kawamura <kawasin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Move the Linux Pid type to unix (it should work for any unix-like
environment) and redefine the Windows Pid as DWORD to match
GetProcessId() and similar APIs (rather than declaring a pid_t type on
Windows).
BUG=b:309848917
BUG=b:309020556
TEST=tools/dev_container tools/presubmit
TEST=cargo build --target=aarch64-apple-darwin -p base
Change-Id: Iaa1cddb84eb16b94fe35c02f2f4988b8d955318a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5014787
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Cody Schuffelen <schuffelen@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Even main process shutdown by an error, the vmm-swap monitor process
should be terminated by Command::Exit via the command tube. Dropping
SwapController without explicit SwapController::exit() has caused panic
by tube disconnection in the monitor process when the main process shuts
down by an error.
SwapController will never be dropped in child processes because
ProxyDevice and fork_process() exit with libc::exit() and not drop
objects in the upper stack.
BUG=b:309463106
TEST=manual test
Change-Id: I35386dd1aefe15dafd3e9981a9432c00f8faa3fa
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5009772
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Every platform was re-exporting the non-platform-specific symbols
defined in descriptor_reflection.rs.
Change-Id: I95d0acdaaf8c157206ca788e4cdf64f15ebd2f84
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5001574
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
monitor_process() returns anyhow::Result, so print it with the alternate
format specifier to get the full chain of context messages in the panic
handler if it fails.
BUG=b:309463106
TEST=cargo build --features=swap
Change-Id: Id50b35dad980fc6e1d3c02ba4a85a598cc34ba8c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5006707
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Shin Kawamura <kawasin@google.com>
Remove pointer casts where the target type is the same as the original.
Change-Id: Ibb7bda2e4afcb5df2cd7c0bd9a075d0288b69f67
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5005512
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
base::Pid is explicitly added since every platform has a process id
type.
Bug: 233233301
Test: tools/dev_container ./tools/presubmit
Change-Id: Id0d54fe06981c1dcbdee3f73b1241af74a26300f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4909061
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Updates are made to source and documentation.
This more accurately represents the currently supported platforms of
Android/Linux and Windows, without unexpectedly including other
unix-like operating systems.
Command to reproduce:
$ find . -type f -not -path '*/\.git/*' | xargs -I {} sed -i 's/cfg(unix)/cfg(any(target_os = "android", target_os = "linux"))/g' {}
$ cargo fmt
md files manually updated to fix line lengths.
Renaming `unix` modules to `linux` will be done in a later CL.
Test: ./tools/dev_container ./tools/presubmit
Bug: b/298269162
Change-Id: I42c1bf0abf80b9a0df25551613910293217c7295
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4909059
Commit-Queue: Cody Schuffelen <schuffelen@google.com>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
As usual, some unsorted and grouped imports have appeared.
Change-Id: I79b51e4c52cee38f5b8c238e46dfe3193c753554
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4847980
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This CL change the message when failed to get THP size from warning
to info, due to it will always use the fallback value on the THP
disabled host environment.
BUG=b:296176956
TEST=CQ
Change-Id: I751d58d12b2ab571e0362221eb35f5d37adf916f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4816246
Reviewed-by: Shin Kawamura <kawasin@google.com>
Commit-Queue: Shengsong Tan <sstan@chromium.org>
Reviewed-by: David Stevens <stevensd@chromium.org>
resident page count is independent from page handler. Showing resident
pages in the guest memory while vmm-swap is disabled is useful for
tracking vmm-swap usage.
This also include typo fix regident -> resident.
BUG=b:291193201
TEST=manually tested
Change-Id: Ib1d0fbd3958705446c73481abf6261bbffe134a0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4705404
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Existing compute_resident_pages() has been invalid since it does not
take removed pages by MADV_REMOVE by virtio-balloon into account.
The overhead from lseek(2) syscall is acceptable because crosvm swap
status command is not called frequently and it only takes less than 100
milliseconds.
Removed `Region::swap_active` as well since it is no longer used.
Alternatives:
* Reduce the counter on `UFFD_EVENT_REMOVE`
* This is incorrect because the pages designated by the address range
of `UFFD_EVENT_REMOVE` may have been empty originally.
* Track the active pages in the guest memory using a list/map data
structure.
* We can reduce the syscall overhead from `lseek(2)`. However the data
structure only used for the resident memory counting is a waste of
memory.
BUG=b:291193201
TEST=manually tested
Change-Id: I7254c93a73b777e16d8863a67cf5639b4efb4aad
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4686451
Reviewed-by: David Stevens <stevensd@chromium.org>
Commit-Queue: Shin Kawamura <kawasin@google.com>
Truncating a large file can result in a significant amount of IO for
updating the filesystem metadata. When swap is disabled because an app
is being launched, this IO ends up competing with the IO needed to
launch the app, resulting in significant latency.
When disabling swap because of an app launch, cleaning up the swap file
is not high priority. However, when disabling swap because of low disk
space, the swap file needs to be cleaned up immediately. As such, make
the slow cleanup of the swap file an extra parameter to disabling swap.
BUG=b:288989607
TEST=tast run -buildbundle=crosint DUT arc.SwapAppLatency
TEST=cargo test -p swap --features enable
Change-Id: Icab2a55cdb9350cbb9e380583a31b546bd8e23cf
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4653111
Reviewed-by: Junichi Uekawa <uekawa@chromium.org>
Commit-Queue: David Stevens <stevensd@chromium.org>
Reviewed-by: Shin Kawamura <kawasin@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reduce logging verbosity of swap status messages. Whatever made the
status request can log the message if it is important.
BUG=None
TEST=CQ
Change-Id: I88a7293ec156fe1d51d7115cfb08802f4f03b02b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4630876
Reviewed-by: Shin Kawamura <kawasin@google.com>
Commit-Queue: David Stevens <stevensd@chromium.org>
