Re: [Qemu-devel] [PATCHv9 1/5] block: add native support for NFS

[Kevin Wolf <kwolf@redhat.com>]

Am 03.02.2014 um 10:26 hat Peter Lieven geschrieben:
> 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[&param2=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 <pl@kamp.de>

Some minor comments in case you need to respin for another reason:

>  MAINTAINERS         |    5 +
>  block/Makefile.objs |    1 +
>  block/nfs.c         |  439 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  configure           |   26 +++
>  qapi-schema.json    |    1 +
>  5 files changed, 472 insertions(+)
>  create mode 100644 block/nfs.c

> +typedef struct NFSRPC {
> +    int ret;
> +    int complete;

Should be bool.

> +    QEMUIOVector *iov;
> +    struct stat *st;
> +    Coroutine *co;
> +    QEMUBH *bh;
> +} NFSRPC;

> +static BlockDriver bdrv_nfs = {
> +    .format_name     = "nfs",
> +    .protocol_name   = "nfs",
> +
> +    .instance_size   = sizeof(NFSClient),
> +    .bdrv_needs_filename = true,
> +    .bdrv_has_zero_init = nfs_has_zero_init,
> +    .bdrv_get_allocated_file_size = nfs_get_allocated_file_size,
> +    .bdrv_truncate = nfs_file_truncate,

This group of options could have used alignment for = as well. :-)

> +    .bdrv_file_open  = nfs_file_open,
> +    .bdrv_close      = nfs_file_close,
> +    .bdrv_create     = nfs_file_create,
> +
> +    .bdrv_co_readv         = nfs_co_readv,
> +    .bdrv_co_writev        = nfs_co_writev,
> +    .bdrv_co_flush_to_disk = nfs_co_flush,
> +};

Kevin