[package-directory] RootDir::new error variant for missing meta.far
RootDir clients may want to know if the error was caused by the
meta.far not being present in storage (e.g. pkg-cache activating cache
packages, in which case an absent cache package is not an error), and
without this new variant it is difficult for clients to know which
variant corresponds to a missing meta.far (and we weren't guaranteeing
it wouldn't change).
Change-Id: Ibbf25318a74b9e7297fd0684f6459f1830475f56
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/697125
Reviewed-by: Sen Jiang <senj@google.com>
Fuchsia-Auto-Submit: Ben Keller <galbanum@google.com>
Commit-Queue: Ben Keller <galbanum@google.com>
diff --git a/src/sys/pkg/lib/package-directory/src/lib.rs b/src/sys/pkg/lib/package-directory/src/lib.rs
index 3d2a7e7..0d4607b 100644
--- a/src/sys/pkg/lib/package-directory/src/lib.rs
+++ b/src/sys/pkg/lib/package-directory/src/lib.rs
@@ -25,6 +25,9 @@
#[derive(thiserror::Error, Debug)]
pub enum Error {
+ #[error("the meta.far was not found")]
+ MissingMetaFar,
+
#[error("while opening the meta.far")]
OpenMetaFar(#[source] fuchsia_fs::node::OpenError),
@@ -43,17 +46,16 @@
impl Error {
fn to_zx_status(&self) -> zx::Status {
- use fuchsia_fs::node::OpenError;
-
- // TODO(fxbug.dev/86995) Align this mapping with pkgfs.
+ use {fuchsia_fs::node::OpenError, Error::*};
match self {
- Error::OpenMetaFar(OpenError::OpenError(s)) => *s,
- Error::OpenMetaFar(_) => zx::Status::INTERNAL,
- Error::ArchiveReader(fuchsia_archive::Error::Read(_)) => zx::Status::NOT_FOUND,
- Error::ArchiveReader(_)
- | Error::ReadMetaContents(_)
- | Error::DeserializeMetaContents(_) => zx::Status::INVALID_ARGS,
- Error::FileDirectoryCollision { .. } => zx::Status::INVALID_ARGS,
+ MissingMetaFar => zx::Status::NOT_FOUND,
+ OpenMetaFar(OpenError::OpenError(s)) => *s,
+ OpenMetaFar(_) => zx::Status::INTERNAL,
+ ArchiveReader(fuchsia_archive::Error::Read(_)) => zx::Status::IO,
+ ArchiveReader(_) | ReadMetaContents(_) | DeserializeMetaContents(_) => {
+ zx::Status::INVALID_ARGS
+ }
+ FileDirectoryCollision { .. } => zx::Status::INVALID_ARGS,
}
}
}
@@ -359,10 +361,7 @@
server_end.into_channel().into(),
)
.await,
- // RootDir opens the meta.far without requesting an OnOpen event, which improves
- // latency, but results in a less-than-ideal error (a PEER_CLOSED while reading from
- // the meta.far).
- Err(Error::ArchiveReader(_))
+ Err(Error::MissingMetaFar)
);
assert_eq!(node_into_on_open_status(proxy).await, Some(zx::Status::NOT_FOUND));
diff --git a/src/sys/pkg/lib/package-directory/src/root_dir.rs b/src/sys/pkg/lib/package-directory/src/root_dir.rs
index 1f10ad4..cd58f3d 100644
--- a/src/sys/pkg/lib/package-directory/src/root_dir.rs
+++ b/src/sys/pkg/lib/package-directory/src/root_dir.rs
@@ -21,6 +21,7 @@
fuchsia_pkg::MetaContents,
fuchsia_syslog::fx_log_err,
fuchsia_zircon as zx,
+ futures::stream::StreamExt as _,
std::{collections::HashMap, sync::Arc},
vfs::{
common::send_on_open_with_error,
@@ -50,13 +51,27 @@
/// Loads the package metadata given by `hash` from `non_meta_storage`, returning an object
/// representing the package, backed by `non_meta_storage`.
pub async fn new(non_meta_storage: S, hash: fuchsia_hash::Hash) -> Result<Self, Error> {
- let meta_far =
- open_for_read_no_describe(&non_meta_storage, &hash).map_err(Error::OpenMetaFar)?;
+ let meta_far = open_for_read(&non_meta_storage, &hash, fio::OpenFlags::DESCRIBE)
+ .map_err(Error::OpenMetaFar)?;
- let reader = fuchsia_fs::file::AsyncFile::from_proxy(Clone::clone(&meta_far));
- let mut async_reader = AsyncReader::new(fuchsia_fs::file::BufferedAsyncReadAt::new(reader))
- .await
- .map_err(Error::ArchiveReader)?;
+ let mut async_reader = match AsyncReader::new(fuchsia_fs::file::BufferedAsyncReadAt::new(
+ fuchsia_fs::file::AsyncFile::from_proxy(Clone::clone(&meta_far)),
+ ))
+ .await
+ {
+ Ok(async_reader) => async_reader,
+ Err(e) => {
+ if matches!(
+ meta_far.take_event_stream().next().await,
+ Some(Ok(fio::FileEvent::OnOpen_{s, ..}))
+ if s == zx::Status::NOT_FOUND.into_raw()
+ ) {
+ return Err(Error::MissingMetaFar);
+ }
+ return Err(Error::ArchiveReader(e));
+ }
+ };
+
let reader_list = async_reader.list();
let mut meta_files = HashMap::with_capacity(reader_list.size_hint().0);
@@ -93,7 +108,7 @@
/// https://fuchsia.dev/fuchsia-src/concepts/process/namespaces?hl=en#object_relative_path_expressions
pub async fn read_file(&self, path: &str) -> Result<Vec<u8>, ReadFileError> {
if let Some(hash) = self.non_meta_files.get(path) {
- let blob = open_for_read_no_describe(&self.non_meta_storage, hash)
+ let blob = open_for_read(&self.non_meta_storage, hash, fio::OpenFlags::empty())
.map_err(ReadFileError::Open)?;
return fuchsia_fs::file::read(&blob).await.map_err(ReadFileError::Read);
}
@@ -350,16 +365,18 @@
open_as_file || !open_as_dir
}
-// Open a non-meta file by hash with flags of OPEN_RIGHT_READABLE and return the proxy.
-fn open_for_read_no_describe(
+// Open a non-meta file by hash with flags of `OPEN_RIGHT_READABLE | additional_flags` and return
+// the proxy.
+fn open_for_read(
non_meta_storage: &impl crate::NonMetaStorage,
blob: &fuchsia_hash::Hash,
+ additional_flags: fio::OpenFlags,
) -> Result<fio::FileProxy, fuchsia_fs::node::OpenError> {
let (file, server_end) = fidl::endpoints::create_proxy::<fio::FileMarker>()
.map_err(fuchsia_fs::node::OpenError::CreateProxy)?;
let () = non_meta_storage.open(
blob,
- fio::OpenFlags::RIGHT_READABLE,
+ fio::OpenFlags::RIGHT_READABLE | additional_flags,
0,
server_end.into_channel().into(),
)?;
@@ -374,7 +391,6 @@
assert_matches::assert_matches,
fidl::endpoints::{create_proxy, Proxy as _},
fuchsia_pkg_testing::{blobfs::Fake as FakeBlobfs, PackageBuilder},
- futures::stream::StreamExt as _,
pretty_assertions::assert_eq,
std::convert::TryInto as _,
vfs::directory::{entry::DirectoryEntry, entry_container::Directory},
@@ -407,6 +423,15 @@
}
#[fuchsia_async::run_singlethreaded(test)]
+ async fn new_missing_meta_far_error() {
+ let (_blobfs_fake, blobfs_client) = FakeBlobfs::new();
+ assert_matches!(
+ RootDir::new(blobfs_client, [0; 32].into()).await,
+ Err(Error::MissingMetaFar)
+ );
+ }
+
+ #[fuchsia_async::run_singlethreaded(test)]
async fn new_initializes_maps() {
let (_env, root_dir) = TestEnv::new().await;
