Am 30.01.2014 um 15:23 schrieb Stefan Hajnoczi <stefa...@gmail.com>:
> On Thu, Jan 30, 2014 at 10:12:35AM +0100, Peter Lieven wrote: >> >> Am 30.01.2014 um 10:05 schrieb Stefan Hajnoczi <stefa...@gmail.com>: >> >>> On Wed, Jan 29, 2014 at 05:38:29PM +0100, Peter Lieven wrote: >>>> On 29.01.2014 17:19, Benoît Canet wrote: >>>>> Le Wednesday 29 Jan 2014 à 09:50:21 (+0100), Peter Lieven a écrit : >>>>>> This patch adds native support for accessing images on NFS >>>>>> shares without the requirement to actually mount the entire >>>>>> NFS share on the host. >>>>>> >>>>>> NFS Images can simply be specified by an url of the form: >>>>>> nfs://<host>/<export>/<filename>[?param=value[¶m2=value2[&...]]] >>>>>> >>>>>> For example: >>>>>> qemu-img create -f qcow2 nfs://10.0.0.1/qemu-images/test.qcow2 >>>>>> >>>>>> You need LibNFS from Ronnie Sahlberg available at: >>>>>> git://github.com/sahlberg/libnfs.git >>>>>> for this to work. >>>>>> >>>>>> During configure it is automatically probed for libnfs and support >>>>>> is enabled on-the-fly. You can forbid or enforce libnfs support >>>>>> with --disable-libnfs or --enable-libnfs respectively. >>>>>> >>>>>> Due to NFS restrictions you might need to execute your binaries >>>>>> as root, allow them to open priviledged ports (<1024) or specify >>>>>> insecure option on the NFS server. >>>>>> >>>>>> For additional information on ROOT vs. non-ROOT operation and URL >>>>>> format + parameters see: >>>>>> https://raw.github.com/sahlberg/libnfs/master/README >>>>>> >>>>>> Supported by qemu are the uid, gid and tcp-syncnt URL parameters. >>>>>> >>>>>> LibNFS currently support NFS version 3 only. >>>>>> >>>>>> Signed-off-by: Peter Lieven <p...@kamp.de> >>>>>> --- >>>>>> MAINTAINERS | 5 + >>>>>> block/Makefile.objs | 1 + >>>>>> block/nfs.c | 440 >>>>>> +++++++++++++++++++++++++++++++++++++++++++++++++++ >>>>>> configure | 26 +++ >>>>>> qapi-schema.json | 1 + >>>>>> 5 files changed, 473 insertions(+) >>>>>> create mode 100644 block/nfs.c >>>>>> >>>>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>>>> index fb53242..f8411f9 100644 >>>>>> --- a/MAINTAINERS >>>>>> +++ b/MAINTAINERS >>>>>> @@ -936,6 +936,11 @@ M: Peter Lieven <p...@kamp.de> >>>>>> S: Supported >>>>>> F: block/iscsi.c >>>>>> +NFS >>>>>> +M: Peter Lieven <p...@kamp.de> >>>>>> +S: Maintained >>>>>> +F: block/nfs.c >>>>>> + >>>>>> SSH >>>>>> M: Richard W.M. Jones <rjo...@redhat.com> >>>>>> S: Supported >>>>>> diff --git a/block/Makefile.objs b/block/Makefile.objs >>>>>> index 4e8c91e..e254a21 100644 >>>>>> --- a/block/Makefile.objs >>>>>> +++ b/block/Makefile.objs >>>>>> @@ -12,6 +12,7 @@ block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o >>>>>> ifeq ($(CONFIG_POSIX),y) >>>>>> block-obj-y += nbd.o nbd-client.o sheepdog.o >>>>>> block-obj-$(CONFIG_LIBISCSI) += iscsi.o >>>>>> +block-obj-$(CONFIG_LIBNFS) += nfs.o >>>>>> block-obj-$(CONFIG_CURL) += curl.o >>>>>> block-obj-$(CONFIG_RBD) += rbd.o >>>>>> block-obj-$(CONFIG_GLUSTERFS) += gluster.o >>>>>> diff --git a/block/nfs.c b/block/nfs.c >>>>>> new file mode 100644 >>>>>> index 0000000..2bbf7a2 >>>>>> --- /dev/null >>>>>> +++ b/block/nfs.c >>>>>> @@ -0,0 +1,440 @@ >>>>>> +/* >>>>>> + * QEMU Block driver for native access to files on NFS shares >>>>>> + * >>>>>> + * Copyright (c) 2014 Peter Lieven <p...@kamp.de> >>>>>> + * >>>>>> + * Permission is hereby granted, free of charge, to any person >>>>>> obtaining a copy >>>>>> + * of this software and associated documentation files (the >>>>>> "Software"), to deal >>>>>> + * in the Software without restriction, including without limitation >>>>>> the rights >>>>>> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or >>>>>> sell >>>>>> + * copies of the Software, and to permit persons to whom the Software is >>>>>> + * furnished to do so, subject to the following conditions: >>>>>> + * >>>>>> + * The above copyright notice and this permission notice shall be >>>>>> included in >>>>>> + * all copies or substantial portions of the Software. >>>>>> + * >>>>>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, >>>>>> EXPRESS OR >>>>>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF >>>>>> MERCHANTABILITY, >>>>>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT >>>>>> SHALL >>>>>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR >>>>>> OTHER >>>>>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, >>>>>> ARISING FROM, >>>>>> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER >>>>>> DEALINGS IN >>>>>> + * THE SOFTWARE. >>>>>> + */ >>>>>> + >>>>>> +#include "config-host.h" >>>>>> + >>>>>> +#include <poll.h> >>>>>> +#include "qemu-common.h" >>>>>> +#include "qemu/config-file.h" >>>>>> +#include "qemu/error-report.h" >>>>>> +#include "block/block_int.h" >>>>>> +#include "trace.h" >>>>>> +#include "qemu/iov.h" >>>>>> +#include "qemu/uri.h" >>>>>> +#include "sysemu/sysemu.h" >>>>>> +#include <nfsc/libnfs.h> >>>>>> + >>>>>> +typedef struct NFSClient { >>>>>> + struct nfs_context *context; >>>>>> + struct nfsfh *fh; >>>>>> + int events; >>>>>> + bool has_zero_init; >>>>>> +} NFSClient; >>>>>> + >>>>>> +typedef struct NFSRPC { >>>>>> + int status; >>>>>> + int complete; >>>>>> + QEMUIOVector *iov; >>>>>> + struct stat *st; >>>>>> + Coroutine *co; >>>>>> + QEMUBH *bh; >>>>>> +} NFSRPC; >>>>>> + >>>>>> +static void nfs_process_read(void *arg); >>>>>> +static void nfs_process_write(void *arg); >>>>>> + >>>>>> +static void nfs_set_events(NFSClient *client) >>>>>> +{ >>>>>> + int ev = nfs_which_events(client->context); >>>>>> + if (ev != client->events) { >>>>>> + qemu_aio_set_fd_handler(nfs_get_fd(client->context), >>>>>> + (ev & POLLIN) ? nfs_process_read : NULL, >>>>>> + (ev & POLLOUT) ? nfs_process_write : NULL, >>>>>> + client); >>>>>> + >>>>>> + } >>>>>> + client->events = ev; >>>>>> +} >>>>>> + >>>>>> +static void nfs_process_read(void *arg) >>>>>> +{ >>>>>> + NFSClient *client = arg; >>>>>> + nfs_service(client->context, POLLIN); >>>>>> + nfs_set_events(client); >>>>>> +} >>>>>> + >>>>>> +static void nfs_process_write(void *arg) >>>>>> +{ >>>>>> + NFSClient *client = arg; >>>>>> + nfs_service(client->context, POLLOUT); >>>>>> + nfs_set_events(client); >>>>>> +} >>>>>> + >>>>>> +static void nfs_co_init_task(NFSClient *client, NFSRPC *task) >>>>>> +{ >>>>>> + *task = (NFSRPC) { >>>>>> + .co = qemu_coroutine_self(), >>>>>> + }; >>>>>> +} >>>>>> + >>>>>> +static void nfs_co_generic_bh_cb(void *opaque) >>>>>> +{ >>>>>> + NFSRPC *task = opaque; >>>>>> + qemu_bh_delete(task->bh); >>>>>> + qemu_coroutine_enter(task->co, NULL); >>>>>> +} >>>>>> + >>>>>> +static void >>>>>> +nfs_co_generic_cb(int status, struct nfs_context *nfs, void *data, >>>>>> + void *private_data) >>>>>> +{ >>>>>> + NFSRPC *task = private_data; >>>>>> + task->complete = 1; >>>>>> + task->status = status; >>>>>> + if (task->status > 0 && task->iov) { >>>>>> + if (task->status <= task->iov->size) { >>>>> It feel very odd to compare something named status with something named >>>>> size. >>>>> >>>>>> + qemu_iovec_from_buf(task->iov, 0, data, task->status); >>>>>> + } else { >>>>>> + task->status = -EIO; >>>>>> + } >>>>>> + } >>>>>> + if (task->status == 0 && task->st) { >>>>>> + memcpy(task->st, data, sizeof(struct stat)); >>>>>> + } >>>>>> + if (task->co) { >>>>>> + task->bh = qemu_bh_new(nfs_co_generic_bh_cb, task); >>>>>> + qemu_bh_schedule(task->bh); >>>>>> + } >>>>>> +} >>>>>> + >>>>>> +static int coroutine_fn nfs_co_readv(BlockDriverState *bs, >>>>>> + int64_t sector_num, int nb_sectors, >>>>>> + QEMUIOVector *iov) >>>>>> +{ >>>>>> + NFSClient *client = bs->opaque; >>>>>> + NFSRPC task; >>>>>> + >>>>>> + nfs_co_init_task(client, &task); >>>>>> + task.iov = iov; >>>>>> + >>>>>> + if (nfs_pread_async(client->context, client->fh, >>>>>> + sector_num * BDRV_SECTOR_SIZE, >>>>>> + nb_sectors * BDRV_SECTOR_SIZE, >>>>>> + nfs_co_generic_cb, &task) != 0) { >>>>>> + return -ENOMEM; >>>>>> + } >>>>>> + >>>>>> + while (!task.complete) { >>>>>> + nfs_set_events(client); >>>>>> + qemu_coroutine_yield(); >>>>>> + } >>>>>> + >>>>>> + if (task.status < 0) { >>>>>> + return task.status; >>>>>> + } >>>>>> + >>>>>> + /* zero pad short reads */ >>>>>> + if (task.status < iov->size) { >>>>>> + qemu_iovec_memset(iov, task.status, 0, iov->size - task.status); >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int coroutine_fn nfs_co_writev(BlockDriverState *bs, >>>>>> + int64_t sector_num, int >>>>>> nb_sectors, >>>>>> + QEMUIOVector *iov) >>>>>> +{ >>>>>> + NFSClient *client = bs->opaque; >>>>>> + NFSRPC task; >>>>>> + char *buf = NULL; >>>>>> + >>>>>> + nfs_co_init_task(client, &task); >>>>>> + >>>>>> + buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE); >>>>>> + qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE); >>>>>> + >>>>>> + if (nfs_pwrite_async(client->context, client->fh, >>>>>> + sector_num * BDRV_SECTOR_SIZE, >>>>>> + nb_sectors * BDRV_SECTOR_SIZE, >>>>>> + buf, nfs_co_generic_cb, &task) != 0) { >>>>>> + g_free(buf); >>>>>> + return -ENOMEM; >>>>>> + } >>>>>> + >>>>>> + while (!task.complete) { >>>>>> + nfs_set_events(client); >>>>>> + qemu_coroutine_yield(); >>>>>> + } >>>>>> + >>>>>> + g_free(buf); >>>>>> + >>>>>> + if (task.status != nb_sectors * BDRV_SECTOR_SIZE) { >>>>>> + return task.status < 0 ? task.status : -EIO; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int coroutine_fn nfs_co_flush(BlockDriverState *bs) >>>>>> +{ >>>>>> + NFSClient *client = bs->opaque; >>>>>> + NFSRPC task; >>>>>> + >>>>>> + nfs_co_init_task(client, &task); >>>>>> + >>>>>> + if (nfs_fsync_async(client->context, client->fh, nfs_co_generic_cb, >>>>>> + &task) != 0) { >>>>>> + return -ENOMEM; >>>>>> + } >>>>>> + >>>>>> + while (!task.complete) { >>>>>> + nfs_set_events(client); >>>>>> + qemu_coroutine_yield(); >>>>>> + } >>>>>> + >>>>>> + return task.status; >>>>>> +} >>>>>> + >>>>>> +/* TODO Convert to fine grained options */ >>>>>> +static QemuOptsList runtime_opts = { >>>>>> + .name = "nfs", >>>>>> + .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), >>>>>> + .desc = { >>>>>> + { >>>>>> + .name = "filename", >>>>>> + .type = QEMU_OPT_STRING, >>>>>> + .help = "URL to the NFS file", >>>>>> + }, >>>>>> + { /* end of list */ } >>>>>> + }, >>>>>> +}; >>>>>> + >>>>>> +static void nfs_client_close(NFSClient *client) >>>>>> +{ >>>>>> + if (client->context) { >>>>>> + if (client->fh) { >>>>>> + nfs_close(client->context, client->fh); >>>>>> + } >>>>>> + qemu_aio_set_fd_handler(nfs_get_fd(client->context), NULL, >>>>>> NULL, NULL); >>>>>> + nfs_destroy_context(client->context); >>>>>> + } >>>>>> + memset(client, 0, sizeof(NFSClient)); >>>>>> +} >>>>>> + >>>>>> +static void nfs_file_close(BlockDriverState *bs) >>>>>> +{ >>>>>> + NFSClient *client = bs->opaque; >>>>>> + nfs_client_close(client); >>>>>> +} >>>>>> + >>>>>> +static int64_t nfs_.cilclient_open(NFSClient *client, const char >>>>>> *filename, >>>>>> + int flags, Error **errp) >>>>>> +{ >>>>>> + int ret = -EINVAL, i; >>>>>> + struct stat st; >>>>>> + URI *uri; >>>>>> + QueryParams *qp = NULL; >>>>>> + char *file = NULL, *strp = NULL; >>>>>> + >>>>>> + uri = uri_parse(filename); >>>>>> + if (!uri) { >>>>>> + error_setg(errp, "Invalid URL specified"); >>>>>> + goto fail; >>>>> Should this be goto out since we don't have done nfs_init_context yet ? >>>>>> + } >>>>>> + strp = strrchr(uri->path, '/'); >>>>>> + if (strp == NULL) { >>>>>> + error_setg(errp, "Invalid URL specified"); >>>>>> + goto fail; >>>>> goto out ? >>>>>> + } >>>>>> + file = g_strdup(strp); >>>>>> + *strp = 0; >>>>>> + >>>>>> + client->context = nfs_init_context(); >>>>>> + if (client->context == NULL) { >>>>>> + error_setg(errp, "Failed to init NFS context"); >>>>>> + goto fail; >>>>>> + } >>>>>> + >>>>>> + qp = query_params_parse(uri->query); >>>>>> + for (i = 0; i < qp->n; i++) { >>>>>> + if (!qp->p[i].value) { >>>>>> + error_setg(errp, "Value for NFS parameter expected: %s", >>>>>> + qp->p[i].name); >>>>>> + goto fail; >>>>>> + } >>>>>> + if (!strncmp(qp->p[i].name, "uid", 3)) { >>>>>> + nfs_set_uid(client->context, atoi(qp->p[i].value)); >>>>>> + } else if (!strncmp(qp->p[i].name, "gid", 3)) { >>>>>> + nfs_set_gid(client->context, atoi(qp->p[i].value)); >>>>>> + } else if (!strncmp(qp->p[i].name, "tcp-syncnt", 10)) { >>>>>> + nfs_set_tcp_syncnt(client->context, atoi(qp->p[i].value)); >>>>>> + } else { >>>>>> + error_setg(errp, "Unknown NFS parameter name: %s", >>>>>> + qp->p[i].name); >>>>>> + goto fail; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + ret = nfs_mount(client->context, uri->server, uri->path); >>>>>> + if (ret < 0) { >>>>>> + error_setg(errp, "Failed to mount nfs share: %s", >>>>>> + nfs_get_error(client->context)); >>>>>> + goto fail; >>>>>> + } >>>>>> + >>>>>> + if (flags & O_CREAT) { >>>>>> + ret = nfs_creat(client->context, file, 0600, &client->fh); >>>>>> + if (ret < 0) { >>>>>> + error_setg(errp, "Failed to create file: %s", >>>>>> + nfs_get_error(client->context)); >>>>>> + goto fail; >>>>>> + } >>>>>> + } else { >>>>>> + ret = nfs_open(client->context, file, flags, &client->fh); >>>>>> + if (ret < 0) { >>>>>> + error_setg(errp, "Failed to open file : %s", >>>>>> + nfs_get_error(client->context)); >>>>>> + goto fail; >>>>>> + } >>>>>> + } >>>>>> + >>>>>> + ret = nfs_fstat(client->context, client->fh, &st); >>>>>> + if (ret < 0) { >>>>>> + error_setg(errp, "Failed to fstat file: %s", >>>>>> + nfs_get_error(client->context)); >>>>>> + goto fail; >>>>>> + } >>>>>> + >>>>>> + ret = DIV_ROUND_UP(st.st_size, BDRV_SECTOR_SIZE); >>>>>> + client->has_zero_init = S_ISREG(st.st_mode); >>>>>> + goto out; >>>>>> +fail: >>>>>> + nfs_client_close(client); >>>>>> +out: >>>>>> + if (qp) { >>>>>> + query_params_free(qp); >>>>>> + } >>>>>> + uri_free(uri); >>>>>> + g_free(file); >>>>>> + return ret; >>>>>> +} >>>>>> + >>>>>> +static int nfs_file_open(BlockDriverState *bs, QDict *options, int >>>>>> flags, >>>>>> + Error **errp) { >>>>>> + NFSClient *client = bs->opaque; >>>>>> + int64_t ret; >>>>>> + QemuOpts *opts; >>>>>> + Error *local_err = NULL; >>>>>> + >>>>>> + opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); >>>>>> + qemu_opts_absorb_qdict(opts, options, &local_err); >>>>>> + if (error_is_set(&local_err)) { >>>>>> + qerror_report_err(local_err); >>>>> I have seen more usage of error_propagate(errp, local_err); in QEMU code. >>>>> Maybe I am missing the point. >>>>> >>>>>> + error_free(local_err); >>>>>> + return -EINVAL; >>>>>> + } >>>>>> + ret = nfs_client_open(client, qemu_opt_get(opts, "filename"), >>>>>> + (flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY, >>>>>> + errp); >>>>>> + if (ret < 0) { >>>>>> + return ret; >>>>>> + } >>>>>> + bs->total_sectors = ret; >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +static int nfs_file_create(const char *url, QEMUOptionParameter >>>>>> *options, >>>>>> + Error **errp) >>>>>> +{ >>>>>> + int ret = 0; >>>>>> + int64_t total_size = 0; >>>>>> + NFSClient *client = g_malloc0(sizeof(NFSClient)); >>>>>> + >>>>>> + /* Read out options */ >>>>>> + while (options && options->name) { >>>>>> + if (!strcmp(options->name, "size")) { >>>>>> + total_size = options->value.n; >>>>>> + } >>>>>> + options++; >>>>>> + } >>>>>> + >>>>>> + ret = nfs_client_open(client, url, O_CREAT, errp); >>>>>> + if (ret < 0) { >>>>>> + goto out; >>>>>> + } >>>>>> + ret = nfs_ftruncate(client->context, client->fh, total_size); >>>>>> +out: >>>>> Should the out: label be after nfs_client_close(client); since the code >>>>> will >>>>> jump to it on nfs_client_open failure ? >>>> You are right, but it doesn't hurt since you can call nfs_client_close >>>> multiple >>>> times. >>> >>> Will you send another revision addressing the other comments? >> >> Sorry, which other comments are you referring to? > > I replied to Benoit's email hilighting his questions which were not > addressed. Sorry, I missed the other comments. > >> Have you been able to test the patch without the bad commit? > > Waiting for a final version to test. I cannot do that, unfortunately. Ronnie, can you revert the bad commit and leave it for the 2.0 development. It would be good if you bump the version to 1.9.2 so I can check for that in the configure script Peter
