* [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS
@ 2016-10-27 10:43 Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 1/2] block/nfs: Introduce runtime_opts in NFS Ashijeet Acharya
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ashijeet Acharya @ 2016-10-27 10:43 UTC (permalink / raw)
To: kwolf
Cc: eblake, pl, jcody, mreitz, armbru, qemu-devel, qemu-block,
Ashijeet Acharya
Previously posted series patches:
v2: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg05844.html
v1: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04487.html
This series adds blockdev-add support for NFS block driver.
Patch 1 helps to prepare NFS driver to make use of several runtime_opts
as they appear in the URI. This will make NFS to do things similar to
the way other drivers available in the block layer do.
Patch 2 helps to allow blockdev-add support for the NFS block driver
by making the NFS option available.
Changes in v3:
- minor coding style fix
- set ret=-EINVAL in nfs_parse_uri()
- fix the bug of setting errp twice
- make all error paths 'goto fail'
- pass 0 as a default value in qemu_opt_get_number()
- drop nfs_set_pagecache_ttl()
- introduce new enum NFSTransport and set 'type' to use it NFSServer
- mention default values of query parameters
- change the names of query parameters in JSON
Changes in v2:
- drop strcmp() condition check for host and path in nfs_parse_uri()
- drop "export" completely
- initialize client->context bedore setting query parameters
- fix the QDict options being passed to nfs_client_open() and make use of url
Ashijeet Acharya (2):
block/nfs: Introduce runtime_opts in NFS
qapi: allow blockdev-add for NFS
block/nfs.c | 331 ++++++++++++++++++++++++++++++++++++---------------
qapi/block-core.json | 77 +++++++++++-
2 files changed, 311 insertions(+), 97 deletions(-)
--
2.6.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v3 1/2] block/nfs: Introduce runtime_opts in NFS
2016-10-27 10:43 [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS Ashijeet Acharya
@ 2016-10-27 10:43 ` Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS Ashijeet Acharya
2016-10-27 16:43 ` [Qemu-devel] [PATCH v3 0/2] block: " Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Ashijeet Acharya @ 2016-10-27 10:43 UTC (permalink / raw)
To: kwolf
Cc: eblake, pl, jcody, mreitz, armbru, qemu-devel, qemu-block,
Ashijeet Acharya
Make NFS block driver use various fine grained runtime_opts.
Set .bdrv_parse_filename() to nfs_parse_filename() and introduce two
new functions nfs_parse_filename() and nfs_parse_uri() to help parsing
the URI.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
block/nfs.c | 331 +++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 238 insertions(+), 93 deletions(-)
diff --git a/block/nfs.c b/block/nfs.c
index 8602a44..739ae8a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -35,8 +35,12 @@
#include "qemu/uri.h"
#include "qemu/cutils.h"
#include "sysemu/sysemu.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qint.h"
+#include "qapi/qmp/qstring.h"
#include <nfsc/libnfs.h>
+
#define QEMU_NFS_MAX_READAHEAD_SIZE 1048576
#define QEMU_NFS_MAX_PAGECACHE_SIZE (8388608 / NFS_BLKSIZE)
#define QEMU_NFS_MAX_DEBUG_LEVEL 2
@@ -61,6 +65,107 @@ typedef struct NFSRPC {
NFSClient *client;
} NFSRPC;
+static int nfs_parse_uri(const char *filename, QDict *options, Error **errp)
+{
+ URI *uri = NULL;
+ QueryParams *qp = NULL;
+ int ret = -EINVAL, i;
+
+ uri = uri_parse(filename);
+ if (!uri) {
+ error_setg(errp, "Invalid URI specified");
+ goto out;
+ }
+ if (strcmp(uri->scheme, "nfs") != 0) {
+ error_setg(errp, "URI scheme must be 'nfs'");
+ goto out;
+ }
+
+ if (!uri->server) {
+ error_setg(errp, "missing hostname in URI");
+ goto out;
+ }
+
+ if (!uri->path) {
+ error_setg(errp, "missing file path in URI");
+ goto out;
+ }
+
+ qp = query_params_parse(uri->query);
+ if (!qp) {
+ error_setg(errp, "could not parse query parameters");
+ goto out;
+ }
+
+ qdict_put(options, "host", qstring_from_str(uri->server));
+ qdict_put(options, "path", qstring_from_str(uri->path));
+
+ 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 out;
+ }
+ if (parse_uint_full(qp->p[i].value, NULL, 0)) {
+ error_setg(errp, "Illegal value for NFS parameter: %s",
+ qp->p[i].name);
+ goto out;
+ }
+ if (!strcmp(qp->p[i].name, "uid")) {
+ qdict_put(options, "user",
+ qstring_from_str(qp->p[i].value));
+ } else if (!strcmp(qp->p[i].name, "gid")) {
+ qdict_put(options, "group",
+ qstring_from_str(qp->p[i].value));
+ } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
+ qdict_put(options, "tcp-syn-count",
+ qstring_from_str(qp->p[i].value));
+ } else if (!strcmp(qp->p[i].name, "readahead")) {
+ qdict_put(options, "readahead-size",
+ qstring_from_str(qp->p[i].value));
+ } else if (!strcmp(qp->p[i].name, "pagecache")) {
+ qdict_put(options, "page-cache-size",
+ qstring_from_str(qp->p[i].value));
+ } else if (!strcmp(qp->p[i].name, "debug")) {
+ qdict_put(options, "debug-level",
+ qstring_from_str(qp->p[i].value));
+ } else {
+ error_setg(errp, "Unknown NFS parameter name: %s",
+ qp->p[i].name);
+ goto out;
+ }
+ }
+ ret = 0;
+out:
+ if (qp) {
+ query_params_free(qp);
+ }
+ if (uri) {
+ uri_free(uri);
+ }
+ return ret;
+}
+
+static void nfs_parse_filename(const char *filename, QDict *options,
+ Error **errp)
+{
+ if (qdict_haskey(options, "host") ||
+ qdict_haskey(options, "path") ||
+ qdict_haskey(options, "user") ||
+ qdict_haskey(options, "group") ||
+ qdict_haskey(options, "tcp-syn-count") ||
+ qdict_haskey(options, "readahead-size") ||
+ qdict_haskey(options, "page-cache-size") ||
+ qdict_haskey(options, "debug-level")) {
+ error_setg(errp, "host/path/uid/gid/tcp-syncnt/readahead/pagecache"
+ "/debug and a filename may not be used at the same"
+ " time");
+ return;
+ }
+
+ nfs_parse_uri(filename, options, errp);
+}
+
static void nfs_process_read(void *arg);
static void nfs_process_write(void *arg);
@@ -228,15 +333,49 @@ static int coroutine_fn nfs_co_flush(BlockDriverState *bs)
return task.ret;
}
-/* TODO Convert to fine grained options */
static QemuOptsList runtime_opts = {
.name = "nfs",
.head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
.desc = {
{
- .name = "filename",
+ .name = "host",
.type = QEMU_OPT_STRING,
- .help = "URL to the NFS file",
+ .help = "Host to connect to",
+ },
+ {
+ .name = "path",
+ .type = QEMU_OPT_STRING,
+ .help = "Path of the image on the host",
+ },
+ {
+ .name = "uid",
+ .type = QEMU_OPT_NUMBER,
+ .help = "UID value to use when talking to the server",
+ },
+ {
+ .name = "gid",
+ .type = QEMU_OPT_NUMBER,
+ .help = "GID value to use when talking to the server",
+ },
+ {
+ .name = "tcp-syncnt",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Number of SYNs to send during the session establish",
+ },
+ {
+ .name = "readahead",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Set the readahead size in bytes",
+ },
+ {
+ .name = "pagecache",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Set the pagecache size in bytes",
+ },
+ {
+ .name = "debug",
+ .type = QEMU_OPT_NUMBER,
+ .help = "Set the NFS debug level (max 2)",
},
{ /* end of list */ }
},
@@ -279,25 +418,40 @@ static void nfs_file_close(BlockDriverState *bs)
nfs_client_close(client);
}
-static int64_t nfs_client_open(NFSClient *client, const char *filename,
+static int64_t nfs_client_open(NFSClient *client, QDict *options,
int flags, Error **errp, int open_flags)
{
- int ret = -EINVAL, i;
+ int ret = -EINVAL;
+ QemuOpts *opts = NULL;
+ Error *local_err = NULL;
struct stat st;
- URI *uri;
- QueryParams *qp = NULL;
char *file = NULL, *strp = NULL;
+ const char *host, *path;
+ unsigned long long val;
- uri = uri_parse(filename);
- if (!uri) {
- error_setg(errp, "Invalid URL specified");
+ opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
+ qemu_opts_absorb_qdict(opts, options, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ ret = -EINVAL;
goto fail;
}
- if (!uri->server) {
- error_setg(errp, "Invalid URL specified");
+
+ host = qemu_opt_get(opts, "host");
+ if (!host) {
+ ret = -EINVAL;
+ error_setg(errp, "No hostname was specified");
goto fail;
}
- strp = strrchr(uri->path, '/');
+
+ path = qemu_opt_get(opts, "path");
+ if (!path) {
+ ret = -EINVAL;
+ error_setg(errp, "No path was specified");
+ goto fail;
+ }
+
+ strp = strrchr(path, '/');
if (strp == NULL) {
error_setg(errp, "Invalid URL specified");
goto fail;
@@ -311,79 +465,76 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
goto fail;
}
- qp = query_params_parse(uri->query);
- for (i = 0; i < qp->n; i++) {
- unsigned long long val;
- if (!qp->p[i].value) {
- error_setg(errp, "Value for NFS parameter expected: %s",
- qp->p[i].name);
+ if (qemu_opt_get(opts, "uid")) {
+ nfs_set_uid(client->context,
+ qemu_opt_get_number(opts, "uid", 0));
+ }
+
+ if (qemu_opt_get(opts, "gid")) {
+ nfs_set_gid(client->context,
+ qemu_opt_get_number(opts, "gid", 0));
+ }
+
+ if (qemu_opt_get(opts, "tcp-syncnt")) {
+ nfs_set_tcp_syncnt(client->context,
+ qemu_opt_get_number(opts, "tcp-syncnt", 0));
+ }
+
+#ifdef LIBNFS_FEATURE_READAHEAD
+ if (qemu_opt_get(opts, "readahead")) {
+ if (open_flags & BDRV_O_NOCACHE) {
+ error_setg(errp, "Cannot enable NFS readahead "
+ "if cache.direct = on");
goto fail;
}
- if (parse_uint_full(qp->p[i].value, &val, 0)) {
- error_setg(errp, "Illegal value for NFS parameter: %s",
- qp->p[i].name);
- goto fail;
+ val = qemu_opt_get_number(opts, "readahead", 0);
+ if (val > QEMU_NFS_MAX_READAHEAD_SIZE) {
+ error_report("NFS Warning: Truncating NFS readahead "
+ "size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
+ val = QEMU_NFS_MAX_READAHEAD_SIZE;
}
- if (!strcmp(qp->p[i].name, "uid")) {
- nfs_set_uid(client->context, val);
- } else if (!strcmp(qp->p[i].name, "gid")) {
- nfs_set_gid(client->context, val);
- } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
- nfs_set_tcp_syncnt(client->context, val);
-#ifdef LIBNFS_FEATURE_READAHEAD
- } else if (!strcmp(qp->p[i].name, "readahead")) {
- if (open_flags & BDRV_O_NOCACHE) {
- error_setg(errp, "Cannot enable NFS readahead "
- "if cache.direct = on");
- goto fail;
- }
- if (val > QEMU_NFS_MAX_READAHEAD_SIZE) {
- error_report("NFS Warning: Truncating NFS readahead"
- " size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
- val = QEMU_NFS_MAX_READAHEAD_SIZE;
- }
- nfs_set_readahead(client->context, val);
+ nfs_set_readahead(client->context, val);
#ifdef LIBNFS_FEATURE_PAGECACHE
- nfs_set_pagecache_ttl(client->context, 0);
+ nfs_set_pagecache_ttl(client->context, 0);
#endif
- client->cache_used = true;
+ client->cache_used = true;
+ }
#endif
+
#ifdef LIBNFS_FEATURE_PAGECACHE
- nfs_set_pagecache_ttl(client->context, 0);
- } else if (!strcmp(qp->p[i].name, "pagecache")) {
- if (open_flags & BDRV_O_NOCACHE) {
- error_setg(errp, "Cannot enable NFS pagecache "
- "if cache.direct = on");
- goto fail;
- }
- if (val > QEMU_NFS_MAX_PAGECACHE_SIZE) {
- error_report("NFS Warning: Truncating NFS pagecache"
- " size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
- val = QEMU_NFS_MAX_PAGECACHE_SIZE;
- }
- nfs_set_pagecache(client->context, val);
- nfs_set_pagecache_ttl(client->context, 0);
- client->cache_used = true;
+ if (qemu_opt_get(opts, "pagecache")) {
+ if (open_flags & BDRV_O_NOCACHE) {
+ error_setg(errp, "Cannot enable NFS pagecache "
+ "if cache.direct = on");
+ goto fail;
+ }
+ val = qemu_opt_get_number(opts, "pagecache", 0);
+ if (val > QEMU_NFS_MAX_PAGECACHE_SIZE) {
+ error_report("NFS Warning: Truncating NFS pagecache "
+ "size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
+ val = QEMU_NFS_MAX_PAGECACHE_SIZE;
+ }
+ nfs_set_pagecache(client->context, val);
+ nfs_set_pagecache_ttl(client->context, 0);
+ client->cache_used = true;
+ }
#endif
+
#ifdef LIBNFS_FEATURE_DEBUG
- } else if (!strcmp(qp->p[i].name, "debug")) {
- /* limit the maximum debug level to avoid potential flooding
- * of our log files. */
- if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
- error_report("NFS Warning: Limiting NFS debug level"
- " to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
- val = QEMU_NFS_MAX_DEBUG_LEVEL;
- }
- nfs_set_debug(client->context, val);
-#endif
- } else {
- error_setg(errp, "Unknown NFS parameter name: %s",
- qp->p[i].name);
- goto fail;
+ if (qemu_opt_get(opts, "debug")) {
+ val = qemu_opt_get_number(opts, "debug", 0);
+ /* limit the maximum debug level to avoid potential flooding
+ * of our log files. */
+ if (val > QEMU_NFS_MAX_DEBUG_LEVEL) {
+ error_report("NFS Warning: Limiting NFS debug level "
+ "to %d", QEMU_NFS_MAX_DEBUG_LEVEL);
+ val = QEMU_NFS_MAX_DEBUG_LEVEL;
}
+ nfs_set_debug(client->context, val);
}
+#endif
- ret = nfs_mount(client->context, uri->server, uri->path);
+ ret = nfs_mount(client->context, host, path);
if (ret < 0) {
error_setg(errp, "Failed to mount nfs share: %s",
nfs_get_error(client->context));
@@ -417,13 +568,11 @@ static int64_t nfs_client_open(NFSClient *client, const char *filename,
client->st_blocks = st.st_blocks;
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);
+ qemu_opts_del(opts);
g_free(file);
return ret;
}
@@ -432,28 +581,17 @@ 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;
client->aio_context = bdrv_get_aio_context(bs);
- opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
- qemu_opts_absorb_qdict(opts, options, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- ret = -EINVAL;
- goto out;
- }
- ret = nfs_client_open(client, qemu_opt_get(opts, "filename"),
+ ret = nfs_client_open(client, options,
(flags & BDRV_O_RDWR) ? O_RDWR : O_RDONLY,
errp, bs->open_flags);
if (ret < 0) {
- goto out;
+ return ret;
}
bs->total_sectors = ret;
ret = 0;
-out:
- qemu_opts_del(opts);
return ret;
}
@@ -475,6 +613,7 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
int ret = 0;
int64_t total_size = 0;
NFSClient *client = g_new0(NFSClient, 1);
+ QDict *options = NULL;
client->aio_context = qemu_get_aio_context();
@@ -482,7 +621,13 @@ static int nfs_file_create(const char *url, QemuOpts *opts, Error **errp)
total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
BDRV_SECTOR_SIZE);
- ret = nfs_client_open(client, url, O_CREAT, errp, 0);
+ options = qdict_new();
+ ret = nfs_parse_uri(url, options, errp);
+ if (ret < 0) {
+ goto out;
+ }
+
+ ret = nfs_client_open(client, options, O_CREAT, errp, 0);
if (ret < 0) {
goto out;
}
@@ -578,7 +723,7 @@ static BlockDriver bdrv_nfs = {
.protocol_name = "nfs",
.instance_size = sizeof(NFSClient),
- .bdrv_needs_filename = true,
+ .bdrv_parse_filename = nfs_parse_filename,
.create_opts = &nfs_create_opts,
.bdrv_has_zero_init = nfs_has_zero_init,
--
2.6.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS
2016-10-27 10:43 [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 1/2] block/nfs: Introduce runtime_opts in NFS Ashijeet Acharya
@ 2016-10-27 10:43 ` Ashijeet Acharya
2016-10-27 16:45 ` Kevin Wolf
2016-10-27 16:43 ` [Qemu-devel] [PATCH v3 0/2] block: " Kevin Wolf
2 siblings, 1 reply; 5+ messages in thread
From: Ashijeet Acharya @ 2016-10-27 10:43 UTC (permalink / raw)
To: kwolf
Cc: eblake, pl, jcody, mreitz, armbru, qemu-devel, qemu-block,
Ashijeet Acharya
Introduce new object 'BlockdevOptionsNFS' in qapi/block-core.json to
support blockdev-add for NFS network protocol driver. Also make a new
struct NFSServer to support tcp connection.
Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
---
qapi/block-core.json | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 73 insertions(+), 4 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d797b8..75e28aa 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1708,15 +1708,16 @@
#
# @host_device, @host_cdrom: Since 2.1
# @gluster: Since 2.7
+# @nfs: Since 2.8
#
# Since: 2.0
##
{ 'enum': 'BlockdevDriver',
'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
- 'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
- 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
- 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
+ 'host_device', 'http', 'https', 'luks', 'nfs', 'null-aio',
+ 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
+ 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
##
# @BlockdevOptionsFile
@@ -2212,6 +2213,74 @@
'*top-id': 'str' } }
##
+# @NFSTransport
+#
+# An enumeration of NFS transport types
+#
+# @inet: host address for NFS server
+#
+# Since 2.8
+##
+{ 'enum': 'NFSTransport',
+ 'data': [ 'inet' ] }
+
+##
+# @NFSServer
+#
+# Captures the address of the socket
+#
+# @type: transport type used for NFS (only TCP supported)
+#
+# @inet: host address for NFS server
+#
+# Since 2.8
+##
+{ 'struct': 'NFSServer',
+ 'data': { 'type': 'NFSTransport',
+ 'host': 'str' } }
+
+##
+# @BlockdevOptionsNfs
+#
+# Driver specific block device option for NFS
+#
+# @server: host address
+#
+# @path: path of the image on the host
+#
+# @user: #optional UID value to use when talking to the
+# server (defaults to 65534 on Windows and getuid()
+# on unix)
+#
+# @group: #optional GID value to use when talking to the
+# server (defaults to 65534 on Windows and getgid()
+# in unix)
+#
+# @tcp-syn-count: #optional number of SYNs during the session
+# establishment (defaults to libnfs default)
+#
+# @readahead-size: #optional set the readahead size in bytes (defaults
+# to libnfs default)
+#
+# @page-cache-size: #optional set the pagecache size in bytes (defaults
+# to libnfs default)
+#
+# @debug-level: #optional set the NFS debug level (max 2) (defaults
+# to libnfs default)
+#
+# Since 2.8
+##
+{ 'struct': 'BlockdevOptionsNfs',
+ 'data': { 'server': 'NFSServer',
+ 'path': 'str',
+ '*user': 'int',
+ '*group': 'int',
+ '*tcp-syn-count': 'int',
+ '*readahead-size': 'int',
+ '*page-cache-size': 'int',
+ '*debug-level': 'int' } }
+
+##
# @BlockdevOptionsCurl
#
# Driver specific block device options for the curl backend.
@@ -2269,7 +2338,7 @@
# TODO iscsi: Wait for structured options
'luks': 'BlockdevOptionsLUKS',
# TODO nbd: Should take InetSocketAddress for 'host'?
-# TODO nfs: Wait for structured options
+ 'nfs': 'BlockdevOptionsNfs',
'null-aio': 'BlockdevOptionsNull',
'null-co': 'BlockdevOptionsNull',
'parallels': 'BlockdevOptionsGenericFormat',
--
2.6.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS
2016-10-27 10:43 [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 1/2] block/nfs: Introduce runtime_opts in NFS Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS Ashijeet Acharya
@ 2016-10-27 16:43 ` Kevin Wolf
2 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2016-10-27 16:43 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: eblake, pl, jcody, mreitz, armbru, qemu-devel, qemu-block
Am 27.10.2016 um 12:43 hat Ashijeet Acharya geschrieben:
> Previously posted series patches:
> v2: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg05844.html
> v1: https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg04487.html
>
> This series adds blockdev-add support for NFS block driver.
>
> Patch 1 helps to prepare NFS driver to make use of several runtime_opts
> as they appear in the URI. This will make NFS to do things similar to
> the way other drivers available in the block layer do.
>
> Patch 2 helps to allow blockdev-add support for the NFS block driver
> by making the NFS option available.
qemu-iotests 104 used to work with NFS before this series and fails now
(see diff below). Probably you need a .bdrv_refresh_filename()
implementation to go back from options to the original URL. This is a
minor problem, though, and we can fix it in a follow-up.
The more important problem is that you didn't address my comment that
you don't actually process options as they are specified in the schema,
which means that you can't actually use blockdev-add:
{"execute":"blockdev-add","arguments":{"driver":"nfs","node-name":"disk","server":{"type":"inet","host":"localhost"},"path":"/home/kwolf/images/hd.img"}}
{"error": {"class": "GenericError", "desc": "No hostname was specified"}}
On the command line you can just directly use "host" without embedding
it into "server", but that doesn't match the schema and therefore
doesn't work with blockdev-add:
{"execute":"blockdev-add","arguments":{"driver":"nfs","node-name":"disk","host":"localhost","path":"/home/kwolf/images/hd.img"}}
{"error": {"class": "GenericError", "desc": "Parameter 'server' is missing"}}
Kevin
--- /home/kwolf/source/qemu/tests/qemu-iotests/104.out 2016-08-12 17:42:34.307082303 +0200
+++ 104.out.bad 2016-10-27 18:34:58.111108932 +0200
@@ -2,11 +2,11 @@
=== Check qemu-img info output ===
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1024
-image: TEST_DIR/t.IMGFMT
+image: json:{"driver": "IMGFMT", "file": {"host": "127.0.0.1", "driver": "nfs", "path": "//home/kwolf/images/tmp/t.IMGFMT"}}
file format: IMGFMT
virtual size: 1.0K (1024 bytes)
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1234
-image: TEST_DIR/t.IMGFMT
+image: json:{"driver": "IMGFMT", "file": {"host": "127.0.0.1", "driver": "nfs", "path": "//home/kwolf/images/tmp/t.IMGFMT"}}
file format: IMGFMT
virtual size: 1.5K (1536 bytes)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS Ashijeet Acharya
@ 2016-10-27 16:45 ` Kevin Wolf
0 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2016-10-27 16:45 UTC (permalink / raw)
To: Ashijeet Acharya
Cc: eblake, pl, jcody, mreitz, armbru, qemu-devel, qemu-block
Am 27.10.2016 um 12:43 hat Ashijeet Acharya geschrieben:
> Introduce new object 'BlockdevOptionsNFS' in qapi/block-core.json to
> support blockdev-add for NFS network protocol driver. Also make a new
> struct NFSServer to support tcp connection.
>
> Signed-off-by: Ashijeet Acharya <ashijeetacharya@gmail.com>
> ---
> qapi/block-core.json | 77 +++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 9d797b8..75e28aa 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -1708,15 +1708,16 @@
> #
> # @host_device, @host_cdrom: Since 2.1
> # @gluster: Since 2.7
> +# @nfs: Since 2.8
> #
> # Since: 2.0
> ##
> { 'enum': 'BlockdevDriver',
> 'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> - 'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
> - 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> - 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
> + 'host_device', 'http', 'https', 'luks', 'nfs', 'null-aio',
> + 'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> + 'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
>
> ##
> # @BlockdevOptionsFile
> @@ -2212,6 +2213,74 @@
> '*top-id': 'str' } }
>
> ##
> +# @NFSTransport
> +#
> +# An enumeration of NFS transport types
> +#
> +# @inet: host address for NFS server
This description belongs to the 'host' field in NFSServer. Here it
should probably say something like "TCP transport".
> +# Since 2.8
> +##
> +{ 'enum': 'NFSTransport',
> + 'data': [ 'inet' ] }
> +
> +##
> +# @NFSServer
> +#
> +# Captures the address of the socket
> +#
> +# @type: transport type used for NFS (only TCP supported)
> +#
> +# @inet: host address for NFS server
Here the description is right, but the field name is wrong (should be
@host instead of @inet).
> +# Since 2.8
> +##
> +{ 'struct': 'NFSServer',
> + 'data': { 'type': 'NFSTransport',
> + 'host': 'str' } }
> +
Kevin
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-10-27 16:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-27 10:43 [Qemu-devel] [PATCH v3 0/2] block: allow blockdev-add for NFS Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 1/2] block/nfs: Introduce runtime_opts in NFS Ashijeet Acharya
2016-10-27 10:43 ` [Qemu-devel] [PATCH v3 2/2] qapi: allow blockdev-add for NFS Ashijeet Acharya
2016-10-27 16:45 ` Kevin Wolf
2016-10-27 16:43 ` [Qemu-devel] [PATCH v3 0/2] block: " Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).