[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS
From: |
Peter Lieven |
Subject: |
Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS |
Date: |
Thu, 19 Jan 2017 16:47:15 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 |
Am 19.01.2017 um 16:42 schrieb Kevin Wolf:
Am 19.01.2017 um 16:34 hat Peter Lieven geschrieben:
Am 19.01.2017 um 16:20 schrieb Kevin Wolf:
Am 19.01.2017 um 15:59 hat Eric Blake geschrieben:
On 01/19/2017 08:30 AM, Peter Lieven wrote:
qemu-img: Could not open
'nfs://172.21.200.61/vcore-dev-cdrom/templates/VC_debian8-20170116.qcow2,linux?readahead=131072':
Block protocol 'nfs' doesn't support the option 'readahead-size'
Please let me know if the below fix would be correct:
No, this needs to be fixed the other way round: runtime_opts must use
the names as specified in the schema, and nfs_client_open() must access
them as such. Without that, blockdev-add can't work (and the command
line only with the "wrong" old option names from the URL, whereas it
should be using the same names as the QAPI schema).
Shouldn't we support both for backwards compatiblity.?
blockdev-add only needs to support the modern naming. But yes,
preserving back-compat spelling of the command-line spellings, as well
as matching blockdev-add spellings, is desirable.
We only just added the individual command line options, previously it
only supported the URL.
It's true that we have the messed up version of the options in 2.8, so
strictly speaking we would break compatibility with a release, but it's
only one release, it's only the nfs driver, and the documentation of the
options is the schema, which had the right option names even in 2.8
(they just didn't work).
So I wouldn't feel bad about removing the wrong names in this specific
case.
So want exactly do you want to do? Fix the names in the QAPI schema
to use the old naming?
No, fix the command line to use the names in the QAPI schema.
The option names from the URL were never supposed to be supported on the
command line.
Kevin
This is what I wanted to do (before I asked ;-)):
diff --git a/block/nfs.c b/block/nfs.c
index baaecff..7c997cd 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -119,19 +119,24 @@ static int nfs_parse_uri(const char *filename, QDict
*options, Error **errp)
qp->p[i].name);
goto out;
}
- if (!strcmp(qp->p[i].name, "uid")) {
+ if (!strcmp(qp->p[i].name, "uid") ||
+ !strcmp(qp->p[i].name, "user")) {
qdict_put(options, "user",
qstring_from_str(qp->p[i].value));
- } else if (!strcmp(qp->p[i].name, "gid")) {
+ } else if (!strcmp(qp->p[i].name, "gid") ||
+ !strcmp(qp->p[i].name, "group")) {
qdict_put(options, "group",
qstring_from_str(qp->p[i].value));
- } else if (!strcmp(qp->p[i].name, "tcp-syncnt")) {
+ } else if (!strcmp(qp->p[i].name, "tcp-syncnt") ||
+ !strcmp(qp->p[i].name, "tcp-syn-count")) {
qdict_put(options, "tcp-syn-count",
qstring_from_str(qp->p[i].value));
- } else if (!strcmp(qp->p[i].name, "readahead")) {
+ } else if (!strcmp(qp->p[i].name, "readahead") ||
+ !strcmp(qp->p[i].name, "readahead-size")) {
qdict_put(options, "readahead-size",
qstring_from_str(qp->p[i].value));
- } else if (!strcmp(qp->p[i].name, "pagecache")) {
+ } else if (!strcmp(qp->p[i].name, "pagecache") ||
+ !strcmp(qp->p[i].name, "page-cache-size")) {
qdict_put(options, "page-cache-size",
qstring_from_str(qp->p[i].value));
} else if (!strcmp(qp->p[i].name, "debug")) {
@@ -359,27 +364,27 @@ static QemuOptsList runtime_opts = {
.help = "Path of the image on the host",
},
{
- .name = "uid",
+ .name = "user",
.type = QEMU_OPT_NUMBER,
.help = "UID value to use when talking to the server",
},
{
- .name = "gid",
+ .name = "group",
.type = QEMU_OPT_NUMBER,
.help = "GID value to use when talking to the server",
},
{
- .name = "tcp-syncnt",
+ .name = "tcp-syn-count",
.type = QEMU_OPT_NUMBER,
.help = "Number of SYNs to send during the session establish",
},
{
- .name = "readahead",
+ .name = "readahead-size",
.type = QEMU_OPT_NUMBER,
.help = "Set the readahead size in bytes",
},
{
- .name = "pagecache",
+ .name = "page-cache-size",
.type = QEMU_OPT_NUMBER,
.help = "Set the pagecache size in bytes",
},
@@ -508,29 +513,29 @@ static int64_t nfs_client_open(NFSClient *client, QDict
*options,
goto fail;
}
- if (qemu_opt_get(opts, "uid")) {
- client->uid = qemu_opt_get_number(opts, "uid", 0);
+ if (qemu_opt_get(opts, "user")) {
+ client->uid = qemu_opt_get_number(opts, "user", 0);
nfs_set_uid(client->context, client->uid);
}
- if (qemu_opt_get(opts, "gid")) {
- client->gid = qemu_opt_get_number(opts, "gid", 0);
+ if (qemu_opt_get(opts, "group")) {
+ client->gid = qemu_opt_get_number(opts, "group", 0);
nfs_set_gid(client->context, client->gid);
}
- if (qemu_opt_get(opts, "tcp-syncnt")) {
- client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syncnt", 0);
+ if (qemu_opt_get(opts, "tcp-syn-count")) {
+ client->tcp_syncnt = qemu_opt_get_number(opts, "tcp-syn-count", 0);
nfs_set_tcp_syncnt(client->context, client->tcp_syncnt);
}
#ifdef LIBNFS_FEATURE_READAHEAD
- if (qemu_opt_get(opts, "readahead")) {
+ if (qemu_opt_get(opts, "readahead-size")) {
if (open_flags & BDRV_O_NOCACHE) {
error_setg(errp, "Cannot enable NFS readahead "
"if cache.direct = on");
goto fail;
}
- client->readahead = qemu_opt_get_number(opts, "readahead", 0);
+ client->readahead = qemu_opt_get_number(opts, "readahead-size", 0);
if (client->readahead > QEMU_NFS_MAX_READAHEAD_SIZE) {
error_report("NFS Warning: Truncating NFS readahead "
"size to %d", QEMU_NFS_MAX_READAHEAD_SIZE);
@@ -545,13 +550,13 @@ static int64_t nfs_client_open(NFSClient *client, QDict
*options,
#endif
#ifdef LIBNFS_FEATURE_PAGECACHE
- if (qemu_opt_get(opts, "pagecache")) {
+ if (qemu_opt_get(opts, "page-cache-size")) {
if (open_flags & BDRV_O_NOCACHE) {
error_setg(errp, "Cannot enable NFS pagecache "
"if cache.direct = on");
goto fail;
}
- client->pagecache = qemu_opt_get_number(opts, "pagecache", 0);
+ client->pagecache = qemu_opt_get_number(opts, "page-cache-size", 0);
if (client->pagecache > QEMU_NFS_MAX_PAGECACHE_SIZE) {
error_report("NFS Warning: Truncating NFS pagecache "
"size to %d pages", QEMU_NFS_MAX_PAGECACHE_SIZE);
Peter
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, (continued)
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Peter Lieven, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Eric Blake, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Kevin Wolf, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Peter Lieven, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Kevin Wolf, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Peter Lieven, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Kevin Wolf, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Peter Lieven, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Kevin Wolf, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS, Peter Lieven, 2017/01/19
- Re: [Qemu-block] [PATCH v6 0/2] allow blockdev-add for NFS,
Peter Lieven <=