Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A few bugfixes #10

Closed
wants to merge 7 commits into from
Closed

Conversation

liubogithub
Copy link
Contributor

This PR consists of several bug fixes regarding to fuse ops readdirplus.

eryugey and others added 5 commits October 21, 2020 15:58
Currently bytes_to_cstr() expects one and only one '\0' and the end of
the buffer, but we've seen lookup request with name buffer having
multiple '\0's at the end and caused failures.

To make it more robust, trim extra zeros in buffer, this also follows
what virtiofsd does.

Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
Writing fuse replies back to kernel should always return `EncodeMessage`
with errors. So writing fuse replies will return unique error code
hinting an error happens when replies.

Signed-off-by: Changwei Ge <chge@linux.alibaba.com>
virtiofsd uses "none" as one of the cache policies, but passthroughfs
uses "never". Support "none" as well to be compatible with virtiofsd's
configuration.

Signed-off-by: Eryu Guan <eguan@linux.alibaba.com>
If there's an error, we can only signal it if we haven't stored any entries
yet - otherwise we'd end up with wrong lookup counts for the entries that
are already in the buffer. So we return what we've collected until that
point.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
The Sys_getdents64 in kernel will pad the name with '\0' bytes up to 8-byte
alignment, so @name of LinuxDirent64 may contain a few null terminators.

This causes an extra lookup from fuse kernel when called by readdirplus,
because kernel path walking only takes name without null terminators, the
dentry with more than 1 null terminators added by readdirplus doesn't
satisfy the path walking so that kernel has to issue another lookup to get
a valid dentry.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
changweige and others added 2 commits October 21, 2020 16:11
cargo clippy --features=fusedev --target-dir target-fusedev --
-Dclippy::all
    Checking fuse-rs v0.1.0 (/root/git_repo/fuse-backend-rs)
error: using `Option.and_then(|x| Some(y))`, which is more succinctly
expressed as `map(|x| y)`
   -->
/root/git_repo/fuse-backend-rs/src/transport/fusedev/mod.rs:166:17
    |
166 | /                 e.as_errno()
167 | |                     .and_then(|r|
Some(io::Error::from_raw_os_error(r as i32)))
    |
|_______________________________________________________________________________^
help: try this: `e.as_errno().map(|r| io::Error::from_raw_os_error(r as
i32))`
    |
    = note: `-D clippy::bind-instead-of-map` implied by `-D clippy::all`
    = help: for further information visit
https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map

Signed-off-by: Changwei Ge <chge@linux.alibaba.com>
Remove some unnecessary map() calls.

Signed-off-by: Liu Bo <bo.liu@linux.alibaba.com>
@liubogithub
Copy link
Contributor Author

liubogithub commented Oct 21, 2020

It is weird that building failed, I did the same test locally, it ran fine.

The test is

clippy and format
      run: |
        cargo clippy --features "fusedev" -- -Dclippy::all
        cargo clippy --features "virtiofs" -- -Dclippy::all
        cargo clippy --features "vhost-user-fs" -- -Dclippy::all
        cargo fmt -- --check
root@6:/mnt/rust# cargo clippy --features "virtiofs" -- -Dclippy::all
    Updating crates.io index
    Updating git repository `https://github.com/cloud-hypervisor/vhost.git`
    Updating git repository `https://github.com/cloud-hypervisor/vm-virtio.git`
    Updating git submodule `https://github.com/rust-vmm/rust-vmm-ci.git`
  Downloaded cfg-if v0.1.10
  Downloaded bimap v0.4.0
  Downloaded libc v0.2.72
  Downloaded void v1.0.2
  Downloaded nix v0.17.0
  Downloaded vm-memory v0.2.1
  Downloaded arc-swap v0.4.7
  Downloaded log v0.4.6
  Downloaded vmm-sys-util v0.6.1
  Downloaded byteorder v1.2.1
  Downloaded bitflags v1.2.1
  Downloaded 11 crates (932.3 KB) in 0.63s
   Compiling libc v0.2.72
   Compiling bitflags v1.2.1
    Checking cfg-if v0.1.10
   Compiling nix v0.17.0
    Checking byteorder v1.2.1
    Checking void v1.0.2
    Checking arc-swap v0.4.7
    Checking log v0.4.6
    Checking bimap v0.4.0
    Checking vm-memory v0.2.1
    Checking vmm-sys-util v0.6.1
    Checking vm-virtio v0.1.0 (https://github.com/cloud-hypervisor/vm-virtio.git?branch=dragonball#2bf58c10)
    Checking fuse-rs v0.1.0 (/mnt/rust)
    Finished dev [unoptimized + debuginfo] target(s) in 44.45s

@bergwolf
Copy link
Contributor

@liubogithub Maybe rust version problem?

@liubogithub
Copy link
Contributor Author

liubogithub commented Oct 22, 2020

Looks like it's vm-memory version causing the trouble.

So the problem is cause by the fact that vm-virtio needs a feature integer-atomics which is only available at vm-memory 0.2.1, but vm-memory 0.3 is pulled from crates.io.

Update:
I've created a PR for vm-virtio dragonball branch to upgrade to vm-memory 0.3.0 cloud-hypervisor/vm-virtio#4, once it gets merged, we'll be fine here.

@jiangliu
Copy link
Contributor

Looks like it's vm-memory version causing the trouble.

So the problem is cause by the fact that vm-virtio needs a feature integer-atomics which is only available at vm-memory 0.2.1, but vm-memory 0.3 is pulled from crates.io.

Update:
I've created a PR for vm-virtio dragonball branch to upgrade to vm-memory 0.3.0 cloud-hypervisor/vm-virtio#4, once it gets merged, we'll be fine here.

Solved by this PR: cloud-hypervisor/vm-virtio#6 (comment)

@liubogithub liubogithub deleted the sync branch January 8, 2021 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants