diff --git a/.travis.yml b/.travis.yml index 4f5ed1c..9472c66 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,26 +11,31 @@ env: - secure: iS8MtXltyXj/T1EgpZaMillH/SuMAinX7ma5ER1Zt54F9CS3/tH+pG9Yx8uM8sUOZU90xvmN0AZ4HI4sPzquj8UUS+a0Xoj4kj43eGG90bi3nqtdNtopQOZpLFhvm7+tMzRztHLocvlPWLu1l8xHeGycm+J7X1sx+CW+p4uotPE= - PINS="nbd:. nbd-lwt-unix:. nbd-tool:." - SCRIPT=.travis-docker.sh + # Pass through these extra env vars into the Docker container + # STRICT=true: Fail the nbd-tool interop tests if the requirements are missing + - EXTRA_ENV="STRICT=true" matrix: - # We need to pass some Travis environment variables to the container to - # enable uploading to coveralls and detection of Travis CI. - # Also, set TESTS to false to avoid running them twice. - # We also have to install the test dependencies because they are not there - # when POST_INSTALL_HOOK is run, even if TESTS is set to true. - PACKAGE=nbd DISTRO="debian-unstable" OCAML_VERSION=4.06 \ - TESTS=false \ - POST_INSTALL_HOOK="opam install alcotest alcotest-lwt io-page-unix; env TRAVIS=$TRAVIS TRAVIS_JOB_ID=$TRAVIS_JOB_ID bash -ex coverage.sh" - PACKAGE=nbd DISTRO="debian-unstable" OCAML_VERSION=4.07 # Upload docs for both nbd and nbd-lwt-unix - PACKAGE=nbd-lwt-unix DISTRO="debian-unstable" OCAML_VERSION=4.06 SCRIPT=.travis-docker-docgen.sh - PACKAGE=nbd-lwt-unix DISTRO="debian-unstable" OCAML_VERSION=4.07 - - PACKAGE=nbd-tool DISTRO="debian-unstable" OCAML_VERSION=4.06 - - PACKAGE=nbd-tool DISTRO="debian-unstable" OCAML_VERSION=4.07 + # We set TESTS to false to avoid running them twice. + # We install the packages used for interop tests with nbd-tool in + # PRE_INSTALL_HOOK. + # We also have to install the opam test dependencies because they are not there + # when POST_INSTALL_HOOK is run, even if TESTS is set to true. + # And we hvae to pass some Travis environment variables to the container to + # enable uploading to coveralls and detection of Travis CI. + - PACKAGE=nbd-tool DISTRO="debian-unstable" PRE_INSTALL_HOOK="sudo apt-get install -y qemu-utils nbd-client" OCAML_VERSION=4.06 \ + TESTS=false \ + POST_INSTALL_HOOK="opam install alcotest alcotest-lwt io-page-unix; env TRAVIS=$TRAVIS TRAVIS_JOB_ID=$TRAVIS_JOB_ID bash -ex coverage.sh" + - PACKAGE=nbd-tool DISTRO="debian-unstable" PRE_INSTALL_HOOK="sudo apt-get install -y qemu-utils nbd-client" OCAML_VERSION=4.07 matrix: fast_finish: true allow_failures: - env: PACKAGE=nbd DISTRO="debian-unstable" OCAML_VERSION=4.07 - env: PACKAGE=nbd-lwt-unix DISTRO="debian-unstable" OCAML_VERSION=4.07 - - env: PACKAGE=nbd-tool DISTRO="debian-unstable" OCAML_VERSION=4.07 + - env: PACKAGE=nbd-tool DISTRO="debian-unstable" PRE_INSTALL_HOOK="sudo apt-get install -y qemu-utils nbd-client" OCAML_VERSION=4.07 branches: only: master diff --git a/Makefile b/Makefile index 794cdd1..db8bd3d 100644 --- a/Makefile +++ b/Makefile @@ -18,7 +18,7 @@ clean: test: dune runtest -# requires sudo access, nbd-client, and hdparm +# requires qemu-img benchmark: build ./benchmark.sh diff --git a/benchmark.sh b/benchmark.sh index f97d818..a643003 100755 --- a/benchmark.sh +++ b/benchmark.sh @@ -1,32 +1,18 @@ #!/bin/bash -# Run this script to benchmark the NBD server using nbd-client and hdparm. +# Run this script to benchmark the NBD server using qemu-img. set -eux dd if=/dev/zero of=/tmp/test bs=1M count=100 -_build/default/cli/main.exe serve --no-tls /tmp/test & +_build/default/cli/main.exe serve --exportname test --no-tls /tmp/test & SERVER_PROCESS=$! -echo $SERVER_PROCESS +# Wait for the server to start the main loop +sleep 0.1 -function stop_server { - kill $(jobs -p) +stop_server() { + kill -9 $SERVER_PROCESS } trap stop_server EXIT - -sudo modprobe nbd - -sudo nbd-client -N test localhost /dev/nbd0 - -function stop_client { - sudo nbd-client -d /dev/nbd0 - stop_server -} -trap stop_client EXIT - -sudo hdparm -t /dev/nbd0 -sudo hdparm -t /dev/nbd0 -sudo hdparm -t /dev/nbd0 -sudo hdparm -t /dev/nbd0 -sudo hdparm -t /dev/nbd0 +qemu-img bench 'nbd:0.0.0.0:10809:exportname=test' diff --git a/cli/main.ml b/cli/main.ml index 8cfb25a..6ec264b 100644 --- a/cli/main.ml +++ b/cli/main.ml @@ -173,17 +173,30 @@ module Impl = struct let ignore_exn t () = Lwt.catch t (fun _ -> Lwt.return_unit) - let serve _common filename port certfile ciphersuites no_tls = + let serve _common filename port exportname certfile ciphersuites no_tls = let tls_role = init_tls_get_server_ctx ~certfile ~ciphersuites no_tls in let filename = require "filename" filename in + let validate ~client_exportname = + match exportname with + | Some exportname when exportname <> client_exportname -> + Lwt.fail_with + (Printf.sprintf "Client requested invalid exportname %s, name of the export is %s" client_exportname exportname) + | _ -> Lwt.return_unit + in let handle_connection fd = Lwt.finalize (fun () -> Nbd_lwt_unix.with_channel fd tls_role (fun clearchan -> + let offer = match exportname with + | None -> None + | Some exportname -> Some [exportname] + in Server.with_connection + ?offer clearchan - (fun _exportname svr -> + (fun client_exportname svr -> + validate ~client_exportname >>= fun () -> Nbd_lwt_unix.with_block filename (Server.serve svr ~read_only:false (module Block)) ) @@ -286,10 +299,19 @@ let serve_cmd = let port = let doc = "Local port to listen for connections on" in Arg.(value & opt int 10809 & info [ "port" ] ~doc) in + let exportname = + let doc = {|Export name to use when serving the file. If specified, clients + will be able to list this export, and only this export name + will be accepted. If unspecified, listing the exports will not + be allowed, and all export names will be accepted when + connecting.|} + in + Arg.(value & opt (some string) None & info ["exportname"] ~doc) + in let no_tls = let doc = "Use NOTLS mode (refusing TLS) instead of the default FORCEDTLS." in Arg.(value & flag & info ["no-tls"] ~doc) in - Term.(ret(pure Impl.serve $ common_options_t $ filename $ port $ certfile $ ciphersuites $ no_tls)), + Term.(ret(pure Impl.serve $ common_options_t $ filename $ port $ exportname $ certfile $ ciphersuites $ no_tls)), Term.info "serve" ~sdocs:_common_options ~doc ~man let mirror_cmd = diff --git a/cli_test/dune b/cli_test/dune new file mode 100644 index 0000000..d2ddac3 --- /dev/null +++ b/cli_test/dune @@ -0,0 +1,20 @@ +(executable + (name suite) + (libraries + alcotest + alcotest-lwt + cmdliner + ) +) + +(alias + (name runtest) + (package nbd-tool) + (deps + (:suite suite.exe) + (:cli ../cli/main.exe) + ./test-qemu.sh + ./test-nbd-client.sh + ) + (action (run %{suite} --cli %{cli})) +) diff --git a/cli_test/suite.ml b/cli_test/suite.ml new file mode 100644 index 0000000..bbb92a2 --- /dev/null +++ b/cli_test/suite.ml @@ -0,0 +1,42 @@ + +let with_command ~command ~strict test = + match Sys.command ("command -v " ^ command) with + | 0 -> test () + | _ -> + if strict then failwith ("Command " ^ command ^ " not present") + else Printf.printf "!!! Skipping test because command %s is not available" command + +let script name ~requires (cli, strict) = + with_command + ~command:requires + ~strict + (fun () -> + Alcotest.(check int) + name + 0 + (Sys.command (name ^ " " ^ cli)) + ) + +let cli = + let doc = "Path to nbd CLI should be first command-line argument" in + Cmdliner.Arg.(required & opt ~vopt:None (some string) None & info ["cli"] ~docv:"CLI" ~doc) + +let strict = + let doc = {|If present, the test will fail when the required program is not + installed. Otherwise the test will simply be skipped.|} + in + Cmdliner.Arg.(value & flag & info ["strict"] ~env:(env_var "STRICT") ~doc) + +let opts = Cmdliner.Term.(const (fun cli strict -> (cli, strict)) $ cli $ strict) + +let () = + Alcotest.run_with_args + "Nbd CLI interoperability tests" + opts + [ ("compatibility with qemu-img", + ["data copying", `Slow, script "./test-qemu.sh" ~requires:"qemu-img"] + ) + ; ("compatibility with nbd-client", + ["listing exports", `Slow, script "./test-nbd-client.sh" ~requires:"nbd-client"] + ) + ] diff --git a/cli_test/test-nbd-client.sh b/cli_test/test-nbd-client.sh new file mode 100755 index 0000000..17a2396 --- /dev/null +++ b/cli_test/test-nbd-client.sh @@ -0,0 +1,26 @@ +#!/bin/sh + +set -eux + +CLI=$1 + +SCRATCH=$(mktemp -d) +EXPORT=$SCRATCH/test + +# Create a test file +dd if=/dev/urandom of="$EXPORT" bs=1M count=40 + +# Serve it +$CLI serve --exportname test --no-tls "$EXPORT" & +SERVER=$! +# Wait for the server to start the main loop +sleep 0.1 + +stop_server() { + kill -9 $SERVER +} +trap stop_server EXIT + +# Try listing the exports +# This will fail if the server does not implement NBD_OPT_LIST correctly +nbd-client -l 0.0.0.0 diff --git a/cli_test/test-qemu.sh b/cli_test/test-qemu.sh new file mode 100755 index 0000000..12c8e7c --- /dev/null +++ b/cli_test/test-qemu.sh @@ -0,0 +1,29 @@ +#!/bin/sh + +set -eux + +CLI=$1 + +SCRATCH=$(mktemp -d) +EXPORT=$SCRATCH/test +OUTPUT=$SCRATCH/out + +# Create a test file +dd if=/dev/urandom of="$EXPORT" bs=1M count=40 + +# Serve it +$CLI serve --exportname test --no-tls "$EXPORT" & +SERVER=$! +# Wait for the server to start the main loop +sleep 0.1 + +stop_server() { + kill -9 $SERVER +} +trap stop_server EXIT + +# Download it as raw from the server +qemu-img convert 'nbd:0.0.0.0:10809:exportname=test' -O raw "$OUTPUT" + +# Check that the two files are the same +cmp --silent "$EXPORT" "$OUTPUT" diff --git a/lib_test/dune b/lib_test/dune index c2bd296..d0e0572 100644 --- a/lib_test/dune +++ b/lib_test/dune @@ -1,4 +1,4 @@ -(executable +(test (name suite) (libraries alcotest @@ -8,8 +8,3 @@ lwt_log nbd) ) - -(alias - (name runtest) - (deps (:suite suite.exe)) - (action (run %{suite}))) diff --git a/nbd-tool.opam b/nbd-tool.opam index f25ee77..62627fe 100644 --- a/nbd-tool.opam +++ b/nbd-tool.opam @@ -6,8 +6,11 @@ homepage: "https://github.com/xapi-project/nbd" dev-repo: "https://github.com/xapi-project/nbd.git" bug-reports: "https://github.com/xapi-project/nbd/issues" build: ["dune" "build" "-p" name "-j" jobs] +build-test: ["dune" "runtest" "-p" name] depends: [ "dune" {build} + "alcotest" {test} + "alcotest-lwt" {test} "cmdliner" "lwt" {>= "2.7.0"} "lwt_log" diff --git a/nbd.opam b/nbd.opam index 27505c2..12c0b3e 100644 --- a/nbd.opam +++ b/nbd.opam @@ -14,6 +14,7 @@ depends: [ "dune" {build} "alcotest" {test} "alcotest-lwt" {test} + "io-page-unix" {test} "cstruct" {>= "3.1.0"} "io-page" "mirage-block-lwt"