open_file does not always open_file. It reuses given fd when it points
to a special path /proc/self/fd/XX.
BUG=b:291832160
TEST=build
Change-Id: If821834e3adbc457616c71e5b8425962286245e7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4706830
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Junichi Uekawa <uekawa@chromium.org>
This is mainly for qcow2, which doesn't immediately write all mutations
to file. It is important that all writes be sent to the OS so that
snapshots taken while asleep do not lose information.
BUG=b:266514327
TEST=patched into AOSP and manually ran snapshot-restore flow
Change-Id: I2a9f24120d51fd053ff1440d0e1750e9bbebf379
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4679843
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Elie Kheirallah <khei@google.com>
MemRegionIter allows iteration over a &[MemRegion] list, with additional
helper functions to skip bytes and truncate the list.
In particular, this is useful for the virtio DescriptorChain utilities
(Reader/Writer) that previously needed to call collect() to allocate a
new copy of the regions list when it was necessary to limit the length
or skip part of the beginning of the chain.
This also replaces the functionality of MemRegion::truncate(), which
used Cow<[MemRegion]>, meaning it would sometimes need to allocate and
copy. The new MemRegionIter implementation never needs to copy the list
of regions; it just maintains a couple of internal counters and adjusts
the returned MemRegions on the fly.
BUG=None
TEST=tools/dev_container tools/presubmit
TEST=cargo test -p cros_async
Change-Id: Ie66c7a8c4061a166c986c6d6937e21549ddb205e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4559367
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Currently, virtio-blk translates a FLUSH request to fsync system call.
However, fsync always updates the metadata of a file, which causes extra
journaling for metadata update. Since virtio-blk cares only the data of
the given block image file, we should avoid updating extra metadata.
After this change, virtio-blk now calls fdatasync for a FLUSH request.
If a write operation wrote to a data block that is already allocated
before, fdatasync can avoid triggering jdb2 journaling on an ext4 file
system.
Note there are structs which fall back to fsync for fdatasync. In
addition, io_uring executor does not implement fdatasync yet.
We observed statistically significant play_store_shown_time improvement
on trogdor-arc-r by -1.614% with this change. Including non
statistically significant data, we observed the following improvements.
| Name | delta | Count |
|-----------------------|---------|-------|
| trogdor-arc-r (lazor) | -1.614% | 170 |
| brya-arc-t (primus) | -1.478% | 200 |
| octopus-arc-t (apel) | -0.887% | 190 |
| kukui-arc-r (kakadu) | -0.451% | 170 |
BUG=b:281609112
TEST=run a vm with block and confirmed fdatasync is called by strace
Change-Id: Idc94a3ec169e9a5e04394079967f6d79ff4c32db
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4523029
Commit-Queue: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
system_api bindings have been regenerated with protobuf 3.2; this should
be okay to land before the full ChromeOS system_api migration, since
crosvm always uses its own copy of the bindings rather than the ones
provided by the dev-rust/system_api package.
The protoc-rust crate is replaced with protobuf_codegen in 3.x.
BUG=b:277243607
BUG=b:279834784
TEST=tools/dev_container tools/presubmit
Change-Id: I6aad45ded2639d7506a7238800584bebab196455
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4405309
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Replace seek(SeekFrom::Current(0)) with the stream_position() helper
function, which has been stable since Rust 1.51.
BUG=b:276487055
TEST=tools/clippy # with rust 1.68
Change-Id: I21e9af65f6ad5ad8443dd1d2b95c08d3be91f174
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4390741
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Uprev to version available in Debian.
This seem to end up doing uprev to 1.3 now.
BUG=b:265082456
BUG=b:229895468
TEST=build
Change-Id: I550778acb675c9034b9cfcea77f4ae847e2d2ea1
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4364559
Commit-Queue: Junichi Uekawa <uekawa@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Junichi Uekawa <uekawa@chromium.org>
Reviewed-by: Frederick Mayle <fmayle@google.com>
This removes two layers of boxing and trait objects. One for the IO
source objects and another for the futures generated by calling methods
on them.
Benefits:
* IO futures will now be `Send` (if the underlying source is `Send`,
which is generally the case).
* It is easier to work with a concrete type than a `Box<dyn ...>`.
* More efficient (less boxing and virtual function calls).
Slightly smaller binary size:
$ cargo build --all-features --profile chromeos && ls -l target/chromeos/crosvm
before: 10,811,576
after: 10,692,792 (1.1% smaller)
Change-Id: I7b52cd24945cfb4ff0a6c377739b5d4ab9cacb0b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004980
Reviewed-by: Noah Gold <nkgold@google.com>
Introduce `try_clone` method to `DiskFile` trait. This allows callers to
create DiskFile instances which share the same underlying file
descriptors, which enables you to manipulate them in multiple threads in
parallel. virtio-blk requires this change to run multiple worker threads
in parallel.
Note `try_clone` is introduced as a method of `DiskFile`, not as a
standalone `TryClone`, since such `TryClone` trait is not an object-safe
and thus cannot be defined actually.
BUG=b:267716786
TEST=build passes
Change-Id: I8aae4ee08992de2649b7157b8dfe7328751208b5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4281742
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Takaya Saeki <takayas@chromium.org>
PunchHole should be originally immutable method since the
file_punch_hole() for both unix and windows are immutable function which
do not change the underlining file offset.
QcowFile is the only object which requires mutability to punch a hole to
its file.
This commit introduce a new PunchHoleMut and the existing PunchHole
traits and make PunchHole trait immutable. This unblocks b/269981962
which tries to punch a hole to non-mut reference File.
This also remove useless top level defined functions in base crate.
BUG=b:269981962
TEST=cargo build --feature=qcow
TEST=./tools/run_test2
Change-Id: I8333d13f4adc6dff319c0ddababe400d5e995141
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4269718
Reviewed-by: David Stevens <stevensd@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Commit-Queue: Shin Kawamura <kawasin@google.com>
This CL removed many uses of DataInit in devices. Some paddings
are manually added/fixed to allow AsBytes to derive without ABI
changes.
TESTED=CQ
BUG=b:204409584
Change-Id: I1f8c2d5304fc8e685cc3e5166c73481f6a3f78f7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4235224
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Auto-Submit: Zihan Chen <zihanchen@google.com>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
If a guest cycled the virtio-blk on and off (e.g. activate, reset,
activate), then android-sparse files would start to be treated like raw
files.
BUG=b:266440668,b:219595052
Change-Id: I9ca00668ba7a7c820a177309edf320426feba81a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4195519
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Auto-Submit: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
BUG=b:219595052
TEST=cd disk && cargo test --features qcow,composite-disk,android-sparse
Change-Id: Id7c49ce3f598260a3b0bd6331249354777f4aa16
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4174617
Auto-Submit: Frederick Mayle <fmayle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
BUG=chromium:901139,b:219595052
TEST=cargo test
TEST=Start VM with rootfs of android sparse disk with sandboxes on v5.7 kernel
Change-Id: If829bffd24f23c06242124ccc8bfafb19b1ff9ed
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4160473
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
`get_len()` returns `AndroidSparse::total_size`, which is set to `size`,
so the check simplified to `size != size` => `false`.
Change-Id: I97ecd7923bce00fdc88e5dc49079ce3b5ee4d44b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4160754
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The file format and canonical parser spec assert this is the case.
It doesn't simplify the code much, but it means there are fewer cases to
test and reasonable about + it's slightly more effecient.
Change-Id: Iacd64cfa439bf38546735869c1b5a815b0c5bb35
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4165863
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
The implementation didn't correctly handle the case where the file's
header size was larger than the ChunkHeader struct. It isn't a problem
in practice, so, instead of fixing it, we are turning this case into an
error.
Change-Id: Ie23a934523f5201c99352159aaa0fdf0fed7dd45
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4160753
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
There is a lot of code and test duplication w.r.t. the non-async impl
but we expect to delete the non-async impl in the near future, so the
temporary duplication is preferable vs increased code complexity.
Bug: 219595052
Change-Id: I402166331606d41178c1557c66be591d95c98c45
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158596
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
`str::len` is a const function now, so it can be used directly.
Change-Id: I2ec0de09adc71df9d2664e319a57f6a376f865c9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4158592
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Frederick Mayle <fmayle@google.com>
These should be written as ChromeOS and ChromiumOS (without the space)
to match the updated branding. The copyright headers were already
migrated to the new style (https://crrev.com/c/3894243), but there were
some more instances left over.
BUG=None
TEST=tools/cargo-doc
Change-Id: I8c76aea2eb33b2e370ab71ee9b5cc0a4cfd00585
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4129934
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
The test would take ~5s to run, cutting down the number of blocks
to write/read in the test will improve execution speed to .5s.
BUG=None
TEST=tools/run_tests -v disk\*
Change-Id: Ia877335bc662f4e7c5c91eff56c072bb1eb30e8d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4004300
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Auto-Submit: Dennis Kempin <denniskempin@google.com>
The patch makes usb feature no-op on windows as USB pass-through is
not supported on windows.
BUG=b:241251677
BUG=b:213149155
TEST=tools/presubmit
Change-Id: Id82d4c732a86e782695d2af698cc08e8e3fd2d35
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006819
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Vikram Auradkar <auradkar@google.com>
Move the logic for locking and DiskFile creation into an implementation
of DiskOption::open(), matching the sys::unix structure. This will
allow future consolidation of the vhost-user devices.
For now, the Windows variant of DiskOption::open() only knows how to
create a SingleFileDisk (not qcow, composite, etc.); switching to
create_disk_file() is left as future work.
This also makes a few tweaks to the creation of the BlockAsync instance
in order to make the unix and windows versions more similar - compare:
- devices/src/virtio/vhost/user/device/block/sys/windows.rs
- devices/src/virtio/vhost/user/device/block/sys/unix.rs
No functional change intended.
BUG=None
TEST=tools/presubmit --all
Change-Id: Iadb1a8913a7445d7c091d0e359c7d8792704c35a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3892136
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
This search/replace updates all copyright notices to drop the
"All rights reserved", Use "ChromiumOS" instead of "Chromium OS"
and drops the trailing dots.
This fulfills the request from legal and unifies our notices.
./tools/health-check has been updated to only accept this style.
BUG=b:246579983
TEST=./tools/health-check
Change-Id: I87a80701dc651f1baf4820e5cc42469d7c5f5bf7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3894243
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
This is a follow-up to
https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3864926,
which implements this for QCOW.
Same rationale: allow to turn off support for disk formats that projects
do not need/want.
BUG=None
TEST=./tools/presubmit; cd crosvm-fuzz && cargo build
Change-Id: I401bad4d4ccb1a00a303ed86f1156f153b70b562
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3872278
Commit-Queue: Christian Blichmann <cblichmann@google.com>
Reviewed-by: David Stevens <stevensd@chromium.org>
Tested-by: Christian Blichmann <cblichmann@google.com>
With AsyncDiskFileWrapper, we can use all disk image formats via the
async API.
The create_async_disk_file() function is removed, and the existing
create_disk_file() function can be used to create async-capable disk
files, as the DiskFile trait now includes ToAsyncDisk.
BUG=b:219595052
TEST=cargo build --features=composite-disk
TEST=Boot x86-64 Linux in crosvm with a qcow2 disk attached
Change-Id: I430d306fe71c325ab529a7031353d20f8a9aa2f3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3824067
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This change allows to turn off support for QCOW. This is useful for
projects that do not need/want it.
A follow-up change (if this one is merged) will do the same for Android
sparse disk images.
BUG=none
TEST=./tools/presubmit
Change-Id: I69083c4c2e21d504db89eff47f4c86c6f61d67e9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3864926
Tested-by: Christian Blichmann <cblichmann@google.com>
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Christian Blichmann <cblichmann@google.com>
As a stepping stone toward fully asynchronous disk formats, add a
wrapper that takes a regular synchronous DiskFile and provides the
async disk API.
BUG=b:219595052
TEST=cargo build --features=composite-disk
Change-Id: Id785ad7ff69ae869a8a9007134fdb4fe7f3be4ba
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3824066
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Rather than tying AsyncDisk::read_to_mem/write_from_mem to the
GuestMemory type specifically, use dyn BackingMemory like the underlying
read_to_mem/write_from_mem calls in cros_async. This will allow an async
version of QcowFile to use these functions to read/write temporary
buffers as part of its internal bookkeeping.
BUG=b:219595052
TEST=tools/presubmit
Change-Id: I4ca3c976bf5dca68e3bbe0e3f163023b47034254
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3824065
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
This would complicate the implementation of ToAsyncDisk for qcow and
other formats. Change the trait bounds so that only the minimal set of
traits used in the asynchronous block device is required.
BUG=b:219595052
TEST=cargo build
Change-Id: Id5ece59596b0a0989f953854a9cf4c30d32f7e34
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3824064
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
- Remove trailing ::{self} on all use statements
- Remove any resulting single-level use statements (e.g. use libc;)
- Reformat with `tools/fmt --nightly`
BUG=b:239937122
TEST=tools/dev_container tools/presubmit --all
Change-Id: I8afd1b0458ca6d08d9b41a24583f7d4148597ccb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3798973
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
crosvm is switching the import style to use one import per line.
While more verbose, this will greatly reduce the occurence of merge
conflicts going forward.
Note: This is using a nightly feature of rustfmt. So it's a one-off
re-format only. We are considering adding a nightly toolchain to
enable the feature permanently.
BUG=b:239937122
TEST=CQ
Change-Id: Id2dd4dbdc0adfc4f8f3dd1d09da1daafa2a39992
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3784345
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
This CL avoids fsyncing when unncessary (optimization originally created for
Windows).
TEST=builds
BUG=b:213150316
Change-Id: I9dc7d2d9c394ac83e4220e2a7ae99d670f4a5630
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3654618
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
Currently we treat any version 1 path as absolute, and any non version 1
path as relative. The proto files don't indicate that this assumption
can be made, and we don't make it on Windows. This CL changes path
handling so that a version 1 path must actually be absolute for us to
treat it as such.
TEST=builds
BUG=b:213150316
Change-Id: I95a1c8751428a9e2f7311f9136940fbdb2410ef1
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3654207
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
This provides at least a minimal one-line description to show what each
crate is about.
BUG=None
TEST=tools/cargo-doc
Change-Id: I26732e8c29062e622d5be09bdc120a49d564b9fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3630422
Reviewed-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
This removes the last usage of the non-At WriteZeroes trait.
BUG=None
TEST=cargo test -p disk
Change-Id: I54e9990140afdebccfd1b97dd2b6e75b17f3135a
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3626022
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Noah Gold <nkgold@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
9da4030ab0 migrated consumers to use
AllocateMode, but this method got the wrong mode. This CL fixes that.
Thanks to idanr@ for spotting this during downstream merging.
BUG=NONE
TEST=bots + inspection.
Change-Id: I10792d4f293d7f672d3474288a7f706a5c0fd835
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3597038
Auto-Submit: Noah Gold <nkgold@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
fallocate modes were previously unix specific constants, which won't work after
we add support for Winodws. Here, we've put a generic abstraction in place so
that each platform can implement the details as appropriate.
Thanks to acourbot@ for suggesting the splits in this series.
BUG=b:213147081
TEST=see final CL in series.
Change-Id: I822ad6c4a26eea716482029e8a6c0489aa72c595
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3583613
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
This cleans up the API and makes it easier to tell what is happening at
the call site. Additionally, callers may now use any options supported
by OpenOptions rather than the two specific flags (read_only and
o_direct) that were previously supported.
BUG=None
TEST=tools/presubmit
Change-Id: Ib8b5350c60807f14ebe0816d71bbf31e4bfef67f
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3553763
Reviewed-by: Noah Gold <nkgold@google.com>
Reviewed-by: Vikram Auradkar <auradkar@google.com>
Reviewed-by: Anton Romanov <romanton@google.com>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
