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

Remove warnings during build and unit test #245

Merged
merged 3 commits into from
May 3, 2018
Merged

Remove warnings during build and unit test #245

merged 3 commits into from
May 3, 2018

Conversation

dianpopa
Copy link
Contributor

@dianpopa dianpopa commented Apr 30, 2018

Changes

  • rust issue inserted a warning if a packed struct (i.e #[repr(C, packed)] which is unaligned (or has unaligned fields) has builtin derives.
    • per the aforementioned issue, the alternatives are: If your struct already derives Copy and has no generics, the compiler will generate code that copies the fields to locals and will therefore work. Otherwise, you'll have to write the derive implementations manually.
    • concluding, we could remove the derives at all or we could add the #[derive(Copy, Clone)]
    • I personally prefered to remove the derives at all and implement them or just add the Copy/Clone when the struct will actually be used.
  • remove warning from virtio block that would appear with cargo test
  • added patches directory for the virtio_sys containing the patch that should be applied whenever virtio_net will be regenerated with bindgen.

Testing

Build Time

Prerequisite

## add the necessary musl target to the active toolchain
rustup target add x86_64-unknown-linux-musl

Build tests

cargo fmt —all
cargo build # no warning
cargo build —release # no warning
sudo env "PATH=$PATH" cargo test --all #no warin
sudo env "PATH=$PATH" cargo kcov —all # overall 44.3%

Integration Testing

rm -f /tmp/firecracker.socket && \
target/debug/firecracker --api-sock=/tmp/firecracker.socket
  • in another console start sending curl requests for starting basic firecracker guest
curl --unix-socket /tmp/firecracker.socket -i  \
     -X PUT "http://localhost/boot-source" \
     -H "accept: application/json" \
     -H "Content-Type: application/json" \
     -d "{ 
           \"boot_source_id\": \"alinux_kernel\",
           \"source_type\": \"LocalImage\", 
           \"local_image\": 
                { 
                    \"kernel_image_path\": \"${kernel_path}\" 
                }
        }"

echo 2
curl --unix-socket /tmp/firecracker.socket -i \
     -X PUT "http://localhost/machine-config" \
     -H "accept: application/json" \
     -H "Content-Type: application/json" \
     -d "{ \"vcpu_count\": 4, \"mem_size_mib\": 256}"

# Add root block device
echo 3
curl --unix-socket /tmp/firecracker.socket -i \
     -X PUT "http://localhost/drives/root" \
     -H "accept: application/json" \
     -H "Content-Type: application/json" \
     -d "{ 
            \"drive_id\": \"root\",
            \"path_on_host\": \"${rootfs_path}\", 
            \"is_root_device\": true, 
            \"permissions\": \"rw\", 
            \"state\": \"Attached\"
         }"


# Add readonly device
echo 4
curl --unix-socket /tmp/firecracker.socket -i \
     -X PUT "http://localhost/drives/read_only_drive" \
     -H "accept: application/json" \
     -H "Content-Type: application/json" \
     -d "{ 
            \"drive_id\": \"read_only_drive\",
            \"path_on_host\": \"${ro_drive_path}\", 
            \"is_root_device\": false, 
            \"permissions\": \"ro\", 
            \"state\": \"Attached\"
         }"

# Create a tap interface
ip tuntap add name vmtap33 mode tap
ifconfig vmtap33 192.168.241.1/24 up
echo 5
curl --unix-socket /tmp/firecracker.socket -i \
     -X PUT "http://localhost/network-interfaces/1" \
     -H "accept: application/json" \
     -H "Content-Type: application/json" \
     -d "{ 
            \"iface_id\": \"1\", 
            \"host_dev_name\": \"vmtap33\", 
            \"guest_mac\": \"06:00:00:00:00:01\", 
            \"state\": \"Attached\" 
        }"

echo 6
curl --unix-socket /tmp/firecracker.socket -i \
     -X PUT "http://localhost/actions/start" \
     -H  "accept: application/json" \
     -H  "Content-Type: application/json" \
     -d "{  
            \"action_id\": \"start\",  
            \"action_type\": \"InstanceStart\"
         }"

# Get the response of starting the instance
echo 7
curl --unix-socket /tmp/firecracker.socket -i \
     -X GET "http://localhost/actions/start" \
     -H "accept: application/json"

sudo ip link delete vmtap33
  • in the initial console the guest should start up
# login: ...
# reboot

P.S. testing

Please do yourself a cargo build and cargo test to check that the warnings are not there anymore.

Miscellanous

  • created patch with:
git format-patch -1 <sha>
  • patch should be applied on a regenerated virtio_net.rs with:
patch -p1 < file.patch

@dianpopa dianpopa self-assigned this Apr 30, 2018
@dianpopa dianpopa requested a review from a team April 30, 2018 11:22
containing an unaligned field.

Signed-off-by: Diana Popa <[email protected]>
This will be used for future updates of virtio_sys auto bindgen'ed
files.It should contain all the patches that need to be applied
over any commit that regenerates any of the virtio_sys files.

Signed-off-by: Diana Popa <[email protected]>
@acatangiu acatangiu requested a review from alxiord May 2, 2018 09:29
@dianpopa dianpopa requested a review from a team May 2, 2018 15:49
@acatangiu acatangiu merged commit 97bb34a into firecracker-microvm:master May 3, 2018
@dianpopa dianpopa mentioned this pull request May 3, 2018
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.

3 participants