Richard W.M. Jones
2020-Nov-05 14:38 UTC
[Libguestfs] [PATCH libnbd] copy: Allowing copying from NBD server to NBD server.
This patch is a straightforward refactoring of libnbd?s nbdcopy program, and not very interesting. However I have plans for nbdcopy (see full todo below). I would like to use this utility for virt-v2v as a replacement for ?qemu-img convert?. qemu-img has caused us a series of problems: - change in zeroing behaviour caused a big performance regression - qemu-img reads extents up-front which is problematic when reading from VDDK because the progress counter sticks at 0% for ages - no support for NBD multi-conn, leaving a lot of performance on the table - inflexible progress bar The idea is to make nbdcopy into a very customizable and highly efficient copying tool [it obviously is not right now]. Rich. ---------------------------------------------------------------------- progress bar - write to fd - machine-readable threads - how many copying threads do we create read order, write order - allow reads/writes in order or out of order multi-conn - control it separately at either end extents - do we check them at all? - do we check them before or as we go along? (probably best to always check as we go along?) - do we zero up front or as we go along? (best always as we go along?) synchronous - some kind of deliberately linear mode, eg for pipes - with FUA on writes --from / --to - send data from/to an NBD server as subprocess - can we do this? block size - force requests to be aligned to block size support for copying ranges - offset and size - in a later version, since this is hard to get right io_uring(?) - in nbdcopy - in libnbd - later version - hard, needs benchmarking to prove it is worthwhile
Richard W.M. Jones
2020-Nov-05 14:38 UTC
[Libguestfs] [PATCH libnbd] copy: Allowing copying from NBD server to NBD server.
This is a general refactoring of nbdcopy in preparation for future work, incidentally adding the feature of copying between NBD servers which was missing before. We abstract the source and destination into structures that can describe either a local (file/device/pipe/stdio) or an NBD server. The copy routine can copy between generic source and destination instead of having separate upload and download functions. --- copy/Makefile.am | 2 + copy/copy-nbd-to-nbd.sh | 59 +++++ copy/nbdcopy.c | 488 ++++++++++++++++++++++++---------------- copy/nbdcopy.pod | 8 +- 4 files changed, 366 insertions(+), 191 deletions(-) diff --git a/copy/Makefile.am b/copy/Makefile.am index f09f860..16d6fb2 100644 --- a/copy/Makefile.am +++ b/copy/Makefile.am @@ -22,6 +22,7 @@ EXTRA_DIST = \ copy-file-to-nbd.sh \ copy-nbd-to-block.sh \ copy-nbd-to-file.sh \ + copy-nbd-to-nbd.sh \ copy-nbd-to-small-block-error.sh \ copy-nbd-to-stdout.sh \ copy-stdin-to-nbd.sh \ @@ -63,6 +64,7 @@ ROOT_TESTS = \ TESTS += \ copy-file-to-nbd.sh \ copy-nbd-to-file.sh \ + copy-nbd-to-nbd.sh \ copy-stdin-to-nbd.sh \ copy-nbd-to-stdout.sh \ $(ROOT_TESTS) \ diff --git a/copy/copy-nbd-to-nbd.sh b/copy/copy-nbd-to-nbd.sh new file mode 100755 index 0000000..7732fe1 --- /dev/null +++ b/copy/copy-nbd-to-nbd.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# nbd client library in userspace +# Copyright (C) 2020 Red Hat Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +. ../tests/functions.sh + +set -e +set -x + +requires nbdkit --exit-with-parent --version +requires stat --version + +pidfile1=copy-nbd-to-nbd.pid1 +pidfile2=copy-nbd-to-nbd.pid2 +sock1=$(mktemp -u /tmp/libnbd-test-copy.XXXXXX) +sock2=$(mktemp -u /tmp/libnbd-test-copy.XXXXXX) +cleanup_fn rm -f $pidfile1 $pidfile2 $sock1 $sock2 + +nbdkit --exit-with-parent -f -v -P $pidfile1 -U $sock1 pattern size=10M & +# Wait for the pidfile to appear. +for i in {1..60}; do + if test -f $pidfile1; then + break + fi + sleep 1 +done +if ! test -f $pidfile1; then + echo "$0: nbdkit did not start up" + exit 1 +fi + +nbdkit --exit-with-parent -f -v -P $pidfile2 -U $sock2 memory size=10M & +# Wait for the pidfile to appear. +for i in {1..60}; do + if test -f $pidfile2; then + break + fi + sleep 1 +done +if ! test -f $pidfile2; then + echo "$0: nbdkit did not start up" + exit 1 +fi + +$VG nbdcopy "nbd+unix:///?socket=$sock1" "nbd+unix:///?socket=$sock2" diff --git a/copy/nbdcopy.c b/copy/nbdcopy.c index e7a7fe0..f583100 100644 --- a/copy/nbdcopy.c +++ b/copy/nbdcopy.c @@ -35,14 +35,28 @@ #define MAX_REQUEST_SIZE (32 * 1024 * 1024) -static bool progress; +static bool progress; /* -p flag */ -static void upload (const char *filename, int fd, - struct stat *filestat, off_t filesize, - struct nbd_handle *nbd); -static void download (struct nbd_handle *nbd, - const char *filename, int fd, - struct stat *filestat, off_t filesize); +/* The source and destination. */ +static struct rw { + enum { LOCAL, NBD } t; + const char *name; /* Printable name, for error messages etc. */ + union { + struct { /* For LOCAL. */ + int fd; + struct stat stat; + off_t size; /* May be -1 for pipes, stdio, etc. */ + } local; + struct nbd_handle *nbd; /* For NBD, the libnbd handle. */ + } u; +} src, dst; + +static bool is_nbd_uri (const char *s); +static int open_local (const char *prog, + const char *filename, bool writing, + struct stat *statbuf, off_t *size_rtn); +static int64_t size (const struct rw *rw); +static void copying (void); static void progress_bar (off_t pos, int64_t size); static void __attribute__((noreturn)) @@ -69,89 +83,6 @@ display_version (void) printf ("%s %s\n", PACKAGE_NAME, PACKAGE_VERSION); } -static bool -is_nbd_uri (const char *s) -{ - return - strncmp (s, "nbd:", 4) == 0 || - strncmp (s, "nbds:", 5) == 0 || - strncmp (s, "nbd+unix:", 9) == 0 || - strncmp (s, "nbds+unix:", 10) == 0 || - strncmp (s, "nbd+vsock:", 10) == 0 || - strncmp (s, "nbds+vsock:", 11) == 0; -} - -static int -open_local (const char *prog, - const char *filename, bool writing, - struct stat *statbuf, off_t *size_rtn) -{ - int flags, fd; - - if (strcmp (filename, "-") == 0) { - fd = writing ? STDOUT_FILENO : STDIN_FILENO; - if (writing && isatty (fd)) { - fprintf (stderr, "%s: refusing to write to tty\n", prog); - exit (EXIT_FAILURE); - } - } - else { - /* If it's a block device and we're writing we don't want to turn - * it into a truncated regular file by accident, so try to open - * without O_CREAT first. - */ - flags = writing ? O_WRONLY : O_RDONLY; - fd = open (filename, flags); - if (fd == -1) { - if (writing) { - /* Try again, with more flags. */ - flags |= O_TRUNC|O_CREAT|O_EXCL; - fd = open (filename, flags, 0644); - } - if (fd == -1) { - perror (filename); - exit (EXIT_FAILURE); - } - } - } - - if (fstat (fd, statbuf) == -1) { - perror (filename); - exit (EXIT_FAILURE); - } - if (S_ISBLK (statbuf->st_mode)) { - /* Block device. */ - *size_rtn = lseek (fd, 0, SEEK_END); - if (*size_rtn == -1) { - perror ("lseek"); - exit (EXIT_FAILURE); - } - if (lseek (fd, 0, SEEK_SET) == -1) { - perror ("lseek"); - exit (EXIT_FAILURE); - } - } - else if (S_ISREG (statbuf->st_mode)) { - /* Reguar file. */ - *size_rtn = statbuf->st_size; - if (writing) { - /* Truncate the file since we might not have done that above. */ - if (ftruncate (fd, 0) == -1) { - perror ("truncate"); - exit (EXIT_FAILURE); - } - } - } - else { - /* Probably stdin/stdout, a pipe or a socket. Set *size_rtn == -1 - * which means don't know. - */ - *size_rtn = -1; - } - - return fd; -} - int main (int argc, char *argv[]) { @@ -169,13 +100,10 @@ main (int argc, char *argv[]) { "version", no_argument, NULL, 'V' }, { NULL } }; - int c, fd; + int c; size_t i; - const char *src, *dst; + const char *src_arg, *dst_arg; bool src_is_uri, dst_is_uri; - struct nbd_handle *nbd; - struct stat filestat; - off_t filesize; for (;;) { c = getopt_long (argc, argv, short_options, long_options, NULL); @@ -218,25 +146,14 @@ main (int argc, char *argv[]) if (argc - optind != 2) usage (stderr, EXIT_FAILURE); - src = argv[optind]; - dst = argv[optind+1]; + src_arg = argv[optind]; + dst_arg = argv[optind+1]; + src_is_uri = is_nbd_uri (src_arg); + dst_is_uri = is_nbd_uri (dst_arg); - /* Currently you cannot use this tool to copy from NBD to NBD - * although we may add this in future. - */ - src_is_uri = is_nbd_uri (src); - dst_is_uri = is_nbd_uri (dst); - if (src_is_uri && dst_is_uri) { - fprintf (stderr, - "%s: currently this tool does not allow you to copy between\n" - "NBD servers. Use: nbdcopy URI - | nbdcopy - URI instead. This\n" - "restriction may be removed in a future version.\n", - argv[0]); - exit (EXIT_FAILURE); - } - - /* Prevent copying between local files or devices. There are - * better ways to do that. + /* Prevent copying between local files or devices. It's unlikely + * this program will ever be better than highly tuned utilities like + * cp. */ if (!src_is_uri && !dst_is_uri) { fprintf (stderr, @@ -246,117 +163,312 @@ main (int argc, char *argv[]) exit (EXIT_FAILURE); } - /* Open the NBD side. */ - nbd = nbd_create (); - if (nbd == NULL) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); + /* Set up the source side. */ + src.name = src_arg; + if (!src_is_uri) { + src.t = LOCAL; + src.u.local.fd = open_local (argv[0], src_arg, false, + &src.u.local.stat, &src.u.local.size); } - nbd_set_uri_allow_local_file (nbd, true); /* Allow ?tls-psk-file. */ - - if (nbd_connect_uri (nbd, src_is_uri ? src : dst) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); + else { + src.t = NBD; + src.u.nbd = nbd_create (); + if (src.u.nbd == NULL) { + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_set_uri_allow_local_file (src.u.nbd, true); /* Allow ?tls-psk-file. */ + if (nbd_connect_uri (src.u.nbd, src_arg) == -1) { + fprintf (stderr, "%s: %s: %s\n", argv[0], src_arg, nbd_get_error ()); + exit (EXIT_FAILURE); + } } - /* Open the local file side. */ - fd = open_local (argv[0], src_is_uri ? dst : src, - src_is_uri /* writing */, - &filestat, &filesize); + /* Set up the destination side. */ + dst.name = dst_arg; + if (!dst_is_uri) { + dst.t = LOCAL; + dst.u.local.fd = open_local (argv[0], dst_arg, true /* writing */, + &dst.u.local.stat, &dst.u.local.size); + } + else { + dst.t = NBD; + dst.u.nbd = nbd_create (); + if (dst.u.nbd == NULL) { + fprintf (stderr, "%s: %s\n", argv[0], nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_set_uri_allow_local_file (dst.u.nbd, true); /* Allow ?tls-psk-file. */ + if (nbd_connect_uri (dst.u.nbd, dst_arg) == -1) { + fprintf (stderr, "%s: %s: %s\n", argv[0], dst_arg, nbd_get_error ()); + exit (EXIT_FAILURE); + } + /* Obviously this is not going to work if the server is + * advertising read-only, so fail early with a nice error message. + */ + if (nbd_is_read_only (dst.u.nbd)) { + fprintf (stderr, "%s: %s: " + "this NBD server is read-only, cannot write to it\n", + argv[0], dst_arg); + exit (EXIT_FAILURE); + } + } - /* Begin the operation. */ - if (dst_is_uri) - upload (src, fd, &filestat, filesize, nbd); - else - download (nbd, dst, fd, &filestat, filesize); + /* Start copying. */ + copying (); - if (nbd_shutdown (nbd, 0) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); + /* Shut down the source side. */ + if (src.t == LOCAL) { + if (close (src.u.local.fd) == -1) { + fprintf (stderr, "%s: %s: close: %m\n", argv[0], src_arg); + exit (EXIT_FAILURE); + } + } + else { + if (nbd_shutdown (src.u.nbd, 0) == -1) { + fprintf (stderr, "%s: %s: %s\n", argv[0], src_arg, nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_close (src.u.nbd); } - nbd_close (nbd); - if (close (fd) == -1) { - perror ("close"); - exit (EXIT_FAILURE); + /* Shut down the destination side. */ + if (dst.t == LOCAL) { + if (close (dst.u.local.fd) == -1) { + fprintf (stderr, "%s: %s: close: %m\n", argv[0], dst_arg); + exit (EXIT_FAILURE); + } + } + else { + if (nbd_shutdown (dst.u.nbd, 0) == -1) { + fprintf (stderr, "%s: %s: %s\n", argv[0], dst_arg, nbd_get_error ()); + exit (EXIT_FAILURE); + } + nbd_close (dst.u.nbd); } exit (EXIT_SUCCESS); } +/* Return true if the parameter is an NBD URI. */ +static bool +is_nbd_uri (const char *s) +{ + return + strncmp (s, "nbd:", 4) == 0 || + strncmp (s, "nbds:", 5) == 0 || + strncmp (s, "nbd+unix:", 9) == 0 || + strncmp (s, "nbds+unix:", 10) == 0 || + strncmp (s, "nbd+vsock:", 10) == 0 || + strncmp (s, "nbds+vsock:", 11) == 0; +} + +/* Open a local (non-NBD) file, ie. a file, device, or "-" for stdio. + * Returns the open file descriptor which the caller must close. + * + * ?writing? is true if this is the destination parameter. ?statbuf? + * and ?size_rtn? return the file stat and size, but size_rtn can be + * returned as -1 if we don't know the size (if it's a pipe or stdio). + */ +static int +open_local (const char *prog, + const char *filename, bool writing, + struct stat *statbuf, off_t *size_rtn) +{ + int flags, fd; + + if (strcmp (filename, "-") == 0) { + fd = writing ? STDOUT_FILENO : STDIN_FILENO; + if (writing && isatty (fd)) { + fprintf (stderr, "%s: refusing to write to tty\n", prog); + exit (EXIT_FAILURE); + } + } + else { + /* If it's a block device and we're writing we don't want to turn + * it into a truncated regular file by accident, so try to open + * without O_CREAT first. + */ + flags = writing ? O_WRONLY : O_RDONLY; + fd = open (filename, flags); + if (fd == -1) { + if (writing) { + /* Try again, with more flags. */ + flags |= O_TRUNC|O_CREAT|O_EXCL; + fd = open (filename, flags, 0644); + } + if (fd == -1) { + perror (filename); + exit (EXIT_FAILURE); + } + } + } + + if (fstat (fd, statbuf) == -1) { + perror (filename); + exit (EXIT_FAILURE); + } + if (S_ISBLK (statbuf->st_mode)) { + /* Block device. */ + *size_rtn = lseek (fd, 0, SEEK_END); + if (*size_rtn == -1) { + perror ("lseek"); + exit (EXIT_FAILURE); + } + if (lseek (fd, 0, SEEK_SET) == -1) { + perror ("lseek"); + exit (EXIT_FAILURE); + } + } + else if (S_ISREG (statbuf->st_mode)) { + /* Reguar file. + * + * Note if we're writing to the file we're going to truncate it, + * so the original file size is not meaningful. + */ + *size_rtn = writing ? -1 : statbuf->st_size; + if (writing) { + /* Truncate the file since we might not have done that above. */ + if (ftruncate (fd, 0) == -1) { + perror ("truncate"); + exit (EXIT_FAILURE); + } + } + } + else { + /* Probably stdin/stdout, a pipe or a socket. Set *size_rtn == -1 + * which means don't know. + */ + *size_rtn = -1; + } + + return fd; +} + +/* Return the size of the struct rw, if known. */ +static int64_t +size (const struct rw *rw) +{ + int64_t ret; + + switch (rw->t) { + case LOCAL: + ret = rw->u.local.size; + break; + + case NBD: + ret = nbd_get_size (rw->u.nbd); + if (ret == -1) { + fprintf (stderr, "%s: %s\n", rw->name, nbd_get_error ()); + exit (EXIT_FAILURE); + } + break; + + default: abort (); + } + + return ret; +} + +/* The loop that does the actual copying from src to dst. */ static char buf[MAX_REQUEST_SIZE]; +static size_t read_src (void *buf, size_t len, + off_t pos, int64_t src_size); +static void write_dst (const void *buf, size_t len, + off_t pos, int64_t dst_size); + static void -upload (const char *filename, int fd, struct stat *filestat, off_t filesize, - struct nbd_handle *nbd) +copying (void) { + int64_t src_size, dst_size; off_t pos = 0; - ssize_t r; + size_t r; - while ((r = read (fd, buf, sizeof buf)) > 0) { - if (nbd_pwrite (nbd, buf, r, pos, 0) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } + /* Check if the source is bigger than the destination, since that + * would truncate (ie. lose) data. Copying from smaller to larger + * is OK. + */ + src_size = size (&src); + dst_size = size (&dst); + if (src_size >= 0 && dst_size >= 0 && src_size > dst_size) { + fprintf (stderr, + "nbdcopy: error: destination size is smaller than source size\n"); + exit (EXIT_FAILURE); + } + + while ((r = read_src (buf, sizeof buf, pos, src_size)) > 0) { + write_dst (buf, r, pos, dst_size); pos += r; if (progress) - progress_bar (pos, (int64_t) filesize); + progress_bar (pos, dst_size); } - if (r == -1) { - perror (filename); - exit (EXIT_FAILURE); - } - if (progress) progress_bar (1, 1); } +/* Read from src up to len bytes into buf. Returns 0 if we've reached + * the end of the input. This exits on failure. + */ +static size_t +read_src (void *buf, size_t len, off_t pos, int64_t src_size) +{ + ssize_t r; + + switch (src.t) { + case LOCAL: + r = read (src.u.local.fd, buf, len); + if (r == -1) { + perror (src.name); + exit (EXIT_FAILURE); + } + return r; + + case NBD: + if (len > src_size - pos) len = src_size - pos; + if (len == 0) + return 0; + if (nbd_pread (src.u.nbd, buf, len, pos, 0) == -1) { + fprintf (stderr, "%s: %s\n", src.name, nbd_get_error ()); + exit (EXIT_FAILURE); + } + return len; + + default: abort (); + } +} + +/* Write buf to dst. This exits on failure. */ static void -download (struct nbd_handle *nbd, - const char *filename, int fd, struct stat *filestat, off_t filesize) +write_dst (const void *buf, size_t len, off_t pos, int64_t dst_size) { - int64_t size; - off_t pos = 0; - size_t n; - char *p; ssize_t r; - size = nbd_get_size (nbd); - if (size == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } - - if (S_ISBLK (filestat->st_mode) && filesize != -1 && size > filesize) { - fprintf (stderr, - "nbdcopy: block device is smaller than NBD source device\n"); - exit (EXIT_FAILURE); - } - - while (pos < size) { - p = buf; - n = sizeof buf; - if (n > size-pos) n = size-pos; - if (nbd_pread (nbd, p, n, pos, 0) == -1) { - fprintf (stderr, "%s\n", nbd_get_error ()); - exit (EXIT_FAILURE); - } - while (n > 0) { - r = write (fd, p, n); + switch (dst.t) { + case LOCAL: + while (len > 0) { + r = write (dst.u.local.fd, buf, len); if (r == -1) { - perror (filename); + perror (dst.name); exit (EXIT_FAILURE); } - p += r; - n -= r; + buf += r; + len -= r; pos += r; if (progress) - progress_bar (pos, size); + progress_bar (pos, dst_size); } + break; + + case NBD: + if (nbd_pwrite (dst.u.nbd, buf, len, pos, 0) == -1) { + fprintf (stderr, "%s: %s\n", dst.name, nbd_get_error ()); + exit (EXIT_FAILURE); + } + break; + + default: abort (); } - - if (progress) - progress_bar (1, 1); } /* Display the progress bar. */ diff --git a/copy/nbdcopy.pod b/copy/nbdcopy.pod index aa3f970..3b237c0 100644 --- a/copy/nbdcopy.pod +++ b/copy/nbdcopy.pod @@ -4,9 +4,9 @@ nbdcopy - copy to and from an NBD server =head1 SYNOPSIS - nbdcopy [-p] -|FILE|DEVICE NBD-URI + nbdcopy [-p] SOURCE DESTINATION - nbdcopy [-p] NBD-URI -|FILE|DEVICE + SOURCE, DESTINATION := - | FILE | DEVICE | NBD-URI nbdcopy --help @@ -22,11 +22,13 @@ nbdcopy - copy to and from an NBD server cat disk1 disk2 | nbdcopy - "nbd+unix:///?socket=/tmp/unixsock" + nbdcopy nbd://server1 nbd://server2 + =head1 DESCRIPTION nbdcopy copies to and from an NBD server. It can upload a local file to an NBD server, or download the contents of an NBD server to a local -file, device or stdin/stdout. +file, device or stdin/stdout. It can also copy between NBD servers. The local file can be a file, a block device (eg. C</dev/cdrom>), or C<-> which means stream in from stdin or stream out to stdout. The -- 2.29.0.rc2
Apparently Analagous Threads
- [PATCH -next 0/7] drm: Remove many unnecessary NULL values
- [PATCH 0/3] protocol: Abstract out socket operations.
- [libnbd PATCH v2 0/3] copy: wrap source code at 80 characters
- [PATCH libnbd 0/5] copy: Allow human sizes for --queue-size, etc
- Issue with downloading files whose path contains multi-byte utf-8 characters