From 5d75987434b79ed35b0d39e4fbd57062d8bbf8fe Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Sun, 10 Jun 2018 13:20:17 +0200 Subject: [PATCH 01/14] Adding testing facility --- Makefile | 5 ++++- README.md | 10 ++++++++++ test/busexmp.sh | 43 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100755 test/busexmp.sh diff --git a/Makefile b/Makefile index 3c768d7..3d06981 100644 --- a/Makefile +++ b/Makefile @@ -7,7 +7,7 @@ CC := /usr/bin/gcc CFLAGS := -g -pedantic -Wall -Wextra -std=c99 LDFLAGS := -L. -lbuse -.PHONY: all clean +.PHONY: all clean test all: $(TARGET) $(TARGET): %: %.o $(STATIC_LIB) @@ -22,5 +22,8 @@ $(STATIC_LIB): $(LIBOBJS) $(LIBOBJS): %.o: %.c $(CC) $(CFLAGS) -o $@ -c $< +test: $(TARGET) + PATH=$(PWD):$$PATH sudo test/busexmp.sh + clean: rm -f $(TARGET) $(OBJS) $(STATIC_LIB) diff --git a/README.md b/README.md index 520085d..9eea09a 100644 --- a/README.md +++ b/README.md @@ -33,3 +33,13 @@ start reading and writing files on it: mkfs.ext4 /dev/nbd0 mount /dev/nbd0 /mnt + +## Tests + +To perform checks you can run scripts in `test/` directory. They require: + * superuser previlages, + * nbd kernel module loaded, + * BUSE and nbd (`nbd-client`) binaries in PATH. + +`make test` will run all test scripts with BUSE added to PATH and using +sudo to grant permissions. diff --git a/test/busexmp.sh b/test/busexmp.sh new file mode 100755 index 0000000..b6b4565 --- /dev/null +++ b/test/busexmp.sh @@ -0,0 +1,43 @@ +#!/usr/bin/env bash +set -e + +BLOCKDEV=/dev/nbd0 +# quiet version of dd +DD="dd status=none" + +# verify if blockdev is not currently in use +set +e +nbd-client -c "$BLOCKDEV" +if [ $? -ne 1 ]; then + echo "device $BLOCKDEV is not ready to use (already in use or corrupted)" +fi +set -e + +# on exit do cleanup actions +function cleanup () { + # kill and wait for BUSE background job + nbd-client -d "$BLOCKDEV" # this should be `kill $BUSEPID` but currently it seems unsupported + wait $BUSEPID + # remove the test file + rm -f "$TESTFILE" +} +trap cleanup EXIT + +# prepare file with some data +TESTFILE=$(mktemp) +$DD if=/dev/urandom of="$TESTFILE" bs=16M count=1 + +# attach BUSE device +busexmp "$BLOCKDEV" & +BUSEPID=$! + +### do checks ### + +# initialy there are all zeros +cmp <($DD if="$BLOCKDEV" ibs=1k count=5 skip=54) <($DD if=/dev/zero bs=1k count=5) + +# write data at the end +$DD if="$TESTFILE" of="$BLOCKDEV" bs=1M count=2 seek=126 + +# read extending past the end of device +cmp <($DD if="$BLOCKDEV" bs=1M count=5 skip=126) <($DD if="$TESTFILE" bs=1M count=2) From ce1f09abb5f8edd7cbfb92a40eb1f5e559d205dc Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Sun, 10 Jun 2018 20:25:26 +0200 Subject: [PATCH 02/14] Terminate gracefuly when SIGINT or SIGTERM is received --- buse.c | 61 ++++++++++++++++++++++++++++++++++++++ test/signal_termination.sh | 39 ++++++++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100755 test/signal_termination.sh diff --git a/buse.c b/buse.c index 4d52fe3..b63b1cf 100644 --- a/buse.c +++ b/buse.c @@ -17,11 +17,15 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#define _POSIX_C_SOURCE (200809L) + #include #include +#include #include #include #include +#include #include #include #include @@ -80,6 +84,30 @@ static int write_all(int fd, char* buf, size_t count) return 0; } +/* Signal handler to gracefully disconnect from nbd kernel driver. */ +static int nbd_dev_to_disconnect = -1; +static void disconnect_nbd(int signal) { + (void)signal; + if (nbd_dev_to_disconnect != -1) { + if(ioctl(nbd_dev_to_disconnect, NBD_DISCONNECT) == -1) { + warn("failed to request disconect on nbd device"); + } else { + nbd_dev_to_disconnect = -1; + fprintf(stderr, "sucessfuly requested disconnect on nbd device\n"); + } + } +} + +/* Sets signal action like regular sigaction but is suspicious. */ +static int set_sigaction(int sig, const struct sigaction * act) { + struct sigaction oact; + int r = sigaction(sig, act, &oact); + if (r == 0 && oact.sa_handler != SIG_DFL) { + warnx("overriden non-default signal handler (%d: %s)", sig, strsignal(sig)); + } + return r; +} + int buse_main(const char* dev_file, const struct buse_operations *aop, void *userdata) { int sp[2]; @@ -120,6 +148,17 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use assert(err != -1); if (!fork()) { + /* Block all signals to not get interrupted in ioctl(NBD_DO_IT), as + * it seems there is no good way to handle such interruption.*/ + sigset_t sigset; + if ( + sigfillset(&sigset) != 0 || + sigprocmask(SIG_SETMASK, &sigset, NULL) != 0 + ) { + warn("failed to block signals in child"); + return EXIT_FAILURE; + } + /* The child needs to continue setting things up. */ close(sp[0]); sk = sp[1]; @@ -145,6 +184,28 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use exit(0); } + /* Parent handles termination signals by terminating nbd device. */ + assert(nbd_dev_to_disconnect == -1); + nbd_dev_to_disconnect = nbd; + struct sigaction act; + act.sa_handler = disconnect_nbd; + act.sa_flags = SA_RESTART; + if ( + sigemptyset(&act.sa_mask) != 0 || + sigaddset(&act.sa_mask, SIGINT) != 0 || + sigaddset(&act.sa_mask, SIGTERM) != 0 + ) { + warn("failed to prepare signal mask in parent"); + return EXIT_FAILURE; + } + if ( + set_sigaction(SIGINT, &act) != 0 || + set_sigaction(SIGTERM, &act) != 0 + ) { + warn("failed to register signal handlers in parent"); + return EXIT_FAILURE; + } + /* The parent opens the device file at least once, to make sure the * partition table is updated. Then it closes it and starts serving up * requests. */ diff --git a/test/signal_termination.sh b/test/signal_termination.sh new file mode 100755 index 0000000..71e3edb --- /dev/null +++ b/test/signal_termination.sh @@ -0,0 +1,39 @@ +#!/usr/bin/env bash +set -ex + +BLOCKDEV=/dev/nbd0 + +# verify if blockdev is not currently in use +set +e +nbd-client -c "$BLOCKDEV" > /dev/null +if [ $? -ne 1 ]; then + echo "device $BLOCKDEV is not ready to use (already in use or corrupted)" + exit 1 +fi +set -e + +# on exit make sure nbd is disconnected +function cleanup () { + nbd-client -d "$BLOCKDEV" > /dev/null +} +trap cleanup EXIT + +# attach BUSE device +busexmp "$BLOCKDEV" & +BUSEPID=$! + +# wait a bit ensure buse is running and connected to device +sleep 1 +nbd-client -c "$BLOCKDEV" > /dev/null + +# kill it with SIGTERM. Exit code should be 0 - if not bash will break because we have -e option. +kill -s SIGTERM $BUSEPID +wait $BUSEPID + +# attach BUSE again to verify if device is left in usable state +busexmp "$BLOCKDEV" & +BUSEPID=$! +sleep 1 +nbd-client -c "$BLOCKDEV" > /dev/null +kill -s SIGINT $BUSEPID # this time kill it with SIGINT +wait $BUSEPID From 65b1fafb4b2e85d78b54e71c7028bc029a195787 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Sun, 10 Jun 2018 21:04:14 +0200 Subject: [PATCH 03/14] Actually abort test when device is in use + don't print irrelevant output --- test/busexmp.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/busexmp.sh b/test/busexmp.sh index b6b4565..f200b35 100755 --- a/test/busexmp.sh +++ b/test/busexmp.sh @@ -7,16 +7,17 @@ DD="dd status=none" # verify if blockdev is not currently in use set +e -nbd-client -c "$BLOCKDEV" +nbd-client -c "$BLOCKDEV" > /dev/null if [ $? -ne 1 ]; then echo "device $BLOCKDEV is not ready to use (already in use or corrupted)" + exit 1 fi set -e # on exit do cleanup actions function cleanup () { # kill and wait for BUSE background job - nbd-client -d "$BLOCKDEV" # this should be `kill $BUSEPID` but currently it seems unsupported + nbd-client -d "$BLOCKDEV" > /dev/null wait $BUSEPID # remove the test file rm -f "$TESTFILE" From 1496a07dba864c1f033f4abc6a27783d8f91af12 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Sun, 10 Jun 2018 21:27:21 +0200 Subject: [PATCH 04/14] Return unsucessful exit codes when errors are detected --- buse.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/buse.c b/buse.c index 4d52fe3..e922437 100644 --- a/buse.c +++ b/buse.c @@ -126,6 +126,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use if(ioctl(nbd, NBD_SET_SOCK, sk) == -1){ fprintf(stderr, "ioctl(nbd, NBD_SET_SOCK, sk) failed.[%s]\n", strerror(errno)); + return EXIT_FAILURE; } #if defined NBD_SET_FLAGS && defined NBD_FLAG_SEND_TRIM else if(ioctl(nbd, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM) == -1){ @@ -135,12 +136,19 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use else{ err = ioctl(nbd, NBD_DO_IT); fprintf(stderr, "nbd device terminated with code %d\n", err); - if (err == -1) - fprintf(stderr, "%s\n", strerror(errno)); + if (err == -1) { + fprintf(stderr, "%s\n", strerror(errno)); + return EXIT_FAILURE; + } } - ioctl(nbd, NBD_CLEAR_QUE); - ioctl(nbd, NBD_CLEAR_SOCK); + if ( + ioctl(nbd, NBD_CLEAR_QUE) == -1 || + ioctl(nbd, NBD_CLEAR_SOCK) == -1 + ) { + fprintf(stderr, "failed to perform nbd cleanup actions: %s\n", strerror(errno)); + return EXIT_FAILURE; + } exit(0); } @@ -228,7 +236,9 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use assert(0); } } - if (bytes_read == -1) + if (bytes_read == -1) { fprintf(stderr, "%s\n", strerror(errno)); + return EXIT_FAILURE; + } return 0; } From ac5f0b2427a1c31c4e5877261fabbff721f29f4a Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Tue, 12 Jun 2018 16:49:52 +0200 Subject: [PATCH 05/14] Test fixes --- Makefile | 1 + test/busexmp.sh | 1 + test/signal_termination.sh | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3d06981..1cd4982 100644 --- a/Makefile +++ b/Makefile @@ -24,6 +24,7 @@ $(LIBOBJS): %.o: %.c test: $(TARGET) PATH=$(PWD):$$PATH sudo test/busexmp.sh + PATH=$(PWD):$$PATH sudo test/signal_termination.sh clean: rm -f $(TARGET) $(OBJS) $(STATIC_LIB) diff --git a/test/busexmp.sh b/test/busexmp.sh index f200b35..5dc3e75 100755 --- a/test/busexmp.sh +++ b/test/busexmp.sh @@ -30,6 +30,7 @@ $DD if=/dev/urandom of="$TESTFILE" bs=16M count=1 # attach BUSE device busexmp "$BLOCKDEV" & +sleep 1 BUSEPID=$! ### do checks ### diff --git a/test/signal_termination.sh b/test/signal_termination.sh index 71e3edb..f144e23 100755 --- a/test/signal_termination.sh +++ b/test/signal_termination.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -set -ex +set -e BLOCKDEV=/dev/nbd0 From 79d49fe2164c548dc995547ebbce1130bed11408 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Tue, 12 Jun 2018 17:08:38 +0200 Subject: [PATCH 06/14] Moved nbd socket serving loop to separate function --- buse.c | 169 +++++++++++++++++++++++++++++---------------------------- 1 file changed, 87 insertions(+), 82 deletions(-) diff --git a/buse.c b/buse.c index 06d44be..444682e 100644 --- a/buse.c +++ b/buse.c @@ -108,10 +108,8 @@ static int set_sigaction(int sig, const struct sigaction * act) { return r; } -int buse_main(const char* dev_file, const struct buse_operations *aop, void *userdata) -{ - int sp[2]; - int nbd, sk, err, tmp_fd; +/* Serve userland side of nbd socket. If everything worked ok, return 0. */ +static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata) { u_int64_t from; u_int32_t len; ssize_t bytes_read; @@ -119,6 +117,90 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use struct nbd_reply reply; void *chunk; + reply.magic = htonl(NBD_REPLY_MAGIC); + reply.error = htonl(0); + + while ((bytes_read = read(sk, &request, sizeof(request))) > 0) { + assert(bytes_read == sizeof(request)); + memcpy(reply.handle, request.handle, sizeof(reply.handle)); + reply.error = htonl(0); + + len = ntohl(request.len); + from = ntohll(request.from); + assert(request.magic == htonl(NBD_REQUEST_MAGIC)); + + switch(ntohl(request.type)) { + /* I may at some point need to deal with the the fact that the + * official nbd server has a maximum buffer size, and divides up + * oversized requests into multiple pieces. This applies to reads + * and writes. + */ + case NBD_CMD_READ: + fprintf(stderr, "Request for read of size %d\n", len); + /* Fill with zero in case actual read is not implemented */ + chunk = malloc(len); + if (aop->read) { + reply.error = aop->read(chunk, len, from, userdata); + } else { + /* If user not specified read operation, return EPERM error */ + reply.error = htonl(EPERM); + } + write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); + write_all(sk, (char*)chunk, len); + + free(chunk); + break; + case NBD_CMD_WRITE: + fprintf(stderr, "Request for write of size %d\n", len); + chunk = malloc(len); + read_all(sk, chunk, len); + if (aop->write) { + reply.error = aop->write(chunk, len, from, userdata); + } else { + /* If user not specified write operation, return EPERM error */ + reply.error = htonl(EPERM); + } + free(chunk); + write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); + break; + case NBD_CMD_DISC: + /* Handle a disconnect request. */ + if (aop->disc) { + aop->disc(userdata); + } + return EXIT_SUCCESS; +#ifdef NBD_FLAG_SEND_FLUSH + case NBD_CMD_FLUSH: + if (aop->flush) { + reply.error = aop->flush(userdata); + } + write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); + break; +#endif +#ifdef NBD_FLAG_SEND_TRIM + case NBD_CMD_TRIM: + if (aop->trim) { + reply.error = aop->trim(from, len, userdata); + } + write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); + break; +#endif + default: + assert(0); + } + } + if (bytes_read == -1) { + fprintf(stderr, "%s\n", strerror(errno)); + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} + +int buse_main(const char* dev_file, const struct buse_operations *aop, void *userdata) +{ + int sp[2]; + int nbd, sk, err, tmp_fd; + err = socketpair(AF_UNIX, SOCK_STREAM, 0, sp); assert(!err); @@ -223,83 +305,6 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use close(tmp_fd); close(sp[1]); - sk = sp[0]; - reply.magic = htonl(NBD_REPLY_MAGIC); - reply.error = htonl(0); - - while ((bytes_read = read(sk, &request, sizeof(request))) > 0) { - assert(bytes_read == sizeof(request)); - memcpy(reply.handle, request.handle, sizeof(reply.handle)); - reply.error = htonl(0); - - len = ntohl(request.len); - from = ntohll(request.from); - assert(request.magic == htonl(NBD_REQUEST_MAGIC)); - - switch(ntohl(request.type)) { - /* I may at some point need to deal with the the fact that the - * official nbd server has a maximum buffer size, and divides up - * oversized requests into multiple pieces. This applies to reads - * and writes. - */ - case NBD_CMD_READ: - fprintf(stderr, "Request for read of size %d\n", len); - /* Fill with zero in case actual read is not implemented */ - chunk = malloc(len); - if (aop->read) { - reply.error = aop->read(chunk, len, from, userdata); - } else { - /* If user not specified read operation, return EPERM error */ - reply.error = htonl(EPERM); - } - write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); - write_all(sk, (char*)chunk, len); - - free(chunk); - break; - case NBD_CMD_WRITE: - fprintf(stderr, "Request for write of size %d\n", len); - chunk = malloc(len); - read_all(sk, chunk, len); - if (aop->write) { - reply.error = aop->write(chunk, len, from, userdata); - } else { - /* If user not specified write operation, return EPERM error */ - reply.error = htonl(EPERM); - } - free(chunk); - write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); - break; - case NBD_CMD_DISC: - /* Handle a disconnect request. */ - if (aop->disc) { - aop->disc(userdata); - } - return 0; -#ifdef NBD_FLAG_SEND_FLUSH - case NBD_CMD_FLUSH: - if (aop->flush) { - reply.error = aop->flush(userdata); - } - write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); - break; -#endif -#ifdef NBD_FLAG_SEND_TRIM - case NBD_CMD_TRIM: - if (aop->trim) { - reply.error = aop->trim(from, len, userdata); - } - write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); - break; -#endif - default: - assert(0); - } - } - if (bytes_read == -1) { - fprintf(stderr, "%s\n", strerror(errno)); - return EXIT_FAILURE; - } - return 0; + return serve_nbd(sp[0], aop, userdata); } From ad47f719b2585d867c794fd7a3d0aabb3076b662 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Mon, 18 Jun 2018 16:12:21 +0200 Subject: [PATCH 07/14] README description of how to terminate BUSE --- README.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/README.md b/README.md index 9eea09a..ca36210 100644 --- a/README.md +++ b/README.md @@ -34,6 +34,16 @@ start reading and writing files on it: mkfs.ext4 /dev/nbd0 mount /dev/nbd0 /mnt +BUSE should gracefuly disconnect from block device upon receiving SIGINT +or SIGTERM. However, if something goes wrong, block device is stuck in +unusable state and BUSE process exited or hung you can request +disconnect by: + + nbd-client -d /dev/nbd0 + +Actually this command performs clean disconnect and can also be used +to terminate running instance of BUSE. + ## Tests To perform checks you can run scripts in `test/` directory. They require: From b8bfacfbdba5ba68683614197e943b1e8ab8edaa Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Mon, 18 Jun 2018 17:52:41 +0200 Subject: [PATCH 08/14] Removed device test-open --- buse.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/buse.c b/buse.c index 444682e..e9ed5da 100644 --- a/buse.c +++ b/buse.c @@ -199,7 +199,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata int buse_main(const char* dev_file, const struct buse_operations *aop, void *userdata) { int sp[2]; - int nbd, sk, err, tmp_fd; + int nbd, sk, err; err = socketpair(AF_UNIX, SOCK_STREAM, 0, sp); assert(!err); @@ -296,14 +296,6 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use return EXIT_FAILURE; } - /* The parent opens the device file at least once, to make sure the - * partition table is updated. Then it closes it and starts serving up - * requests. */ - - tmp_fd = open(dev_file, O_RDONLY); - assert(tmp_fd != -1); - close(tmp_fd); - close(sp[1]); return serve_nbd(sp[0], aop, userdata); From caf7bd37eb4af633339ec46e2df111076f38a275 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Mon, 18 Jun 2018 18:56:40 +0200 Subject: [PATCH 09/14] Child process should exit instead of return --- buse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/buse.c b/buse.c index 444682e..ad7fed4 100644 --- a/buse.c +++ b/buse.c @@ -238,7 +238,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use sigprocmask(SIG_SETMASK, &sigset, NULL) != 0 ) { warn("failed to block signals in child"); - return EXIT_FAILURE; + exit(EXIT_FAILURE); } /* The child needs to continue setting things up. */ @@ -247,7 +247,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use if(ioctl(nbd, NBD_SET_SOCK, sk) == -1){ fprintf(stderr, "ioctl(nbd, NBD_SET_SOCK, sk) failed.[%s]\n", strerror(errno)); - return EXIT_FAILURE; + exit(EXIT_FAILURE); } #if defined NBD_SET_FLAGS && defined NBD_FLAG_SEND_TRIM else if(ioctl(nbd, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM) == -1){ @@ -259,7 +259,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use fprintf(stderr, "nbd device terminated with code %d\n", err); if (err == -1) { fprintf(stderr, "%s\n", strerror(errno)); - return EXIT_FAILURE; + exit(EXIT_FAILURE); } } @@ -268,7 +268,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use ioctl(nbd, NBD_CLEAR_SOCK) == -1 ) { fprintf(stderr, "failed to perform nbd cleanup actions: %s\n", strerror(errno)); - return EXIT_FAILURE; + exit(EXIT_FAILURE); } exit(0); From a5dc0a743a9829382e4033099e3cad75d2e6b3bd Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Mon, 18 Jun 2018 22:24:10 +0200 Subject: [PATCH 10/14] Parent process waits for child process to exit --- buse.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/buse.c b/buse.c index ad7fed4..d474bd2 100644 --- a/buse.c +++ b/buse.c @@ -32,6 +32,7 @@ #include #include #include +#include #include #include "buse.h" @@ -229,7 +230,8 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use err = ioctl(nbd, NBD_CLEAR_SOCK); assert(err != -1); - if (!fork()) { + pid_t pid = fork(); + if (pid == 0) { /* Block all signals to not get interrupted in ioctl(NBD_DO_IT), as * it seems there is no good way to handle such interruption.*/ sigset_t sigset; @@ -306,5 +308,20 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use close(sp[1]); - return serve_nbd(sp[0], aop, userdata); + /* serve NBD socket */ + int status; + status = serve_nbd(sp[0], aop, userdata); + if (close(sp[0]) != 0) warn("problem closing server side nbd socket"); + if (status != 0) return status; + + /* wait for subprocess */ + if (waitpid(pid, &status, 0) == -1) { + warn("waitpid failed"); + return EXIT_FAILURE; + } + if (WEXITSTATUS(status) != 0) { + return WEXITSTATUS(status); + } + + return EXIT_SUCCESS; } From 336cb77055edfe9008193cdd500ed6c86b8cf9c2 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Wed, 20 Jun 2018 00:44:28 +0200 Subject: [PATCH 11/14] Cleanup imports --- buse.c | 2 +- buse.h | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/buse.c b/buse.c index d017302..288b473 100644 --- a/buse.c +++ b/buse.c @@ -23,7 +23,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/buse.h b/buse.h index c3d379c..c3a08d2 100644 --- a/buse.h +++ b/buse.h @@ -4,11 +4,8 @@ #ifdef __cplusplus extern "C" { #endif - - /* Most of this file was copied from nbd.h in the nbd distribution. */ -#include + #include -#include struct buse_operations { int (*read)(void *buf, u_int32_t len, u_int64_t offset, void *userdata); From aeceb3a89d8dc203a951c37041dd8bca4aabf770 Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Fri, 22 Jun 2018 16:17:47 +0200 Subject: [PATCH 12/14] Debug switch --- Makefile | 2 +- README.md | 4 ++++ buse.c | 21 ++++++++++++++------- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 1cd4982..d86558d 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ OBJS := $(TARGET:=.o) $(LIBOBJS) STATIC_LIB := libbuse.a CC := /usr/bin/gcc -CFLAGS := -g -pedantic -Wall -Wextra -std=c99 +override CFLAGS += -g -pedantic -Wall -Wextra -std=c99 LDFLAGS := -L. -lbuse .PHONY: all clean test diff --git a/README.md b/README.md index ca36210..2bdd407 100644 --- a/README.md +++ b/README.md @@ -53,3 +53,7 @@ To perform checks you can run scripts in `test/` directory. They require: `make test` will run all test scripts with BUSE added to PATH and using sudo to grant permissions. + +To increase verbosity define `BUSE_DEBUG`. You can do this in make command: + + make test CFLAGS=-DBUSE_DEBUG diff --git a/buse.c b/buse.c index 288b473..04bbf7d 100644 --- a/buse.c +++ b/buse.c @@ -37,6 +37,10 @@ #include "buse.h" +#ifndef BUSE_DEBUG + #define BUSE_DEBUG (0) +#endif + /* * These helper functions were taken from cliserv.h in the nbd distribution. */ @@ -137,7 +141,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata * and writes. */ case NBD_CMD_READ: - fprintf(stderr, "Request for read of size %d\n", len); + if (BUSE_DEBUG) fprintf(stderr, "Request for read of size %d\n", len); /* Fill with zero in case actual read is not implemented */ chunk = malloc(len); if (aop->read) { @@ -152,7 +156,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata free(chunk); break; case NBD_CMD_WRITE: - fprintf(stderr, "Request for write of size %d\n", len); + if (BUSE_DEBUG) fprintf(stderr, "Request for write of size %d\n", len); chunk = malloc(len); read_all(sk, chunk, len); if (aop->write) { @@ -165,6 +169,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata write_all(sk, (char*)&reply, sizeof(struct nbd_reply)); break; case NBD_CMD_DISC: + if (BUSE_DEBUG) fprintf(stderr, "Got NBD_CMD_DISC\n"); /* Handle a disconnect request. */ if (aop->disc) { aop->disc(userdata); @@ -172,6 +177,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata return EXIT_SUCCESS; #ifdef NBD_FLAG_SEND_FLUSH case NBD_CMD_FLUSH: + if (BUSE_DEBUG) fprintf(stderr, "Got NBD_CMD_FLUSH\n"); if (aop->flush) { reply.error = aop->flush(userdata); } @@ -180,6 +186,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata #endif #ifdef NBD_FLAG_SEND_TRIM case NBD_CMD_TRIM: + if (BUSE_DEBUG) fprintf(stderr, "Got NBD_CMD_TRIM\n"); if (aop->trim) { reply.error = aop->trim(from, len, userdata); } @@ -191,7 +198,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata } } if (bytes_read == -1) { - fprintf(stderr, "%s\n", strerror(errno)); + warn("error reading userside of nbd socket"); return EXIT_FAILURE; } return EXIT_SUCCESS; @@ -209,7 +216,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use if (nbd == -1) { fprintf(stderr, "Failed to open `%s': %s\n" - "Is kernel module `nbd' is loaded and you have permissions " + "Is kernel module `nbd' loaded and you have permissions " "to access the device?\n", dev_file, strerror(errno)); return 1; } @@ -258,9 +265,9 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use #endif else{ err = ioctl(nbd, NBD_DO_IT); - fprintf(stderr, "nbd device terminated with code %d\n", err); + if (BUSE_DEBUG) fprintf(stderr, "nbd device terminated with code %d\n", err); if (err == -1) { - fprintf(stderr, "%s\n", strerror(errno)); + warn("NBD_DO_IT terminated with error"); exit(EXIT_FAILURE); } } @@ -269,7 +276,7 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use ioctl(nbd, NBD_CLEAR_QUE) == -1 || ioctl(nbd, NBD_CLEAR_SOCK) == -1 ) { - fprintf(stderr, "failed to perform nbd cleanup actions: %s\n", strerror(errno)); + warn("failed to perform nbd cleanup actions"); exit(EXIT_FAILURE); } From b58c1c11f6a1e093a9353102c2a8c9cdc735e65a Mon Sep 17 00:00:00 2001 From: Jan Rydzewski Date: Fri, 22 Jun 2018 19:10:14 +0200 Subject: [PATCH 13/14] busexmp cmdline argument parsing --- README.md | 4 +- busexmp.c | 132 ++++++++++++++++++++++++++++++------- test/busexmp.sh | 2 +- test/signal_termination.sh | 5 +- 4 files changed, 113 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index ca36210..bdca4a5 100644 --- a/README.md +++ b/README.md @@ -20,12 +20,12 @@ user. ## Running the Example Code -BUSE comes with an example driver in `busexmp.c` that implements a 128 MB +BUSE comes with an example driver in `busexmp.c` that implements a memory disk. To try out the example code, run `make` and then execute the following as root: modprobe nbd - ./busexmp /dev/nbd0 + ./busexmp 128M /dev/nbd0 You should then have an in-memory disk running, represented by the device file `/dev/nbd0`. You can create a file system on the virtual disk, mount it, and diff --git a/busexmp.c b/busexmp.c index d342a6e..57cd503 100644 --- a/busexmp.c +++ b/busexmp.c @@ -17,14 +17,16 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#include +#include #include #include #include #include "buse.h" +/* BUSE callbacks */ static void *data; -static int xmpl_debug = 1; static int xmp_read(void *buf, u_int32_t len, u_int64_t offset, void *userdata) { @@ -44,47 +46,127 @@ static int xmp_write(const void *buf, u_int32_t len, u_int64_t offset, void *use static void xmp_disc(void *userdata) { - (void)(userdata); - fprintf(stderr, "Received a disconnect request.\n"); + if (*(int *)userdata) + fprintf(stderr, "Received a disconnect request.\n"); } static int xmp_flush(void *userdata) { - (void)(userdata); - fprintf(stderr, "Received a flush request.\n"); + if (*(int *)userdata) + fprintf(stderr, "Received a flush request.\n"); return 0; } static int xmp_trim(u_int64_t from, u_int32_t len, void *userdata) { - (void)(userdata); - fprintf(stderr, "T - %lu, %u\n", from, len); + if (*(int *)userdata) + fprintf(stderr, "T - %lu, %u\n", from, len); return 0; } +/* argument parsing using argp */ -static struct buse_operations aop = { - .read = xmp_read, - .write = xmp_write, - .disc = xmp_disc, - .flush = xmp_flush, - .trim = xmp_trim, - .size = 128 * 1024 * 1024, +static struct argp_option options[] = { + {"verbose", 'v', 0, 0, "Produce verbose output", 0}, + {0}, }; -int main(int argc, char *argv[]) -{ - if (argc != 2) - { - fprintf(stderr, - "Usage:\n" - " %s /dev/nbd0\n" - "Don't forget to load nbd kernel module (`modprobe nbd`) and\n" - "run example from root.\n", argv[0]); - return 1; +struct arguments { + unsigned long long size; + char * device; + int verbose; +}; + +static unsigned long long strtoull_with_prefix(const char * str, char * * end) { + unsigned long long v = strtoull(str, end, 0); + switch (**end) { + case 'K': + v *= 1024; + *end += 1; + break; + case 'M': + v *= 1024 * 1024; + *end += 1; + break; + case 'G': + v *= 1024 * 1024 * 1024; + *end += 1; + break; + } + return v; +} + +/* Parse a single option. */ +static error_t parse_opt(int key, char *arg, struct argp_state *state) { + struct arguments *arguments = state->input; + char * endptr; + + switch (key) { + + case 'v': + arguments->verbose = 1; + break; + + case ARGP_KEY_ARG: + switch (state->arg_num) { + + case 0: + arguments->size = strtoull_with_prefix(arg, &endptr); + if (*endptr != '\0') { + /* failed to parse integer */ + errx(EXIT_FAILURE, "SIZE must be an integer"); + } + break; + + case 1: + arguments->device = arg; + break; + + default: + /* Too many arguments. */ + return ARGP_ERR_UNKNOWN; + } + break; + + case ARGP_KEY_END: + if (state->arg_num < 2) { + warnx("not enough arguments"); + argp_usage(state); + } + break; + + default: + return ARGP_ERR_UNKNOWN; } + return 0; +} + +static struct argp argp = { + .options = options, + .parser = parse_opt, + .args_doc = "SIZE DEVICE", + .doc = "BUSE virtual block device that stores its content in memory.\n" + "`SIZE` accepts suffixes K, M, G. `DEVICE` is path to block device, for example \"/dev/nbd0\".", +}; + + +int main(int argc, char *argv[]) { + struct arguments arguments = { + .verbose = 0, + }; + argp_parse(&argp, argc, argv, 0, 0, &arguments); + + struct buse_operations aop = { + .read = xmp_read, + .write = xmp_write, + .disc = xmp_disc, + .flush = xmp_flush, + .trim = xmp_trim, + .size = arguments.size, + }; data = malloc(aop.size); + if (data == NULL) err(EXIT_FAILURE, "failed to alloc space for data"); - return buse_main(argv[1], &aop, (void *)&xmpl_debug); + return buse_main(arguments.device, &aop, (void *)&arguments.verbose); } diff --git a/test/busexmp.sh b/test/busexmp.sh index 5dc3e75..e72f846 100755 --- a/test/busexmp.sh +++ b/test/busexmp.sh @@ -29,7 +29,7 @@ TESTFILE=$(mktemp) $DD if=/dev/urandom of="$TESTFILE" bs=16M count=1 # attach BUSE device -busexmp "$BLOCKDEV" & +busexmp 128M "$BLOCKDEV" & sleep 1 BUSEPID=$! diff --git a/test/signal_termination.sh b/test/signal_termination.sh index f144e23..c14916e 100755 --- a/test/signal_termination.sh +++ b/test/signal_termination.sh @@ -2,6 +2,7 @@ set -e BLOCKDEV=/dev/nbd0 +SIZE=16M # verify if blockdev is not currently in use set +e @@ -19,7 +20,7 @@ function cleanup () { trap cleanup EXIT # attach BUSE device -busexmp "$BLOCKDEV" & +busexmp "$SIZE" "$BLOCKDEV" & BUSEPID=$! # wait a bit ensure buse is running and connected to device @@ -31,7 +32,7 @@ kill -s SIGTERM $BUSEPID wait $BUSEPID # attach BUSE again to verify if device is left in usable state -busexmp "$BLOCKDEV" & +busexmp "$SIZE" "$BLOCKDEV" & BUSEPID=$! sleep 1 nbd-client -c "$BLOCKDEV" > /dev/null From 6cd937f733a220a9d1e8ae048e9719f822866121 Mon Sep 17 00:00:00 2001 From: Alberto Faria Date: Thu, 28 Jun 2018 16:44:41 +0100 Subject: [PATCH 14/14] Flush support is now reported to the NBD client. --- buse.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/buse.c b/buse.c index 04bbf7d..b8bb02a 100644 --- a/buse.c +++ b/buse.c @@ -207,7 +207,7 @@ static int serve_nbd(int sk, const struct buse_operations * aop, void * userdata int buse_main(const char* dev_file, const struct buse_operations *aop, void *userdata) { int sp[2]; - int nbd, sk, err; + int nbd, sk, err, flags; err = socketpair(AF_UNIX, SOCK_STREAM, 0, sp); assert(!err); @@ -258,12 +258,20 @@ int buse_main(const char* dev_file, const struct buse_operations *aop, void *use fprintf(stderr, "ioctl(nbd, NBD_SET_SOCK, sk) failed.[%s]\n", strerror(errno)); exit(EXIT_FAILURE); } -#if defined NBD_SET_FLAGS && defined NBD_FLAG_SEND_TRIM - else if(ioctl(nbd, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM) == -1){ - fprintf(stderr, "ioctl(nbd, NBD_SET_FLAGS, NBD_FLAG_SEND_TRIM) failed.[%s]\n", strerror(errno)); - } -#endif else{ +#if defined NBD_SET_FLAGS + flags = 0; +#if defined NBD_FLAG_SEND_TRIM + flags |= NBD_FLAG_SEND_TRIM; +#endif +#if defined NBD_FLAG_SEND_FLUSH + flags |= NBD_FLAG_SEND_FLUSH; +#endif + if (flags != 0 && ioctl(nbd, NBD_SET_FLAGS, flags) == -1){ + fprintf(stderr, "ioctl(nbd, NBD_SET_FLAGS, %d) failed.[%s]\n", flags, strerror(errno)); + exit(EXIT_FAILURE); + } +#endif err = ioctl(nbd, NBD_DO_IT); if (BUSE_DEBUG) fprintf(stderr, "nbd device terminated with code %d\n", err); if (err == -1) {