Skip to content

voxel image export --raw reports success despite read error #57

Description

@taspelund

When exporting an image created by voxel using voxel image export --raw, I see a vague I/O error printed followed by a report that the export was successful:

trey@korgano 01:26:35 AM | ~/git/voxel ➦ 1a7cb55
‣ pfexec ./target/release/voxel image export voxel-bird-proto "$PWD/voxel-bird-proto_0.raw.xz" --raw
[voxel] exporting DATA/falcon/img/voxel-bird-proto@base -> /home/trey/git/voxel/voxel-bird-proto_0.raw.xz
read: I/O error
exported /home/trey/git/voxel/voxel-bird-proto_0.raw.xz

I threw claude at the error and it thinks there are two problems colluding to produce this symptom:

Problem 1:
The export logic effectively shells out to bash -c and supplies dd if={zvol} bs=1M status=none | zx -T0 -c without setting pipefail, which means that the return code of dd is not considered before deciding whether to pipe the result through xz. This allows the dd error to go unnoticed, allowing the export logic to continue.

Problem 2:
It seems like ZFS returns EIO when userspace tries to read from an offset that is exactly the same as the volsize. It's currently unclear to me whether this is a bug in ZFS, a bug in dd (not thinking ahead to avoid the final read when it's at exactly that offset), or somewhere in between.

Either way, the end result is bad for the user (me!).

Claude's suggested fix:

A minimal fix in voxel/src/image.rswould bound the read using volsize and propagate pipeline failures.

Replace the raw branch’s dd construction with:

let volume = format!("{dataset}/img/{name}");
let zvol = format!("/dev/zvol/rdsk/{volume}");

let size = std::process::Command::new("zfs")
    .args(["get", "-Hp", "-o", "value", "volsize", &volume])
    .output()
    .with_context(|| format!("query volsize for {volume}"))?;

if !size.status.success() {
    bail!(
        "query volsize for {volume}: {}",
        String::from_utf8_lossy(&size.stderr).trim()
    );
}

let bytes: u64 = std::str::from_utf8(&size.stdout)?
    .trim()
    .parse()
    .with_context(|| format!("invalid volsize for {volume}"))?;

// Prefer 1 MiB reads, falling back to sectors for non-aligned volumes.
let block: u64 = if bytes % 1_048_576 == 0 { 1_048_576 } else { 512 };
if bytes == 0 || bytes % block != 0 {
    bail!("unsupported volsize for {volume}: {bytes} bytes");
}
let count = bytes / block;

(
    format!("{name}.raw.xz"),
    format!(
        "dd if={} bs={block} count={count} status=none | xz -T0 -c",
        shell_quote(&zvol)
    ),
)

Then change the pipeline execution and error reporting to:

let status = std::process::Command::new("bash")
    .args(["-o", "pipefail", "-c"])
    .arg(format!("{pipe} > {}", shell_quote(out.as_str())))
    .status()
    .with_context(|| format!("export {snap} to {out}"))?;

if !status.success() {
    bail!(
        "export {snap} to {out} failed ({status}); \
         output may be incomplete—see command errors above"
    );
}
println!("exported {out}");

The count prevents the extra boundary read. pipefail ensures a genuine reader or compressor failure cannot produce a success message.

This is a proposed snippet, not an applied change. A fuller fix should also write to a temporary file and rename it only after success, preserving any previous successful export

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions