[libvirt] [PATCH 0/6] v3: Allow data upload/download to/from storage volumes
An update of http://www.redhat.com/archives/libvir-list/2011-March/msg00880.html Addressing all the feedback from v2 and this time with the correct set of patches!
The O_NONBLOCK flag doesn't work as desired on plain files or block devices. Introduce an I/O helper program that does the blocking I/O operations, communicating over a pipe that can support O_NONBLOCK * src/fdstream.c, src/fdstream.h: Add non-blocking I/O on plain files/block devices * src/Makefile.am, src/util/iohelper.c: I/O helper program * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c, src/uml/uml_driver.c, src/xen/xen_driver.c: Update for streams API change --- po/POTFILES.in | 1 + src/Makefile.am | 12 +++ src/fdstream.c | 233 ++++++++++++++++++++++++++++++++++++------------ src/fdstream.h | 5 + src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/iohelper.c | 203 +++++++++++++++++++++++++++++++++++++++++ src/xen/xen_driver.c | 2 +- 9 files changed, 402 insertions(+), 60 deletions(-) create mode 100644 src/util/iohelper.c diff --git a/po/POTFILES.in b/po/POTFILES.in index 805e5ca..12adb3e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -94,6 +94,7 @@ src/util/event_poll.c src/util/hash.c src/util/hooks.c src/util/hostusb.c +src/util/iohelper.c src/util/interface.c src/util/iptables.c src/util/json.c diff --git a/src/Makefile.am b/src/Makefile.am index c3729a6..1d8115b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES = \ STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c +UTIL_IO_HELPER_SOURCES = \ + util/iohelper.c + # Network filters NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) libexec_PROGRAMS = +libexec_PROGRAMS += libvirt_iohelper +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES) +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) +libvirt_iohelper_LDADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + +libvirt_iohelper_CFLAGS = $(AM_CFLAGS) + if WITH_STORAGE_DISK if WITH_LIBVIRTD libexec_PROGRAMS += libvirt_parthelper diff --git a/src/fdstream.c b/src/fdstream.c index 701fafc..41e2db6 100644 --- a/src/fdstream.c +++ b/src/fdstream.c @@ -30,14 +30,17 @@ # include <sys/un.h> #endif #include <netinet/in.h> +#include <signal.h> #include "fdstream.h" #include "virterror_internal.h" #include "datatypes.h" +#include "logging.h" #include "memory.h" #include "event.h" #include "util.h" #include "files.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STREAMS #define streamsReportError(code, ...) \ @@ -47,6 +50,10 @@ /* Tunnelled migration stream support */ struct virFDStreamData { int fd; + int errfd; + virCommandPtr cmd; + unsigned long long offset; + unsigned long long length; int watch; unsigned int cbRemoved; @@ -206,6 +213,35 @@ static int virFDStreamFree(struct virFDStreamData *fdst) { int ret; ret = VIR_CLOSE(fdst->fd); + if (fdst->cmd) { + char buf[1024]; + ssize_t len; + int status; + if ((len = saferead(fdst->errfd, buf, sizeof(buf)-1)) < 0) + buf[0] = '\0'; + else + buf[len] = '\0'; + + if (virCommandWait(fdst->cmd, &status) < 0) { + ret = -1; + } else if (status != 0) { + if (buf[0] == '\0') { + if (WIFEXITED(status)) { + streamsReportError(VIR_ERR_INTERNAL_ERROR, + _("I/O helper exited with status %d"), + WEXITSTATUS(status)); + } else { + streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("I/O helper exited abnormally")); + } + } else { + streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", + buf); + } + ret = -1; + } + virCommandFree(fdst->cmd); + } VIR_FREE(fdst); return ret; } @@ -217,6 +253,8 @@ virFDStreamClose(virStreamPtr st) struct virFDStreamData *fdst = st->privateData; int ret; + VIR_DEBUG("st=%p", st); + if (!fdst) return 0; @@ -250,6 +288,18 @@ static int virFDStreamWrite(virStreamPtr st, const char *bytes, size_t nbytes) virMutexLock(&fdst->lock); + if (fdst->length) { + if (fdst->length == fdst->offset) { + virReportSystemError(ENOSPC, "%s", + _("cannot write to stream")); + virMutexUnlock(&fdst->lock); + return -1; + } + + if ((fdst->length - fdst->offset) < nbytes) + nbytes = fdst->length - fdst->offset; + } + retry: ret = write(fdst->fd, bytes, nbytes); if (ret < 0) { @@ -262,6 +312,8 @@ retry: virReportSystemError(errno, "%s", _("cannot write to stream")); } + } else if (fdst->length) { + fdst->offset += ret; } virMutexUnlock(&fdst->lock); @@ -288,6 +340,16 @@ static int virFDStreamRead(virStreamPtr st, char *bytes, size_t nbytes) virMutexLock(&fdst->lock); + if (fdst->length) { + if (fdst->length == fdst->offset) { + virMutexUnlock(&fdst->lock); + return 0; + } + + if ((fdst->length - fdst->offset) < nbytes) + nbytes = fdst->length - fdst->offset; + } + retry: ret = read(fdst->fd, bytes, nbytes); if (ret < 0) { @@ -300,6 +362,8 @@ retry: virReportSystemError(errno, "%s", _("cannot read from stream")); } + } else if (fdst->length) { + fdst->offset += ret; } virMutexUnlock(&fdst->lock); @@ -317,11 +381,17 @@ static virStreamDriver virFDStreamDrv = { .streamRemoveCallback = virFDStreamRemoveCallback }; -int virFDStreamOpen(virStreamPtr st, - int fd) +static int virFDStreamOpenInternal(virStreamPtr st, + int fd, + virCommandPtr cmd, + int errfd, + unsigned long long length) { struct virFDStreamData *fdst; + VIR_DEBUG("st=%p fd=%d cmd=%p errfd=%d length=%llu", + st, fd, cmd, errfd, length); + if ((st->flags & VIR_STREAM_NONBLOCK) && virSetNonBlock(fd) < 0) return -1; @@ -332,6 +402,9 @@ int virFDStreamOpen(virStreamPtr st, } fdst->fd = fd; + fdst->cmd = cmd; + fdst->errfd = errfd; + fdst->length = length; if (virMutexInit(&fdst->lock) < 0) { VIR_FREE(fdst); streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -346,6 +419,13 @@ int virFDStreamOpen(virStreamPtr st, } +int virFDStreamOpen(virStreamPtr st, + int fd) +{ + return virFDStreamOpenInternal(st, fd, NULL, -1, 0); +} + + #if HAVE_SYS_UN_H int virFDStreamConnectUNIX(virStreamPtr st, const char *path, @@ -387,7 +467,7 @@ int virFDStreamConnectUNIX(virStreamPtr st, goto error; } while ((++i <= timeout*5) && (usleep(.2 * 1000000) <= 0)); - if (virFDStreamOpen(st, fd) < 0) + if (virFDStreamOpenInternal(st, fd, NULL, -1, 0) < 0) goto error; return 0; @@ -406,19 +486,29 @@ int virFDStreamConnectUNIX(virStreamPtr st ATTRIBUTE_UNUSED, } #endif -int virFDStreamOpenFile(virStreamPtr st, - const char *path, - int flags) +static int +virFDStreamOpenFileInternal(virStreamPtr st, + const char *path, + unsigned long long offset, + unsigned long long length, + int flags, + int mode) { - int fd; + int fd = -1; + int fds[2] = { -1, -1 }; struct stat sb; + virCommandPtr cmd = NULL; + int errfd = -1; + pid_t pid = 0; - if (flags & O_CREAT) { - streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Unexpected O_CREAT flag when opening existing file")); - } + VIR_DEBUG("st=%p path=%s flags=%d offset=%llu length=%llu mode=%d", + st, path, flags, offset, length, mode); - if ((fd = open(path, flags)) < 0) { + if (flags & O_CREAT) + fd = open(path, flags, mode); + else + fd = open(path, flags); + if (fd < 0) { virReportSystemError(errno, _("Unable to open stream for '%s'"), path); @@ -440,64 +530,95 @@ int virFDStreamOpenFile(virStreamPtr st, if ((st->flags & VIR_STREAM_NONBLOCK) && (!S_ISCHR(sb.st_mode) && !S_ISFIFO(sb.st_mode))) { - streamsReportError(VIR_ERR_INTERNAL_ERROR, - _("Non-blocking I/O is not supported on %s"), - path); - goto error; + int childfd; + + if ((flags & O_RDWR) == O_RDWR) { + streamsReportError(VIR_ERR_INTERNAL_ERROR, + _("%s: Cannot request read and write flags together"), + path); + goto error; + } + + VIR_FORCE_CLOSE(fd); + if (pipe(fds) < 0) { + virReportSystemError(errno, "%s", + _("Unable to create pipe")); + goto error; + } + + cmd = virCommandNewArgList(LIBEXECDIR "/libvirt_iohelper", + path, + NULL); + virCommandAddArgFormat(cmd, "%d", flags); + virCommandAddArgFormat(cmd, "%d", mode); + virCommandAddArgFormat(cmd, "%llu", offset); + virCommandAddArgFormat(cmd, "%llu", length); + + if (flags == O_RDONLY) { + childfd = fds[1]; + fd = fds[0]; + virCommandSetOutputFD(cmd, &childfd); + } else { + childfd = fds[0]; + fd = fds[1]; + virCommandSetInputFD(cmd, childfd); + } + virCommandSetErrorFD(cmd, &errfd); + + if (virCommandRunAsync(cmd, &pid) < 0) + goto error; + + VIR_FORCE_CLOSE(childfd); + } else { + if (offset && + lseek(fd, offset, SEEK_SET) != offset) { + virReportSystemError(errno, + _("Unable to seek %s to %llu"), + path, offset); + goto error; + } } - if (virFDStreamOpen(st, fd) < 0) + if (virFDStreamOpenInternal(st, fd, cmd, errfd, length) < 0) goto error; return 0; error: + if (pid) + kill(SIGTERM, pid); + virCommandFree(cmd); + VIR_FORCE_CLOSE(fds[0]); + VIR_FORCE_CLOSE(fds[1]); VIR_FORCE_CLOSE(fd); return -1; } -int virFDStreamCreateFile(virStreamPtr st, - const char *path, - int flags, - mode_t mode) +int virFDStreamOpenFile(virStreamPtr st, + const char *path, + unsigned long long offset, + unsigned long long length, + int flags) { - int fd = open(path, flags, mode); - struct stat sb; - - if (fd < 0) { - virReportSystemError(errno, - _("Unable to open stream for '%s'"), - path); - return -1; - } - - if (fstat(fd, &sb) < 0) { - virReportSystemError(errno, - _("Unable to access stream for '%s'"), - path); - goto error; - } - - /* Thanks to the POSIX i/o model, we can't reliably get - * non-blocking I/O on block devs/regular files. To - * support those we need to fork a helper process todo - * the I/O so we just have a fifo. Or use AIO :-( - */ - if ((st->flags & VIR_STREAM_NONBLOCK) && - (!S_ISCHR(sb.st_mode) && - !S_ISFIFO(sb.st_mode))) { + if (flags & O_CREAT) { streamsReportError(VIR_ERR_INTERNAL_ERROR, - _("Non-blocking I/O is not supported on %s"), + _("Attempt to create %s without specifying mode"), path); - goto error; + return -1; } + return virFDStreamOpenFileInternal(st, path, + offset, length, + flags, 0); +} - if (virFDStreamOpen(st, fd) < 0) - goto error; - - return 0; - -error: - VIR_FORCE_CLOSE(fd); - return -1; +int virFDStreamCreateFile(virStreamPtr st, + const char *path, + unsigned long long offset, + unsigned long long length, + int flags, + mode_t mode) +{ + return virFDStreamOpenFileInternal(st, path, + offset, length, + flags | O_CREAT, mode); } diff --git a/src/fdstream.h b/src/fdstream.h index 53cbaa7..6b395b6 100644 --- a/src/fdstream.h +++ b/src/fdstream.h @@ -24,6 +24,7 @@ # define __VIR_FDSTREAM_H_ # include "internal.h" +# include "command.h" int virFDStreamOpen(virStreamPtr st, int fd); @@ -34,9 +35,13 @@ int virFDStreamConnectUNIX(virStreamPtr st, int virFDStreamOpenFile(virStreamPtr st, const char *path, + unsigned long long offset, + unsigned long long length, int flags); int virFDStreamCreateFile(virStreamPtr st, const char *path, + unsigned long long offset, + unsigned long long length, int flags, mode_t mode); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index 3159e1b..96f6c4b 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -2780,7 +2780,7 @@ lxcDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (virFDStreamOpenFile(st, chr->source.data.file.path, O_RDWR) < 0) + if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 018f8a1..fbe3666 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7076,7 +7076,7 @@ qemuDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (virFDStreamOpenFile(st, chr->source.data.file.path, O_RDWR) < 0) + if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index f19a4a8..0c6f1cd 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -2126,7 +2126,7 @@ umlDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (virFDStreamOpenFile(st, chr->source.data.file.path, O_RDWR) < 0) + if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; diff --git a/src/util/iohelper.c b/src/util/iohelper.c new file mode 100644 index 0000000..d5821b9 --- /dev/null +++ b/src/util/iohelper.c @@ -0,0 +1,203 @@ +/* + * iohelper.c: Helper program to perform I/O operations on files + * + * Copyright (C) 2011 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.1 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + * Author: Daniel P. Berrange <berrange@redhat.com> + * + * Current support + * - Read existing file + * - Write existing file + * - Create & write new file + */ + +#include <config.h> + +#include <locale.h> +#include <unistd.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> + +#include "util.h" +#include "threads.h" +#include "files.h" +#include "memory.h" +#include "virterror_internal.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +static int runIO(const char *path, + int flags, + int mode, + unsigned long long offset, + unsigned long long length) +{ + char *buf = NULL; + size_t buflen = 1024*1024; + int fd; + int ret = -1; + int fdin, fdout; + const char *fdinname, *fdoutname; + unsigned long long total = 0; + + if (flags & O_CREAT) { + fd = open(path, flags, mode); + } else { + fd = open(path, flags); + } + if (fd < 0) { + virReportSystemError(errno, _("Unable to open %s"), path); + goto cleanup; + } + + if (offset) { + if (lseek(fd, offset, SEEK_SET) < 0) { + virReportSystemError(errno, _("Unable to seek %s to %llu"), + path, offset); + goto cleanup; + } + } + + if (VIR_ALLOC_N(buf, buflen) < 0) { + virReportOOMError(); + goto cleanup; + } + + switch (flags & O_ACCMODE) { + case O_RDONLY: + fdin = fd; + fdinname = path; + fdout = STDOUT_FILENO; + fdoutname = "stdout"; + break; + case O_WRONLY: + fdin = STDIN_FILENO; + fdinname = "stdin"; + fdout = fd; + fdoutname = path; + break; + + case O_RDWR: + default: + virReportSystemError(EINVAL, + _("Unable to process file with flags %d"), + (flags & O_ACCMODE)); + goto cleanup; + } + + while (1) { + ssize_t got; + + if (length && + (length - total) < buflen) + buflen = length - total; + + if (buflen == 0) + break; /* End of requested data from client */ + + if ((got = saferead(fdin, buf, buflen)) < 0) { + virReportSystemError(errno, _("Unable to read %s"), fdinname); + goto cleanup; + } + if (got == 0) + break; /* End of file before end of requested data */ + + total += got; + if (safewrite(fdout, buf, got) < 0) { + virReportSystemError(errno, _("Unable to write %s"), fdoutname); + goto cleanup; + } + } + + ret = 0; + +cleanup: + if (VIR_CLOSE(fd) < 0 && + ret == 0) { + virReportSystemError(errno, _("Unable to close %s"), path); + ret = -1; + } + + VIR_FREE(buf); + return ret; +} + +int main(int argc, char **argv) +{ + const char *path; + virErrorPtr err; + unsigned long long offset; + unsigned long long length; + int flags; + int mode; + + if (setlocale(LC_ALL, "") == NULL || + bindtextdomain(PACKAGE, LOCALEDIR) == NULL || + textdomain(PACKAGE) == NULL) { + fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + exit(EXIT_FAILURE); + } + + if (virThreadInitialize() < 0 || + virErrorInitialize() < 0 || + virRandomInitialize(time(NULL) ^ getpid())) { + fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + exit(EXIT_FAILURE); + } + + if (argc != 6) { + fprintf(stderr, _("%s: syntax FILENAME FLAGS MODE OFFSET LENGTH\n"), argv[0]); + exit(EXIT_FAILURE); + } + + path = argv[1]; + + if (virStrToLong_i(argv[2], NULL, 10, &flags) < 0) { + fprintf(stderr, _("%s: malformed file flags %s"), argv[0], argv[2]); + exit(EXIT_FAILURE); + } + + if (virStrToLong_i(argv[3], NULL, 10, &mode) < 0) { + fprintf(stderr, _("%s: malformed file mode %s"), argv[0], argv[3]); + exit(EXIT_FAILURE); + } + + if (virStrToLong_ull(argv[4], NULL, 10, &offset) < 0) { + fprintf(stderr, _("%s: malformed file offset %s"), argv[0], argv[4]); + exit(EXIT_FAILURE); + } + if (virStrToLong_ull(argv[5], NULL, 10, &length) < 0) { + fprintf(stderr, _("%s: malformed file length %s"), argv[0], argv[5]); + exit(EXIT_FAILURE); + } + + if (runIO(path, flags, mode, offset, length) < 0) + goto error; + + return 0; + +error: + err = virGetLastError(); + if (err) { + fprintf(stderr, "%s: %s\n", argv[0], err->message); + } else { + fprintf(stderr, _("%s: unknown failure with %s\n"), argv[0], path); + } + exit(EXIT_FAILURE); +} diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index bf422e6..9f47722 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2019,7 +2019,7 @@ xenUnifiedDomainOpenConsole(virDomainPtr dom, goto cleanup; } - if (virFDStreamOpenFile(st, chr->source.data.file.path, O_RDWR) < 0) + if (virFDStreamOpenFile(st, chr->source.data.file.path, 0, 0, O_RDWR) < 0) goto cleanup; ret = 0; -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
The O_NONBLOCK flag doesn't work as desired on plain files or block devices. Introduce an I/O helper program that does the blocking I/O operations, communicating over a pipe that can support O_NONBLOCK
* src/fdstream.c, src/fdstream.h: Add non-blocking I/O on plain files/block devices * src/Makefile.am, src/util/iohelper.c: I/O helper program * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c, src/uml/uml_driver.c, src/xen/xen_driver.c: Update for streams API change
@@ -206,6 +213,35 @@ static int virFDStreamFree(struct virFDStreamData *fdst) { int ret; ret = VIR_CLOSE(fdst->fd); + if (fdst->cmd) { + char buf[1024]; + ssize_t len; + int status; + if ((len = saferead(fdst->errfd, buf, sizeof(buf)-1)) < 0) + buf[0] = '\0'; + else + buf[len] = '\0'; + + if (virCommandWait(fdst->cmd, &status) < 0) { + ret = -1; + } else if (status != 0) { + if (buf[0] == '\0') { + if (WIFEXITED(status)) { + streamsReportError(VIR_ERR_INTERNAL_ERROR, + _("I/O helper exited with status %d"), + WEXITSTATUS(status)); + } else { + streamsReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("I/O helper exited abnormally"));
Is it worth using virCommandTranslateStatus here to come up with a better message? That depends on this unreviewed patch: https://www.redhat.com/archives/libvir-list/2011-March/msg01119.html If you push this one first, I can rebase that one.
@@ -346,6 +419,13 @@ int virFDStreamOpen(virStreamPtr st, }
+int virFDStreamOpen(virStreamPtr st, + int fd) +{ + return virFDStreamOpenInternal(st, fd, NULL, -1, 0); +}
Hmm. This just blindly uses fd as a stream, even if it is a regular file. And my fd: migration code (eek - I really need to repost that soon, after running more testing on it) sometimes passes a regular file; and not just any regular file, but a fd that might have been opened with virFileOpenAs to bypass NFS root-squash limitations. You _want_ to use the iohelper for regular file fds. And thinking about it more...
+static int +virFDStreamOpenFileInternal(virStreamPtr st, + const char *path, + unsigned long long offset, + unsigned long long length, + int flags, + int mode) { ... + if (flags & O_CREAT) + fd = open(path, flags, mode); + else + fd = open(path, flags); + if (fd < 0) { virReportSystemError(errno,
@@ -440,64 +530,95 @@ int virFDStreamOpenFile(virStreamPtr st, if ((st->flags & VIR_STREAM_NONBLOCK) && (!S_ISCHR(sb.st_mode) && !S_ISFIFO(sb.st_mode))) { ...
+ + VIR_FORCE_CLOSE(fd);
This block of virFDStreamOpenFile needs to instead live in virFDStreamOpen, and instead of passing a file name to the iohelper process (which is slightly racy, because the file could change between the two calls to open(), and only works for non-root-squash files), you should instead pass an open fd to the iohelper process (along with a string representation of which fd the iohelper process should have expected to inherit). That is, the iohelper should _not_ call open(), but should already be handed the fd in question from the parent process. Perhaps you might _also_ want to change the signature of virFDStreamOpen to take an optional const char * name of the fd, which if non-NULL can be used for more informative error messages, but which is not essential.
+ cmd = virCommandNewArgList(LIBEXECDIR "/libvirt_iohelper", + path, + NULL); + virCommandAddArgFormat(cmd, "%d", flags); + virCommandAddArgFormat(cmd, "%d", mode); + virCommandAddArgFormat(cmd, "%llu", offset); + virCommandAddArgFormat(cmd, "%llu", length);
Given the above discussion, you don't need to pass flags or mode, but do need to pass offset and length as well as a new fd argument, and pass an empty string when path is otherwise unknown (since you can't safely pass NULL). Hmm, even offset might not be necessary (the iohelper could use lseek to learn the current position); but length is necessary to cap io at the right limits.
+ + if (virCommandRunAsync(cmd, &pid) < 0) + goto error; + ...
error: + if (pid) + kill(SIGTERM, pid);
You could replace the two line if and kill() with a one-liner virCommandAbort(cmd)...
+ virCommandFree(cmd);
if my patch goes in first (this is a case where virCommandFree won't do it for you, because you grabbed the pid during virCommandRunAsync). https://www.redhat.com/archives/libvir-list/2011-March/msg01120.html Now, all that said, I don't have any technical objections to this patch as-is (virFDStreamOpen is just as broken before and after your patch if I get my fd: migration code working). In other words, all my ideas about converting iohelper to use an inherited fd could be done as an incremental followup patch rather than holding this one up any longer; that is, I'd rather see file upload/download get into 0.9.0 rather than miss out on it just because there's room for further improvement at a later date. So you have my: ACK and it's up to you whether to use that ACK and push now, or do a v4 with the improved iohelper via fd inheritance. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
The O_NONBLOCK flag doesn't work as desired on plain files or block devices. Introduce an I/O helper program that does the blocking I/O operations, communicating over a pipe that can support O_NONBLOCK
* src/fdstream.c, src/fdstream.h: Add non-blocking I/O on plain files/block devices * src/Makefile.am, src/util/iohelper.c: I/O helper program * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c, src/uml/uml_driver.c, src/xen/xen_driver.c: Update for streams API change --- po/POTFILES.in | 1 + src/Makefile.am | 12 +++ src/fdstream.c | 233 ++++++++++++++++++++++++++++++++++++------------ src/fdstream.h | 5 + src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/iohelper.c | 203 +++++++++++++++++++++++++++++++++++++++++ src/xen/xen_driver.c | 2 +- 9 files changed, 402 insertions(+), 60 deletions(-) create mode 100644 src/util/iohelper.c
diff --git a/po/POTFILES.in b/po/POTFILES.in index 805e5ca..12adb3e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -94,6 +94,7 @@ src/util/event_poll.c src/util/hash.c src/util/hooks.c src/util/hostusb.c +src/util/iohelper.c src/util/interface.c src/util/iptables.c src/util/json.c diff --git a/src/Makefile.am b/src/Makefile.am index c3729a6..1d8115b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES = \ STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c
+UTIL_IO_HELPER_SOURCES = \ + util/iohelper.c + # Network filters NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
libexec_PROGRAMS =
+libexec_PROGRAMS += libvirt_iohelper +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES) +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) +libvirt_iohelper_LDADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + +libvirt_iohelper_CFLAGS = $(AM_CFLAGS) +
Is libvirt_iohelper for libvirtd? libvirt_iohelper is provided by libvirt-<version>.rpm, but we still install it when we build withoud libvirtd. We will meet the following problems: Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64 error: Installed (but unpackaged) file(s) found: /usr/libexec/libvirt_iohelper
if WITH_STORAGE_DISK if WITH_LIBVIRTD libexec_PROGRAMS += libvirt_parthelper
Wen Congyang wrote:
At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
The O_NONBLOCK flag doesn't work as desired on plain files or block devices. Introduce an I/O helper program that does the blocking I/O operations, communicating over a pipe that can support O_NONBLOCK
* src/fdstream.c, src/fdstream.h: Add non-blocking I/O on plain files/block devices * src/Makefile.am, src/util/iohelper.c: I/O helper program * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c, src/uml/uml_driver.c, src/xen/xen_driver.c: Update for streams API change --- po/POTFILES.in | 1 + src/Makefile.am | 12 +++ src/fdstream.c | 233 ++++++++++++++++++++++++++++++++++++------------ src/fdstream.h | 5 + src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/iohelper.c | 203 +++++++++++++++++++++++++++++++++++++++++ src/xen/xen_driver.c | 2 +- 9 files changed, 402 insertions(+), 60 deletions(-) create mode 100644 src/util/iohelper.c
diff --git a/po/POTFILES.in b/po/POTFILES.in index 805e5ca..12adb3e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -94,6 +94,7 @@ src/util/event_poll.c src/util/hash.c src/util/hooks.c src/util/hostusb.c +src/util/iohelper.c src/util/interface.c src/util/iptables.c src/util/json.c diff --git a/src/Makefile.am b/src/Makefile.am index c3729a6..1d8115b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES = \ STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c
+UTIL_IO_HELPER_SOURCES = \ + util/iohelper.c + # Network filters NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
libexec_PROGRAMS =
+libexec_PROGRAMS += libvirt_iohelper +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES) +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) +libvirt_iohelper_LDADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + +libvirt_iohelper_CFLAGS = $(AM_CFLAGS) +
Is libvirt_iohelper for libvirtd?
libvirt_iohelper is provided by libvirt-<version>.rpm, but we still install it when we build withoud libvirtd. We will meet the following problems:
Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64 error: Installed (but unpackaged) file(s) found: /usr/libexec/libvirt_iohelper
I met the same problem and added libvirt-iohelper to our client package. Is it used in client-only configuration? Regards, Jim
On Thu, Apr 07, 2011 at 10:41:34AM -0600, Jim Fehlig wrote:
Wen Congyang wrote:
At 03/24/2011 01:36 AM, Daniel P. Berrange Write:
The O_NONBLOCK flag doesn't work as desired on plain files or block devices. Introduce an I/O helper program that does the blocking I/O operations, communicating over a pipe that can support O_NONBLOCK
* src/fdstream.c, src/fdstream.h: Add non-blocking I/O on plain files/block devices * src/Makefile.am, src/util/iohelper.c: I/O helper program * src/qemu/qemu_driver.c, src/lxc/lxc_driver.c, src/uml/uml_driver.c, src/xen/xen_driver.c: Update for streams API change --- po/POTFILES.in | 1 + src/Makefile.am | 12 +++ src/fdstream.c | 233 ++++++++++++++++++++++++++++++++++++------------ src/fdstream.h | 5 + src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 2 +- src/uml/uml_driver.c | 2 +- src/util/iohelper.c | 203 +++++++++++++++++++++++++++++++++++++++++ src/xen/xen_driver.c | 2 +- 9 files changed, 402 insertions(+), 60 deletions(-) create mode 100644 src/util/iohelper.c
diff --git a/po/POTFILES.in b/po/POTFILES.in index 805e5ca..12adb3e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -94,6 +94,7 @@ src/util/event_poll.c src/util/hash.c src/util/hooks.c src/util/hostusb.c +src/util/iohelper.c src/util/interface.c src/util/iptables.c src/util/json.c diff --git a/src/Makefile.am b/src/Makefile.am index c3729a6..1d8115b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -380,6 +380,9 @@ STORAGE_DRIVER_DISK_SOURCES = \ STORAGE_HELPER_DISK_SOURCES = \ storage/parthelper.c
+UTIL_IO_HELPER_SOURCES = \ + util/iohelper.c + # Network filters NWFILTER_DRIVER_SOURCES = \ nwfilter/nwfilter_driver.h nwfilter/nwfilter_driver.c \ @@ -1203,6 +1206,15 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
libexec_PROGRAMS =
+libexec_PROGRAMS += libvirt_iohelper +libvirt_iohelper_SOURCES = $(UTIL_IO_HELPER_SOURCES) +libvirt_iohelper_LDFLAGS = $(WARN_LDFLAGS) $(AM_LDFLAGS) +libvirt_iohelper_LDADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + +libvirt_iohelper_CFLAGS = $(AM_CFLAGS) +
Is libvirt_iohelper for libvirtd?
libvirt_iohelper is provided by libvirt-<version>.rpm, but we still install it when we build withoud libvirtd. We will meet the following problems:
Checking for unpackaged file(s): /usr/lib/rpm/check-files /home/wency/rpmbuild/BUILDROOT/libvirt-0.9.0-1.el6.x86_64 error: Installed (but unpackaged) file(s) found: /usr/libexec/libvirt_iohelper
I met the same problem and added libvirt-iohelper to our client package. Is it used in client-only configuration?
It is only currently used in the QEMU or storage drivers, which are both daemon based. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
New APIs are added allowing streaming of content to/from storage volumes. * include/libvirt/libvirt.h.in: Add virStorageVolUpload and virStorageVolDownload APIs * src/driver.h, src/libvirt.c, src/libvirt_public.syms: Stub code for new APIs * src/storage/storage_driver.c, src/esx/esx_storage_driver.c: Add dummy entries in driver table for new APIs --- include/libvirt/libvirt.h.in | 10 +++ include/libvirt/virterror.h | 1 + src/driver.h | 14 ++++ src/esx/esx_storage_driver.c | 2 + src/libvirt.c | 140 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/storage/storage_driver.c | 2 + src/util/virterror.c | 6 ++ 8 files changed, 177 insertions(+), 0 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1cf9273..bd36015 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1504,6 +1504,16 @@ virStorageVolPtr virStorageVolCreateXMLFrom (virStoragePoolPtr pool, const char *xmldesc, virStorageVolPtr clonevol, unsigned int flags); +int virStorageVolDownload (virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags); +int virStorageVolUpload (virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags); int virStorageVolDelete (virStorageVolPtr vol, unsigned int flags); int virStorageVolWipe (virStorageVolPtr vol, diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 1d8275b..59f7731 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -230,6 +230,7 @@ typedef enum { VIR_ERR_HOOK_SCRIPT_FAILED = 70, /* a synchronous hook script failed */ VIR_ERR_INVALID_DOMAIN_SNAPSHOT = 71,/* invalid domain snapshot */ VIR_ERR_NO_DOMAIN_SNAPSHOT = 72, /* domain snapshot not found */ + VIR_ERR_INVALID_STREAM = 73 /* stream pointer not valid */ } virErrorNumber; /** diff --git a/src/driver.h b/src/driver.h index 286130a..e5f91ca 100644 --- a/src/driver.h +++ b/src/driver.h @@ -905,6 +905,18 @@ typedef virStorageVolPtr const char *xmldesc, virStorageVolPtr clone, unsigned int flags); +typedef int + (*virDrvStorageVolDownload) (virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags); +typedef int + (*virDrvStorageVolUpload) (virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags); typedef int (*virDrvStoragePoolIsActive)(virStoragePoolPtr pool); @@ -959,6 +971,8 @@ struct _virStorageDriver { virDrvStorageVolLookupByPath volLookupByPath; virDrvStorageVolCreateXML volCreateXML; virDrvStorageVolCreateXMLFrom volCreateXMLFrom; + virDrvStorageVolDownload volDownload; + virDrvStorageVolUpload volUpload; virDrvStorageVolDelete volDelete; virDrvStorageVolWipe volWipe; virDrvStorageVolGetInfo volGetInfo; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 136a90b..9e4dd9e 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -1671,6 +1671,8 @@ static virStorageDriver esxStorageDriver = { esxStorageVolumeLookupByPath, /* volLookupByPath */ esxStorageVolumeCreateXML, /* volCreateXML */ esxStorageVolumeCreateXMLFrom, /* volCreateXMLFrom */ + NULL, /* volDownload */ + NULL, /* volUpload */ esxStorageVolumeDelete, /* volDelete */ esxStorageVolumeWipe, /* volWipe */ esxStorageVolumeGetInfo, /* volGetInfo */ diff --git a/src/libvirt.c b/src/libvirt.c index e46c18b..d2f18d3 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -9066,6 +9066,146 @@ error: /** + * virStorageVolDownload: + * @vol: pointer to volume to download from + * @stream: stream to use as output + * @offset: position in @vol to start reading from + * @length: limit on amount of data to download + * @flags: future flags (unused, pass 0) + * + * Download the content of the volume as a stream. If @length + * is zero, then the remaining contents of the volume after + * @offset will be downloaded. + * + * This call sets up an asynchronous stream; subsequent use of + * stream APIs is necessary to transfer the actual data, + * determine how much data is successfully transferred, and + * detect any errors. The results will be unpredictable if + * another active stream is writing to the storage volume. + * + * Returns 0, or -1 upon error. + */ +int +virStorageVolDownload(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + VIR_DEBUG("vol=%p stream=%p offset=%llu length=%llu flags=%u", + vol, stream, offset, length, flags); + + virResetLastError(); + + if (!VIR_IS_STORAGE_VOL(vol)) { + virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); + return -1; + } + + if (!VIR_IS_STREAM(stream)) { + virLibConnError(VIR_ERR_INVALID_STREAM, __FUNCTION__); + return -1; + } + + if (vol->conn->flags & VIR_CONNECT_RO || + stream->conn->flags & VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (vol->conn->storageDriver && + vol->conn->storageDriver->volDownload) { + int ret; + ret = vol->conn->storageDriver->volDownload(vol, + stream, + offset, + length, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(vol->conn); + return -1; +} + + +/** + * virStorageVolUpload: + * @vol: pointer to volume to download + * @stream: stream to use as output + * @offset: position to start writing to + * @length: limit on amount of data to upload + * @flags: flags for creation (unused, pass 0) + * + * Upload new content to the volume from a stream. This call + * will fail if @offset + @length exceeds the size of the + * volume. Otherwise, if @length is non-zero, and an error + * will be raised if an attempt is made to upload greater + * than @length bytes of data. + * + * This call sets up an asynchronous stream; subsequent use of + * stream APIs is necessary to transfer the actual data, + * determine how much data is successfully transferred, and + * detect any errors. The results will be unpredictable if + * another active stream is writing to the storage volume. + * + * Returns 0, or -1 upon error. + */ +int +virStorageVolUpload(virStorageVolPtr vol, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + VIR_DEBUG("vol=%p stream=%p offset=%llu length=%llu flags=%u", + vol, stream, offset, length, flags); + + virResetLastError(); + + if (!VIR_IS_STORAGE_VOL(vol)) { + virLibConnError(VIR_ERR_INVALID_STORAGE_VOL, __FUNCTION__); + return -1; + } + + if (!VIR_IS_STREAM(stream)) { + virLibConnError(VIR_ERR_INVALID_STREAM, __FUNCTION__); + return -1; + } + + if (vol->conn->flags & VIR_CONNECT_RO || + stream->conn->flags & VIR_CONNECT_RO) { + virLibConnError(VIR_ERR_OPERATION_DENIED, __FUNCTION__); + goto error; + } + + if (vol->conn->storageDriver && + vol->conn->storageDriver->volUpload) { + int ret; + ret = vol->conn->storageDriver->volUpload(vol, + stream, + offset, + length, + flags); + if (ret < 0) + goto error; + return ret; + } + + virLibConnError(VIR_ERR_NO_SUPPORT, __FUNCTION__); + +error: + virDispatchError(vol->conn); + return -1; +} + + +/** * virStorageVolDelete: * @vol: pointer to storage volume * @flags: future flags, use 0 for now diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 5caab4c..b4aed41 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -432,6 +432,8 @@ LIBVIRT_0.9.0 { virDomainSetMemoryFlags; virEventRegisterDefaultImpl; virEventRunDefaultImpl; + virStorageVolDownload; + virStorageVolUpload; } LIBVIRT_0.8.8; # .... define new API here using predicted next version number .... diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 5373025..ce528cf 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1989,6 +1989,8 @@ static virStorageDriver storageDriver = { .volLookupByPath = storageVolumeLookupByPath, .volCreateXML = storageVolumeCreateXML, .volCreateXMLFrom = storageVolumeCreateXMLFrom, + .volDownload = NULL, + .volUpload = NULL, .volDelete = storageVolumeDelete, .volWipe = storageVolumeWipe, .volGetInfo = storageVolumeGetInfo, diff --git a/src/util/virterror.c b/src/util/virterror.c index 160c953..b7d8924 100644 --- a/src/util/virterror.c +++ b/src/util/virterror.c @@ -1201,6 +1201,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("Domain snapshot not found: %s"); break; + case VIR_ERR_INVALID_STREAM: + if (info == NULL) + errmsg = _("invalid stream pointer"); + else + errmsg = _("invalid stream pointer in %s"); + break; } return (errmsg); } -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
New APIs are added allowing streaming of content to/from storage volumes.
* include/libvirt/libvirt.h.in: Add virStorageVolUpload and virStorageVolDownload APIs * src/driver.h, src/libvirt.c, src/libvirt_public.syms: Stub code for new APIs * src/storage/storage_driver.c, src/esx/esx_storage_driver.c: Add dummy entries in driver table for new APIs --- include/libvirt/libvirt.h.in | 10 +++ include/libvirt/virterror.h | 1 + src/driver.h | 14 ++++ src/esx/esx_storage_driver.c | 2 + src/libvirt.c | 140 ++++++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 2 + src/storage/storage_driver.c | 2 + src/util/virterror.c | 6 ++ 8 files changed, 177 insertions(+), 0 deletions(-)
+++ b/include/libvirt/virterror.h @@ -230,6 +230,7 @@ typedef enum { VIR_ERR_HOOK_SCRIPT_FAILED = 70, /* a synchronous hook script failed */ VIR_ERR_INVALID_DOMAIN_SNAPSHOT = 71,/* invalid domain snapshot */ VIR_ERR_NO_DOMAIN_SNAPSHOT = 72, /* domain snapshot not found */ + VIR_ERR_INVALID_STREAM = 73 /* stream pointer not valid */
Add the trailing comma now, to reduce the diff burden later on the next addition.
+ +/** + * virStorageVolUpload: + * @vol: pointer to volume to download
s/download/upload/
+ * @stream: stream to use as output
s/output/input/
+ * @offset: position to start writing to + * @length: limit on amount of data to upload + * @flags: flags for creation (unused, pass 0) + * + * Upload new content to the volume from a stream. This call + * will fail if @offset + @length exceeds the size of the + * volume. Otherwise, if @length is non-zero, and an error
s/and // ACK with those four nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
The new commands vol-upload and vol-download, allow a local file to be transferred to/from a storage volume. * tools/virsh.c: Add vol-upload and vol-download commands --- tools/virsh.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 230 insertions(+), 0 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 50ca50f..708003b 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -266,6 +266,9 @@ static int vshCommandOptString(const vshCmd *cmd, const char *name, static int vshCommandOptLongLong(const vshCmd *cmd, const char *name, long long *value) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; +static int vshCommandOptULongLong(const vshCmd *cmd, const char *name, + unsigned long long *value) + ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; static int vshCommandOptBool(const vshCmd *cmd, const char *name); static char *vshCommandOptArgv(const vshCmd *cmd, int count); @@ -7082,6 +7085,209 @@ cleanup: return ret; } + +/* + * "vol-upload" command + */ +static const vshCmdInfo info_vol_upload[] = { + {"help", N_("upload a file into a volume")}, + {"desc", N_("Upload a file into a volume")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_upload[] = { + {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file")}, + {"offset", VSH_OT_INT, 0, N_("volume offset to upload to") }, + {"length", VSH_OT_INT, 0, N_("amount of data to upload") }, + {NULL, 0, 0, NULL} +}; + +static int +cmdVolUploadSource(virStreamPtr st ATTRIBUTE_UNUSED, + char *bytes, size_t nbytes, void *opaque) +{ + int *fd = opaque; + + return saferead(*fd, bytes, nbytes); +} + +static int +cmdVolUpload (vshControl *ctl, const vshCmd *cmd) +{ + const char *file = NULL; + virStorageVolPtr vol = NULL; + int ret = FALSE; + int fd = -1; + virStreamPtr st = NULL; + const char *name = NULL; + unsigned long long offset = 0, length = 0; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { + vshError(ctl, _("Unable to parse integer")); + return FALSE; + } + + if (vshCommandOptULongLong(cmd, "length", &length) < 0) { + vshError(ctl, _("Unable to parse integer")); + return FALSE; + } + + if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) { + return FALSE; + } + + if (vshCommandOptString(cmd, "file", &file) < 0) + goto cleanup; + + if ((fd = open(file, O_RDONLY)) < 0) { + vshError(ctl, "cannot read %s", file); + goto cleanup; + } + + st = virStreamNew(ctl->conn, 0); + if (virStorageVolUpload(vol, st, offset, length, 0) < 0) { + vshError(ctl, "cannot upload to volume %s", name); + goto cleanup; + } + + if (virStreamSendAll(st, cmdVolUploadSource, &fd) < 0) { + vshError(ctl, "cannot send data to volume %s", name); + goto cleanup; + } + + if (VIR_CLOSE(fd) < 0) { + vshError(ctl, "cannot close file %s", file); + virStreamAbort(st); + goto cleanup; + } + + if (virStreamFinish(st) < 0) { + vshError(ctl, "cannot close volume %s", name); + goto cleanup; + } + + ret = TRUE; + +cleanup: + if (vol) + virStorageVolFree(vol); + if (st) + virStreamFree(st); + VIR_FORCE_CLOSE(fd); + return ret; +} + + + +/* + * "vol-download" command + */ +static const vshCmdInfo info_vol_download[] = { + {"help", N_("Download a volume to a file")}, + {"desc", N_("Download a volume to a file")}, + {NULL, NULL} +}; + +static const vshCmdOptDef opts_vol_download[] = { + {"pool", VSH_OT_STRING, 0, N_("pool name or uuid")}, + {"vol", VSH_OT_DATA, VSH_OFLAG_REQ, N_("vol name, key or path")}, + {"file", VSH_OT_DATA, VSH_OFLAG_REQ, N_("file")}, + {"offset", VSH_OT_INT, 0, N_("volume offset to download from") }, + {"length", VSH_OT_INT, 0, N_("amount of data to download") }, + {NULL, 0, 0, NULL} +}; + + +static int +cmdVolDownloadSink(virStreamPtr st ATTRIBUTE_UNUSED, + const char *bytes, size_t nbytes, void *opaque) +{ + int *fd = opaque; + + return safewrite(*fd, bytes, nbytes); +} + +static int +cmdVolDownload (vshControl *ctl, const vshCmd *cmd) +{ + const char *file = NULL; + virStorageVolPtr vol = NULL; + int ret = FALSE; + int fd = -1; + virStreamPtr st = NULL; + const char *name = NULL; + unsigned long long offset = 0, length = 0; + bool created = true; + + if (!vshConnectionUsability(ctl, ctl->conn)) + goto cleanup; + + if (vshCommandOptULongLong(cmd, "offset", &offset) < 0) { + vshError(ctl, _("Unable to parse integer")); + return FALSE; + } + + if (vshCommandOptULongLong(cmd, "length", &length) < 0) { + vshError(ctl, _("Unable to parse integer")); + return FALSE; + } + + if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) + return FALSE; + + if (vshCommandOptString(cmd, "file", &file) < 0) + goto cleanup; + + if ((fd = open(file, O_WRONLY|O_TRUNC|O_CREAT|O_EXCL, 0666)) < 0) { + created = false; + if (errno != EEXIST || + (fd = open(file, O_WRONLY|O_TRUNC|O_CREAT, 0666)) < 0) { + vshError(ctl, _("cannot create %s"), file); + goto cleanup; + } + } + + st = virStreamNew(ctl->conn, 0); + if (virStorageVolDownload(vol, st, offset, length, 0) < 0) { + vshError(ctl, _("cannot download from volume %s"), name); + goto cleanup; + } + + if (virStreamRecvAll(st, cmdVolDownloadSink, &fd) < 0) { + vshError(ctl, _("cannot receive data from volume %s"), name); + goto cleanup; + } + + if (VIR_CLOSE(fd) < 0) { + vshError(ctl, "cannot close file %s", file); + virStreamAbort(st); + goto cleanup; + } + + if (virStreamFinish(st) < 0) { + vshError(ctl, "cannot close volume %s", name); + goto cleanup; + } + + ret = TRUE; + +cleanup: + if (ret == FALSE && created) + unlink(file); + if (vol) + virStorageVolFree(vol); + if (st) + virStreamFree(st); + VIR_FORCE_CLOSE(fd); + return ret; +} + + /* * "vol-delete" command */ @@ -9901,6 +10107,7 @@ cmdEdit (vshControl *ctl, const vshCmd *cmd) return ret; } + /* * "net-edit" command */ @@ -10538,6 +10745,7 @@ static const vshCmdDef storageVolCmds[] = { {"vol-create", cmdVolCreate, opts_vol_create, info_vol_create}, {"vol-create-from", cmdVolCreateFrom, opts_vol_create_from, info_vol_create_from}, {"vol-delete", cmdVolDelete, opts_vol_delete, info_vol_delete}, + {"vol-download", cmdVolDownload, opts_vol_download, info_vol_download }, {"vol-dumpxml", cmdVolDumpXML, opts_vol_dumpxml, info_vol_dumpxml}, {"vol-info", cmdVolInfo, opts_vol_info, info_vol_info}, {"vol-key", cmdVolKey, opts_vol_key, info_vol_key}, @@ -10545,6 +10753,7 @@ static const vshCmdDef storageVolCmds[] = { {"vol-name", cmdVolName, opts_vol_name, info_vol_name}, {"vol-path", cmdVolPath, opts_vol_path, info_vol_path}, {"vol-pool", cmdVolPool, opts_vol_pool, info_vol_pool}, + {"vol-upload", cmdVolUpload, opts_vol_upload, info_vol_upload }, {"vol-wipe", cmdVolWipe, opts_vol_wipe, info_vol_wipe}, {NULL, NULL, NULL, NULL} }; @@ -11037,6 +11246,27 @@ vshCommandOptLongLong(const vshCmd *cmd, const char *name, return ret; } +static int +vshCommandOptULongLong(const vshCmd *cmd, const char *name, + unsigned long long *value) +{ + vshCmdOpt *arg = vshCommandOpt(cmd, name); + int ret = 0; + unsigned long long num; + char *end_p = NULL; + + if ((arg != NULL) && (arg->data != NULL)) { + num = strtoull(arg->data, &end_p, 10); + ret = -1; + if ((arg->data != end_p) && (*end_p == 0)) { + *value = num; + ret = 1; + } + } + return ret; +} + + /* * Returns TRUE/FALSE if the option exists */ -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
The new commands vol-upload and vol-download, allow a local file to be transferred to/from a storage volume.
* tools/virsh.c: Add vol-upload and vol-download commands --- tools/virsh.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 230 insertions(+), 0 deletions(-)
Shame on you for forgetting tools/virsh.pod (and on me for forgetting to mention it on v1 and v2). However, documentation can be written in the week between feature freeze and the actual 0.9.0 release, so while I'd rather you write some up, I'm okay if you push this patch with only the nits below fixed, and save the documentation for next week (and I'll help write that, if necessary, since I forgot to mention it sooner), if that ensures that we get this into 0.9.0.
+ if (vshCommandOptULongLong(cmd, "length", &length) < 0) { + vshError(ctl, _("Unable to parse integer")); + return FALSE; + } + + if (!(vol = vshCommandOptVol(ctl, cmd, "vol", "pool", &name))) { + return FALSE; + } + + if (vshCommandOptString(cmd, "file", &file) < 0) + goto cleanup;
Missing a call to vshError(ctl, _("file must not be empty"));
+static int +cmdVolDownload (vshControl *ctl, const vshCmd *cmd) +{
+ + if (vshCommandOptString(cmd, "file", &file) < 0) + goto cleanup;
Again.
+ + if ((fd = open(file, O_WRONLY|O_TRUNC|O_CREAT|O_EXCL, 0666)) < 0) {
O_TRUNC should be omitted (when O_CREAT|O_EXCL is in effect, you are guaranteeing that there is nothing to truncate; while Linux silently ignores the O_TRUNC, someone else might conceivably fail with EINVAL on the odd combination).
+ created = false; + if (errno != EEXIST || + (fd = open(file, O_WRONLY|O_TRUNC|O_CREAT, 0666)) < 0) {
Here, the O_CREAT can be omitted (you just proved the file exited), but that's not as essential as omitting the O_TRUNC on the previous open. Reluctant ACK, with the above nits (minus virsh.pod) fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
Use generic FD streams to allow data upload/download to/from any storage volume * src/storage/storage_driver.c: Wire up upload/download APIs --- src/storage/storage_driver.c | 132 +++++++++++++++++++++++++++++++++++++++++- 1 files changed, 130 insertions(+), 2 deletions(-) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ce528cf..cf1f6d4 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -46,6 +46,7 @@ #include "storage_backend.h" #include "logging.h" #include "files.h" +#include "fdstream.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -1528,6 +1529,133 @@ cleanup: } +static int +storageVolumeDownload(virStorageVolPtr obj, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); + storageDriverUnlock(driver); + + if (!pool) { + virStorageReportError(VIR_ERR_NO_STORAGE_POOL, + "%s", _("no storage pool with matching uuid")); + goto out; + } + + if (!virStoragePoolObjIsActive(pool)) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("storage pool is not active")); + goto out; + } + + vol = virStorageVolDefFindByName(pool, obj->name); + + if (vol == NULL) { + virStorageReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + obj->name); + goto out; + } + + if (vol->building) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("volume '%s' is still being allocated."), + vol->name); + goto out; + } + + if (virFDStreamOpenFile(stream, + vol->target.path, + offset, length, + O_RDONLY) < 0) + goto out; + + ret = 0; + +out: + if (pool) + virStoragePoolObjUnlock(pool); + + return ret; +} + + +static int +storageVolumeUpload(virStorageVolPtr obj, + virStreamPtr stream, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + virStorageDriverStatePtr driver = obj->conn->storagePrivateData; + virStoragePoolObjPtr pool = NULL; + virStorageVolDefPtr vol = NULL; + int ret = -1; + + virCheckFlags(0, -1); + + storageDriverLock(driver); + pool = virStoragePoolObjFindByName(&driver->pools, obj->pool); + storageDriverUnlock(driver); + + if (!pool) { + virStorageReportError(VIR_ERR_NO_STORAGE_POOL, + "%s", _("no storage pool with matching uuid")); + goto out; + } + + if (!virStoragePoolObjIsActive(pool)) { + virStorageReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("storage pool is not active")); + goto out; + } + + vol = virStorageVolDefFindByName(pool, obj->name); + + if (vol == NULL) { + virStorageReportError(VIR_ERR_NO_STORAGE_VOL, + _("no storage vol with matching name '%s'"), + obj->name); + goto out; + } + + if (vol->building) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("volume '%s' is still being allocated."), + vol->name); + goto out; + } + + /* Not using O_CREAT because the file is required to + * already exist at this point */ + if (virFDStreamOpenFile(stream, + vol->target.path, + offset, length, + O_WRONLY) < 0) + goto out; + + ret = 0; + +out: + if (pool) + virStoragePoolObjUnlock(pool); + + return ret; +} + + + /* If the volume we're wiping is already a sparse file, we simply * truncate and extend it to its original size, filling it with * zeroes. This behavior is guaranteed by POSIX: @@ -1989,8 +2117,8 @@ static virStorageDriver storageDriver = { .volLookupByPath = storageVolumeLookupByPath, .volCreateXML = storageVolumeCreateXML, .volCreateXMLFrom = storageVolumeCreateXMLFrom, - .volDownload = NULL, - .volUpload = NULL, + .volDownload = storageVolumeDownload, + .volUpload = storageVolumeUpload, .volDelete = storageVolumeDelete, .volWipe = storageVolumeWipe, .volGetInfo = storageVolumeGetInfo, -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
Use generic FD streams to allow data upload/download to/from any storage volume
* src/storage/storage_driver.c: Wire up upload/download APIs --- src/storage/storage_driver.c | 132 +++++++++++++++++++++++++++++++++++++++++- 1 files changed, 130 insertions(+), 2 deletions(-)
ACK.
+ + /* Not using O_CREAT because the file is required to + * already exist at this point */ + if (virFDStreamOpenFile(stream, + vol->target.path, + offset, length, + O_WRONLY) < 0)
This won't work with root-squash NFS volumes, but that can be a later fixup. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
* daemon/remote.c, src/remote/remote_driver.c: Implementation of storage vol upload/download APIs * src/remote/remote_protocol.x: Wire protocol definition for upload/download * daemon/remote_dispatch_args.h, daemon/remote_dispatch_prototypes.h, daemon/remote_dispatch_table.h, src/remote/remote_protocol.h, src/remote/remote_protocol.c: Re-generate --- daemon/remote.c | 92 +++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 2 + daemon/remote_dispatch_prototypes.h | 16 ++++++ daemon/remote_dispatch_table.h | 10 ++++ src/remote/remote_driver.c | 92 +++++++++++++++++++++++++++++++++- src/remote/remote_protocol.c | 30 +++++++++++ src/remote/remote_protocol.h | 22 ++++++++ src/remote/remote_protocol.x | 19 +++++++- src/remote_protocol-structs | 12 +++++ 9 files changed, 291 insertions(+), 4 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index a8fef4d..8cadfc2 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -5816,7 +5816,99 @@ remoteDispatchNodeDeviceDestroy(struct qemud_server *server ATTRIBUTE_UNUSED, virNodeDeviceFree(dev); return 0; } +static int remoteDispatchStorageVolUpload(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *rerr, + remote_storage_vol_upload_args *args, + void *ret ATTRIBUTE_UNUSED) +{ + int rv = -1; + struct qemud_client_stream *stream = NULL; + virStorageVolPtr vol; + + vol = get_nonnull_storage_vol(conn, args->vol); + if (vol == NULL) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + stream = remoteCreateClientStream(conn, hdr); + if (!stream) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + if (virStorageVolUpload(vol, stream->st, + args->offset, args->length, + args->flags) < 0) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + if (remoteAddClientStream(client, stream, 0) < 0) { + remoteDispatchConnError(rerr, conn); + virStreamAbort(stream->st); + goto cleanup; + } + + rv = 0; + +cleanup: + if (vol) + virStorageVolFree(vol); + if (stream && rv != 0) + remoteFreeClientStream(client, stream); + return rv; +} +static int remoteDispatchStorageVolDownload(struct qemud_server *server ATTRIBUTE_UNUSED, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *rerr, + remote_storage_vol_download_args *args, + void *ret ATTRIBUTE_UNUSED) +{ + int rv = -1; + struct qemud_client_stream *stream = NULL; + virStorageVolPtr vol; + + vol = get_nonnull_storage_vol (conn, args->vol); + if (vol == NULL) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + stream = remoteCreateClientStream(conn, hdr); + if (!stream) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + if (virStorageVolDownload(vol, stream->st, + args->offset, args->length, + args->flags) < 0) { + remoteDispatchConnError(rerr, conn); + goto cleanup; + } + + if (remoteAddClientStream(client, stream, 1) < 0) { + remoteDispatchConnError(rerr, conn); + virStreamAbort(stream->st); + goto cleanup; + } + + rv = 0; + +cleanup: + if (vol) + virStorageVolFree(vol); + if (stream && rv != 0) + remoteFreeClientStream(client, stream); + return rv; +} /*************************** diff --git a/daemon/remote_dispatch_args.h b/daemon/remote_dispatch_args.h index 15fa1a0..f9537d7 100644 --- a/daemon/remote_dispatch_args.h +++ b/daemon/remote_dispatch_args.h @@ -176,3 +176,5 @@ remote_domain_set_blkio_parameters_args val_remote_domain_set_blkio_parameters_args; remote_domain_get_blkio_parameters_args val_remote_domain_get_blkio_parameters_args; remote_domain_migrate_set_max_speed_args val_remote_domain_migrate_set_max_speed_args; + remote_storage_vol_upload_args val_remote_storage_vol_upload_args; + remote_storage_vol_download_args val_remote_storage_vol_download_args; diff --git a/daemon/remote_dispatch_prototypes.h b/daemon/remote_dispatch_prototypes.h index 3fcf87c..18bf41d 100644 --- a/daemon/remote_dispatch_prototypes.h +++ b/daemon/remote_dispatch_prototypes.h @@ -1538,6 +1538,14 @@ static int remoteDispatchStorageVolDelete( remote_error *err, remote_storage_vol_delete_args *args, void *ret); +static int remoteDispatchStorageVolDownload( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + remote_storage_vol_download_args *args, + void *ret); static int remoteDispatchStorageVolDumpXml( struct qemud_server *server, struct qemud_client *client, @@ -1586,6 +1594,14 @@ static int remoteDispatchStorageVolLookupByPath( remote_error *err, remote_storage_vol_lookup_by_path_args *args, remote_storage_vol_lookup_by_path_ret *ret); +static int remoteDispatchStorageVolUpload( + struct qemud_server *server, + struct qemud_client *client, + virConnectPtr conn, + remote_message_header *hdr, + remote_error *err, + remote_storage_vol_upload_args *args, + void *ret); static int remoteDispatchStorageVolWipe( struct qemud_server *server, struct qemud_client *client, diff --git a/daemon/remote_dispatch_table.h b/daemon/remote_dispatch_table.h index c5f6653..b39f7c2 100644 --- a/daemon/remote_dispatch_table.h +++ b/daemon/remote_dispatch_table.h @@ -1042,3 +1042,13 @@ .args_filter = (xdrproc_t) xdr_remote_domain_migrate_set_max_speed_args, .ret_filter = (xdrproc_t) xdr_void, }, +{ /* StorageVolUpload => 208 */ + .fn = (dispatch_fn) remoteDispatchStorageVolUpload, + .args_filter = (xdrproc_t) xdr_remote_storage_vol_upload_args, + .ret_filter = (xdrproc_t) xdr_void, +}, +{ /* StorageVolDownload => 209 */ + .fn = (dispatch_fn) remoteDispatchStorageVolDownload, + .args_filter = (xdrproc_t) xdr_remote_storage_vol_download_args, + .ret_filter = (xdrproc_t) xdr_void, +}, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 40da5c2..eaf53ef 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8484,7 +8484,6 @@ done: static struct private_stream_data * remoteStreamOpen(virStreamPtr st, - int output ATTRIBUTE_UNUSED, unsigned int proc_nr, unsigned int serial) { @@ -9006,7 +9005,7 @@ remoteDomainMigratePrepareTunnel(virConnectPtr conn, remoteDriverLock(priv); - if (!(privst = remoteStreamOpen(st, 1, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL, priv->counter))) + if (!(privst = remoteStreamOpen(st, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL, priv->counter))) goto done; st->driver = &remoteStreamDrv; @@ -9592,6 +9591,91 @@ done: return rv; } +static int +remoteStorageVolUpload(virStorageVolPtr vol, + virStreamPtr st, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + struct private_data *priv = vol->conn->privateData; + struct private_stream_data *privst = NULL; + int rv = -1; + remote_storage_vol_upload_args args; + + remoteDriverLock(priv); + + if (!(privst = remoteStreamOpen(st, + REMOTE_PROC_STORAGE_VOL_UPLOAD, + priv->counter))) + goto done; + + st->driver = &remoteStreamDrv; + st->privateData = privst; + + make_nonnull_storage_vol(&args.vol, vol); + args.offset = offset; + args.length = length; + args.flags = flags; + + if (call (vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_UPLOAD, + (xdrproc_t) xdr_remote_storage_vol_upload_args, (char *) &args, + (xdrproc_t) xdr_void, NULL) == -1) { + remoteStreamRelease(st); + goto done; + } + + rv = 0; + +done: + remoteDriverUnlock(priv); + + return rv; +} + + +static int +remoteStorageVolDownload(virStorageVolPtr vol, + virStreamPtr st, + unsigned long long offset, + unsigned long long length, + unsigned int flags) +{ + struct private_data *priv = vol->conn->privateData; + struct private_stream_data *privst = NULL; + int rv = -1; + remote_storage_vol_download_args args; + + remoteDriverLock(priv); + + if (!(privst = remoteStreamOpen(st, + REMOTE_PROC_STORAGE_VOL_DOWNLOAD, + priv->counter))) + goto done; + + st->driver = &remoteStreamDrv; + st->privateData = privst; + + make_nonnull_storage_vol(&args.vol, vol); + args.offset = offset; + args.length = length; + args.flags = flags; + + if (call (vol->conn, priv, 0, REMOTE_PROC_STORAGE_VOL_DOWNLOAD, + (xdrproc_t) xdr_remote_storage_vol_download_args, (char *) &args, + (xdrproc_t) xdr_void, NULL) == -1) { + remoteStreamRelease(st); + goto done; + } + + rv = 0; + +done: + remoteDriverUnlock(priv); + + return rv; +} + static int remoteDomainOpenConsole(virDomainPtr dom, @@ -9606,7 +9690,7 @@ remoteDomainOpenConsole(virDomainPtr dom, remoteDriverLock(priv); - if (!(privst = remoteStreamOpen(st, 1, REMOTE_PROC_DOMAIN_OPEN_CONSOLE, priv->counter))) + if (!(privst = remoteStreamOpen(st, REMOTE_PROC_DOMAIN_OPEN_CONSOLE, priv->counter))) goto done; st->driver = &remoteStreamDrv; @@ -11277,6 +11361,8 @@ static virStorageDriver storage_driver = { .volLookupByPath = remoteStorageVolLookupByPath, .volCreateXML = remoteStorageVolCreateXML, .volCreateXMLFrom = remoteStorageVolCreateXMLFrom, + .volDownload = remoteStorageVolDownload, + .volUpload = remoteStorageVolUpload, .volDelete = remoteStorageVolDelete, .volWipe = remoteStorageVolWipe, .volGetInfo = remoteStorageVolGetInfo, diff --git a/src/remote/remote_protocol.c b/src/remote/remote_protocol.c index 7ecea9d..5604371 100644 --- a/src/remote/remote_protocol.c +++ b/src/remote/remote_protocol.c @@ -3872,6 +3872,36 @@ xdr_remote_domain_open_console_args (XDR *xdrs, remote_domain_open_console_args } bool_t +xdr_remote_storage_vol_upload_args (XDR *xdrs, remote_storage_vol_upload_args *objp) +{ + + if (!xdr_remote_nonnull_storage_vol (xdrs, &objp->vol)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->offset)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->length)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t +xdr_remote_storage_vol_download_args (XDR *xdrs, remote_storage_vol_download_args *objp) +{ + + if (!xdr_remote_nonnull_storage_vol (xdrs, &objp->vol)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->offset)) + return FALSE; + if (!xdr_uint64_t (xdrs, &objp->length)) + return FALSE; + if (!xdr_u_int (xdrs, &objp->flags)) + return FALSE; + return TRUE; +} + +bool_t xdr_remote_procedure (XDR *xdrs, remote_procedure *objp) { diff --git a/src/remote/remote_protocol.h b/src/remote/remote_protocol.h index 87de0da..d9bf151 100644 --- a/src/remote/remote_protocol.h +++ b/src/remote/remote_protocol.h @@ -2184,6 +2184,22 @@ struct remote_domain_open_console_args { u_int flags; }; typedef struct remote_domain_open_console_args remote_domain_open_console_args; + +struct remote_storage_vol_upload_args { + remote_nonnull_storage_vol vol; + uint64_t offset; + uint64_t length; + u_int flags; +}; +typedef struct remote_storage_vol_upload_args remote_storage_vol_upload_args; + +struct remote_storage_vol_download_args { + remote_nonnull_storage_vol vol; + uint64_t offset; + uint64_t length; + u_int flags; +}; +typedef struct remote_storage_vol_download_args remote_storage_vol_download_args; #define REMOTE_PROGRAM 0x20008086 #define REMOTE_PROTOCOL_VERSION 1 @@ -2395,6 +2411,8 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS = 205, REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, + REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209, }; typedef enum remote_procedure remote_procedure; @@ -2776,6 +2794,8 @@ extern bool_t xdr_remote_domain_snapshot_current_ret (XDR *, remote_domain_snap extern bool_t xdr_remote_domain_revert_to_snapshot_args (XDR *, remote_domain_revert_to_snapshot_args*); extern bool_t xdr_remote_domain_snapshot_delete_args (XDR *, remote_domain_snapshot_delete_args*); extern bool_t xdr_remote_domain_open_console_args (XDR *, remote_domain_open_console_args*); +extern bool_t xdr_remote_storage_vol_upload_args (XDR *, remote_storage_vol_upload_args*); +extern bool_t xdr_remote_storage_vol_download_args (XDR *, remote_storage_vol_download_args*); extern bool_t xdr_remote_procedure (XDR *, remote_procedure*); extern bool_t xdr_remote_message_type (XDR *, remote_message_type*); extern bool_t xdr_remote_message_status (XDR *, remote_message_status*); @@ -3131,6 +3151,8 @@ extern bool_t xdr_remote_domain_snapshot_current_ret (); extern bool_t xdr_remote_domain_revert_to_snapshot_args (); extern bool_t xdr_remote_domain_snapshot_delete_args (); extern bool_t xdr_remote_domain_open_console_args (); +extern bool_t xdr_remote_storage_vol_upload_args (); +extern bool_t xdr_remote_storage_vol_download_args (); extern bool_t xdr_remote_procedure (); extern bool_t xdr_remote_message_type (); extern bool_t xdr_remote_message_status (); diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 7310689..675eccd 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1926,6 +1926,21 @@ struct remote_domain_open_console_args { unsigned int flags; }; +struct remote_storage_vol_upload_args { + remote_nonnull_storage_vol vol; + unsigned hyper offset; + unsigned hyper length; + unsigned int flags; +}; + +struct remote_storage_vol_download_args { + remote_nonnull_storage_vol vol; + unsigned hyper offset; + unsigned hyper length; + unsigned int flags; +}; + + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -2159,7 +2174,9 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_SET_MEMORY_FLAGS = 204, REMOTE_PROC_DOMAIN_SET_BLKIO_PARAMETERS = 205, REMOTE_PROC_DOMAIN_GET_BLKIO_PARAMETERS = 206, - REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207 + REMOTE_PROC_DOMAIN_MIGRATE_SET_MAX_SPEED = 207, + REMOTE_PROC_STORAGE_VOL_UPLOAD = 208, + REMOTE_PROC_STORAGE_VOL_DOWNLOAD = 209 /* * Notice how the entries are grouped in sets of 10 ? diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 81889c3..7dabb33 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1414,6 +1414,18 @@ struct remote_domain_open_console_args { remote_string devname; u_int flags; }; +struct remote_storage_vol_upload_args { + remote_nonnull_storage_vol vol; + uint64_t offset; + uint64_t length; + u_int flags; +}; +struct remote_storage_vol_download_args { + remote_nonnull_storage_vol vol; + uint64_t offset; + uint64_t length; + u_int flags; +}; struct remote_message_header { u_int prog; u_int vers; -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
* daemon/remote.c, src/remote/remote_driver.c: Implementation of storage vol upload/download APIs * src/remote/remote_protocol.x: Wire protocol definition for upload/download * daemon/remote_dispatch_args.h, daemon/remote_dispatch_prototypes.h, daemon/remote_dispatch_table.h, src/remote/remote_protocol.h, src/remote/remote_protocol.c: Re-generate --- daemon/remote.c | 92 +++++++++++++++++++++++++++++++++++ daemon/remote_dispatch_args.h | 2 + daemon/remote_dispatch_prototypes.h | 16 ++++++ daemon/remote_dispatch_table.h | 10 ++++ src/remote/remote_driver.c | 92 +++++++++++++++++++++++++++++++++- src/remote/remote_protocol.c | 30 +++++++++++ src/remote/remote_protocol.h | 22 ++++++++ src/remote/remote_protocol.x | 19 +++++++- src/remote_protocol-structs | 12 +++++ 9 files changed, 291 insertions(+), 4 deletions(-)
ACK. I can deal with the src/remote_protocol-structs fallout if your patch is pushed before my patch gets reviewed: https://www.redhat.com/archives/libvir-list/2011-March/msg00519.html -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
The daemon loops over the linked list of streams when a client quits, closing any that the client hadn't already closed. Except it didn't ever move to the next element in the list! * daemon/stream.c: Fix loop over linked list of streams --- daemon/stream.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/daemon/stream.c b/daemon/stream.c index 967aea2..b94e3df 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -343,6 +343,7 @@ remoteRemoveClientStream(struct qemud_client *client, filter->next = filter->next->next; break; } + filter = filter->next; } } -- 1.7.4
On 03/23/2011 11:36 AM, Daniel P. Berrange wrote:
The daemon loops over the linked list of streams when a client quits, closing any that the client hadn't already closed. Except it didn't ever move to the next element in the list!
* daemon/stream.c: Fix loop over linked list of streams --- daemon/stream.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/daemon/stream.c b/daemon/stream.c index 967aea2..b94e3df 100644 --- a/daemon/stream.c +++ b/daemon/stream.c @@ -343,6 +343,7 @@ remoteRemoveClientStream(struct qemud_client *client, filter->next = filter->next->next; break; } + filter = filter->next; } }
No change from v2; ACK still applies. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Jim Fehlig -
Wen Congyang