Re: [Qemu-block] [PATCH V3] block/nfs: add support for setting debug level

[Kevin Wolf]

Am 07.07.2015 um 08:50 hat Peter Lieven geschrieben:
> upcoming libnfs versions will support logging debug messages. Add
> support for it in qemu through a per-drive option.
> 
> Examples:
>  qemu -drive if=virtio,file=nfs://...,file.debug=2
>  qemu-img create -o debug=2 nfs://... 10G
> 
> Signed-off-by: Peter Lieven <address@hidden>

This patch does more than advertised in the commit message: It enables
NFS for blockdev-add, turning the options into a stable API.

'filename' is definitely not a good field name for an option taking an
NFS URL. What's more, a URL isn't really appropriate for QAPI either,
because it encodes structured information in a string rather than in
JSON objects. This part should work more like NBD, which has a
.bdrv_parse_filename implementation that splits a URL into individual
options.

But even with NBD, we weren't entirely sure if this is the right set of
options. The reason is that socket_optslist has a few more options than
just the hostname and port, and we would probably want to expose them in
blockdev-add. Not sure about NFS, it might be more limited there because
the connection is opened in libnfs.

Anyway, all of this isn't quite trivial to figure out, so I'm afraid we
need to revert the QAPI part before the 2.4 release.

Kevin



> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 7b2efb8..f43a1b1 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1381,9 +1381,9 @@
>  { 'enum': 'BlockdevDriver',
>    'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
>              'dmg', 'file', 'ftp', 'ftps', 'host_cdrom', 'host_device',
> -            'host_floppy', 'http', 'https', 'null-aio', 'null-co', 
> 'parallels',
> -            'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp', 'vdi', 'vhdx',
> -            'vmdk', 'vpc', 'vvfat' ] }
> +            'host_floppy', 'http', 'https', 'nfs', 'null-aio', 'null-co',
> +            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw', 'tftp',
> +            'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>  
>  ##
>  # @BlockdevOptionsBase
> @@ -1635,6 +1635,18 @@
>              '*vport': 'int',
>              '*segment': 'str' } }
>  
> +##
> +# @BlockdevOptionsNFS
> +#
> +# Driver specific block device options for NFS.
> +#
> +# @debug:       #optional set libnfs debug level (default: 0 = disabled)
> +#
> +# Since: 2.4
> +##
> +{ 'struct': 'BlockdevOptionsNFS',
> +  'base': 'BlockdevOptionsFile',
> +  'data': { '*debug': 'int' } }
>  
>  ##
>  # @BlkdebugEvent
> @@ -1816,7 +1828,7 @@
>        'https':      'BlockdevOptionsFile',
>  # TODO iscsi: Wait for structured options
>  # TODO nbd: Should take InetSocketAddress for 'host'?
> -# TODO nfs: Wait for structured options
> +      'nfs':        'BlockdevOptionsNFS',
>        'null-aio':   'BlockdevOptionsNull',
>        'null-co':    'BlockdevOptionsNull',
>        'parallels':  'BlockdevOptionsGenericFormat',
> -- 
> 1.9.1
>