On 06/23/2017 09:33 AM, Peter Krempa wrote:
> Allow specifying offset to read an arbitrary position in the file. This
> warrants a rename to virStorageFileRead.
> ---
> src/qemu/qemu_driver.c | 3 +--
> src/storage/storage_backend.h | 9 +++++----
> src/storage/storage_backend_fs.c | 20 +++++++++++++------
> src/storage/storage_backend_gluster.c | 37 ++++++++++++++++++++++-------------
> src/storage/storage_source.c | 30 +++++++++++++++-------------
> src/storage/storage_source.h | 7 ++++---
> 6 files changed, 63 insertions(+), 43 deletions(-)
>
[...]
> diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c
> index 93dce4042..8ea7e603c 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
> @@ -151,20 +151,20 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
>
>
> static ssize_t
> -virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
> - const char *name,
> - ssize_t maxlen,
> - char **buf)
> +virStorageBackendGlusterRead(glfs_fd_t *fd,
> + const char *name,
> + size_t len,
> + char **buf)
> {
> char *s;
> size_t nread = 0;
>
> - if (VIR_ALLOC_N(*buf, maxlen) < 0)
> + if (VIR_ALLOC_N(*buf, len) < 0)
> return -1;
>
> s = *buf;
> - while (maxlen) {
> - ssize_t r = glfs_read(fd, s, maxlen, 0);
> + while (len) {
> + ssize_t r = glfs_read(fd, s, len, 0);
> if (r < 0 && errno == EINTR)
> continue;
> if (r < 0) {
> @@ -175,7 +175,7 @@ virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
> if (r == 0)
> return nread;
> s += r;
> - maxlen -= r;
> + len -= r;
> nread += r;
> }
> return nread;
> @@ -292,7 +292,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> goto cleanup;
> }
>
Should :
ssize_t len = VIR_STORAGE_MAX_HEADER;
change to size_t and a new variable "ssize_t read_len" be created and
used for the following and subsequent virStorageFileGetMetadataFromBuf
call? (although that also takes a size_t for the 3rd param).
> - if ((len = virStorageBackendGlusterReadHeader(fd, name, len, &header)) < 0)
> + if ((len = virStorageBackendGlusterRead(fd, name, len, &header)) < 0)
> goto cleanup;
>
> if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
> @@ -721,9 +721,10 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
>
>
> static ssize_t
> -virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
> - ssize_t max_len,
> - char **buf)
> +virStorageFileBackendGlusterRead(virStorageSourcePtr src,
> + size_t offset,
> + size_t len,
> + char **buf)
> {
> virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
> glfs_fd_t *fd = NULL;
> @@ -737,8 +738,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
> return -1;
> }
>
> - ret = virStorageBackendGlusterReadHeader(fd, src->path, max_len, buf);
> + if (offset > 0) {
> + if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) {
> + virReportSystemError(errno, _("cannot seek into '%s'"), src->path);
> + goto cleanup;
> + }
> + }
> +
> + ret = virStorageBackendGlusterRead(fd, src->path, len, buf);
>
> + cleanup:
> if (fd)
> glfs_close(fd);
>
> @@ -851,7 +860,7 @@ virStorageFileBackend virStorageFileBackendGluster = {
> .storageFileCreate = virStorageFileBackendGlusterCreate,
> .storageFileUnlink = virStorageFileBackendGlusterUnlink,
> .storageFileStat = virStorageFileBackendGlusterStat,
> - .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
> + .storageFileRead = virStorageFileBackendGlusterRead,
> .storageFileAccess = virStorageFileBackendGlusterAccess,
> .storageFileChown = virStorageFileBackendGlusterChown,
>
> diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
> index c91d629c8..130c2d2f3 100644
> --- a/src/storage/storage_source.c
> +++ b/src/storage/storage_source.c
> @@ -64,7 +64,7 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
> }
>
> return backend->storageFileGetUniqueIdentifier &&
> - backend->storageFileReadHeader &&
> + backend->storageFileRead &&
> backend->storageFileAccess;
> }
>
> @@ -263,10 +263,11 @@ virStorageFileStat(virStorageSourcePtr src,
>
>
> /**
> - * virStorageFileReadHeader: read the beginning bytes of a file into a buffer
> + * virStorageFileRead: read the beginning bytes of a file into a buffer
Probably should put the description below the args and restate since it
reading @len bytes of data starting at @offset
> *
> * @src: file structure pointing to the file
> - * @max_len: maximum number of bytes read from the storage file
> + * @offset: number of bytes to skip in the storage file
> + * @len: maximum number of bytes read from the storage file
> * @buf: buffer to read the data into. buffer shall be freed by caller)
> *
> * Returns the count of bytes read on success and -1 on failure, -2 if the
> @@ -274,9 +275,10 @@ virStorageFileStat(virStorageSourcePtr src,
> * Libvirt error is reported on failure.
> */
> ssize_t
> -virStorageFileReadHeader(virStorageSourcePtr src,
> - ssize_t max_len,
> - char **buf)
> +virStorageFileRead(virStorageSourcePtr src,
> + size_t offset,
> + size_t len,
> + char **buf)
> {
> ssize_t ret;
>
> @@ -286,18 +288,18 @@ virStorageFileReadHeader(virStorageSourcePtr src,
> return -1;
> }
>
> - if (!src->drv->backend->storageFileReadHeader) {
> + if (!src->drv->backend->storageFileRead) {
> virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("storage file header reading is not supported for "
> + _("storage file reading is not supported for "
> "storage type %s (protocol: %s)"),
> virStorageTypeToString(src->type),
> virStorageNetProtocolTypeToString(src->protocol));
> return -2;
> }
>
> - ret = src->drv->backend->storageFileReadHeader(src, max_len, buf);
> + ret = src->drv->backend->storageFileRead(src, offset, len, buf);
>
> - VIR_DEBUG("read of storage header %p: ret=%zd", src, ret);
> + VIR_DEBUG("read of storage %p: ret=%zd", src, ret);
"read %zd bytes from storage %p starting at offset %zu", ret, src, offset
With a few tweaks:
Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
John
[...]
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
