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

riscv64: Mount tmpfs to support unit-tests expecting /tmp #117

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

TimePrinciple
Copy link
Collaborator

@TimePrinciple TimePrinciple commented Sep 28, 2024

Summary of the PR

  • As 9p rootfs documented [1], mount a tmpfs onto /tmp to avoid E: Unable to determine file size for fd 7 - fstat (2: No such file or directory).

Modify /etc/fstab to apply the mount operation during boot.

[1] https://wiki.qemu.org/Documentation/9p_root_fs#Let's_start_the_Installation

  • Previous implementation uses netcat to test existence of
    pre-configured target port, but that port is occupied in advance of
    sshd inside VM is ready.

Replace netcat with ssh with ConnectTimeout set to directly verify
the connectivity of sshd.

Requirements

Before submitting your PR, please make sure you addressed the following
requirements:

  • All commits in this PR have Signed-Off-By trailers (with
    git commit -s), and the commit message has max 60 characters for the
    summary and max 75 characters for each description line.
  • All added/changed functionality has a corresponding unit/integration
    test.
  • All added/changed public-facing functionality has entries in the "Upcoming
    Release" section of CHANGELOG.md (if no such section exists, please create one).
  • Any newly added unsafe code is properly documented.

@TimePrinciple
Copy link
Collaborator Author

😵‍💫 Finally worked, going to flesh out this skeleton after dinner

@TimePrinciple
Copy link
Collaborator Author

This version (virtio-blk as rootfs):

image

9p as rootfs version:

image

Copy link
Member

@stefano-garzarella stefano-garzarella left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the order of the patch. IMHO the first patch should be applied only when we have the new way to build the rootfs, otherwise we can break the bisectabiliy.
So you can move that patch after riscv64: Update container build workflow, or just merge with it.

riscv64/start_in_qemu.sh Outdated Show resolved Hide resolved
riscv64/rootfs_finalize.sh Outdated Show resolved Hide resolved
riscv64/rootfs_finalize.sh Outdated Show resolved Hide resolved
@roypat
Copy link
Collaborator

roypat commented Sep 30, 2024

Mh, shouldn't /tmp be an in-memory file system? Why does switching the storage driver from 9p to virtio-blk affect it at all? :o

@TimePrinciple
Copy link
Collaborator Author

TimePrinciple commented Sep 30, 2024

Mh, shouldn't /tmp be an in-memory file system? Why does switching the storage driver from 9p to virtio-blk affect it at all? :o

I'm not sure about the cause of this issue :(

I normally develop and test in an normally booted vm, i.e., the virtio-blk as rootfs way, I never fail any case relates to /tmp.

I tried to address that issue with in 9p, originally I thought it was the sticky bit set which prevents them from passing, but I tried to turn that bit off, unfortunately those cases still fail 😵‍💫

@roypat
Copy link
Collaborator

roypat commented Sep 30, 2024

Mh, shouldn't /tmp be an in-memory file system? Why does switching the storage driver from 9p to virtio-blk affect it at all? :o

I'm not sure about the cause of this issue :(

I normally develop and test in an normally booted vm, i.e., the virtio-blk as rootfs way, I never fail any case relates to /tmp.

I tried to address that issue with in 9p, originally I thought it was the sticky bit set which prevents them from passing, but I tried to turn that bit off, unfortunately those cases still fail 😵‍💫

I found some random documentation about using a 9p rootfs with qemu (https://wiki.qemu.org/Documentation/9p_root_fs#Let's_start_the_Installation). It seems similar to enough to what we're doing. Maybe this section also applies to us:

Important: now we need to mount a tmpfs on /tmp (inside the chroot environment that we are in now).

mount -t tmpfs -o noatime,size=500M tmpfs /tmp

If you omit the previous step, you will most likely get error messages like the following with the subsequent apt commands below:

E: Unable to determine file size for fd 7 - fstat (2: No such file or directory)

Could you try explicitly mounting a tmpfs inside the qemu guest?

@TimePrinciple
Copy link
Collaborator Author

Mh, shouldn't /tmp be an in-memory file system? Why does switching the storage driver from 9p to virtio-blk affect it at all? :o

I'm not sure about the cause of this issue :(
I normally develop and test in an normally booted vm, i.e., the virtio-blk as rootfs way, I never fail any case relates to /tmp.
I tried to address that issue with in 9p, originally I thought it was the sticky bit set which prevents them from passing, but I tried to turn that bit off, unfortunately those cases still fail 😵‍💫

I found some random documentation about using a 9p rootfs with qemu (https://wiki.qemu.org/Documentation/9p_root_fs#Let's_start_the_Installation). It seems similar to enough to what we're doing. Maybe this section also applies to us:

Important: now we need to mount a tmpfs on /tmp (inside the chroot environment that we are in now).
mount -t tmpfs -o noatime,size=500M tmpfs /tmp
If you omit the previous step, you will most likely get error messages like the following with the subsequent apt commands below:
E: Unable to determine file size for fd 7 - fstat (2: No such file or directory)

Could you try explicitly mounting a tmpfs inside the qemu guest?

Yes, I'll try it now

@TimePrinciple
Copy link
Collaborator Author

Mh, shouldn't /tmp be an in-memory file system? Why does switching the storage driver from 9p to virtio-blk affect it at all? :o

I'm not sure about the cause of this issue :(
I normally develop and test in an normally booted vm, i.e., the virtio-blk as rootfs way, I never fail any case relates to /tmp.
I tried to address that issue with in 9p, originally I thought it was the sticky bit set which prevents them from passing, but I tried to turn that bit off, unfortunately those cases still fail 😵‍💫

I found some random documentation about using a 9p rootfs with qemu (https://wiki.qemu.org/Documentation/9p_root_fs#Let's_start_the_Installation). It seems similar to enough to what we're doing. Maybe this section also applies to us:

Important: now we need to mount a tmpfs on /tmp (inside the chroot environment that we are in now).
mount -t tmpfs -o noatime,size=500M tmpfs /tmp
If you omit the previous step, you will most likely get error messages like the following with the subsequent apt commands below:
E: Unable to determine file size for fd 7 - fstat (2: No such file or directory)

Could you try explicitly mounting a tmpfs inside the qemu guest?

It worked 🎉, by mounting the tmpfs inside QEMU VM.

I just recollected I have tried something similar, I was mounting tmpfs from the outside, i.e. the container which prevents me from mounting 😵‍💫

I will repurpose this PR. Thanks to @roypat @stefano-garzarella :)

@TimePrinciple TimePrinciple changed the title riscv64: Use raw image to boot VM riscv64: Fix Unable to determine file size error Sep 30, 2024
@TimePrinciple TimePrinciple changed the title riscv64: Fix Unable to determine file size error riscv64: Mount tmpfs to support unit-tests expecting /tmp Sep 30, 2024
As `9p` rootfs documented [1], mount a `tmpfs` onto `/tmp` could avoid
`E: Unable to determine file size for fd 7 - fstat (2: No such file or
directory)`.

And various unittests in `linux-loader`, `vm-memory` and etc. explicitly
expect `/tmp` to exist, and were failing on riscv because no `tmpfs` was
mounted.

Modify `/etc/fstab` to apply the mount operation during boot.

[1] https://wiki.qemu.org/Documentation/9p_root_fs#Let's_start_the_Installation

Signed-off-by: Ruoqing He <[email protected]>
Previous implementation uses `netcat` to test existence of
pre-configured target port, but that port is occupied in advance of
`sshd` inside VM is ready.

Replace `netcat` with `ssh` with `ConnectTimeout` set to directly verify
the connectivity of `sshd`.

Signed-off-by: Ruoqing He <[email protected]>
TimePrinciple added a commit to TimePrinciple/rust-vmm-ci that referenced this pull request Sep 30, 2024
This version [1] incorporates:
- Support of running unit-tests of `linux-loader`, `vm-memory` and etc.
  by mounting `tmpfs` onto `/tmp`
- Improvement on `sshd` inside QEMU VM connectivity test.

[1] rust-vmm/rust-vmm-container#117

Signed-off-by: Ruoqing He <[email protected]>
@roypat roypat merged commit 5ad7c27 into rust-vmm:main Sep 30, 2024
3 checks passed
@TimePrinciple TimePrinciple deleted the final branch September 30, 2024 16:24
roypat pushed a commit to rust-vmm/rust-vmm-ci that referenced this pull request Oct 1, 2024
This version [1] incorporates:
- Support of running unit-tests of `linux-loader`, `vm-memory` and etc.
  by mounting `tmpfs` onto `/tmp`
- Improvement on `sshd` inside QEMU VM connectivity test.

[1] rust-vmm/rust-vmm-container#117

Signed-off-by: Ruoqing He <[email protected]>
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.

4 participants