[V2,2/2] block/nfs: fix naming of runtime opts

Commit Message

Peter Lieven Jan. 31, 2017, 3:56 p.m. UTC

commit 94d6a7a accidently left the naming of runtime opts and QAPI
scheme inconsistent. As one consequence passing of parameters in the
URI is broken. Sync the naming of the runtime opts to the QAPI
scheme.

Please note that this is technically backwards incompatible with the 2.8
release, but the 2.8 release is the only version that had the wrong naming.
Furthermore release 2.8 suffered from a NULL pointer deference during
URI parsing.

Fixes: 94d6a7a76e9df9919629428f6c598e2b97d9426c
Cc: [e-mail]
Signed-off-by: Peter Lieven <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
---
 block/nfs.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

Comments

Max Reitz Feb. 1, 2017, 1:06 a.m. UTC | #1

On 31.01.2017 16:56, Peter Lieven wrote:
> commit 94d6a7a accidently left the naming of runtime opts and QAPI
> scheme inconsistent. As one consequence passing of parameters in the
> URI is broken. Sync the naming of the runtime opts to the QAPI
> scheme.
> 
> Please note that this is technically backwards incompatible with the 2.8
> release, but the 2.8 release is the only version that had the wrong naming.
> Furthermore release 2.8 suffered from a NULL pointer deference during
> URI parsing.

I always love things like this. "This technically breaks X, but
obviously nobody used X, because X always crashed." Reminds me of how we
success^W accidentally removed qcow2 encryption.

Technically however this also changes the runtime parameter names, I
think. Giving them probably did work in 2.8, so it is not compatible
there. I don't mind very much, though, and you're the maintainer, so
it's fine with me.

Anyway, I think this patch should also update nfs_refresh_filename().

Max

Peter Lieven Feb. 1, 2017, 9:22 a.m. UTC | #2

Am 01.02.2017 um 02:06 schrieb Max Reitz:
> On 31.01.2017 16:56, Peter Lieven wrote:
>> commit 94d6a7a accidently left the naming of runtime opts and QAPI
>> scheme inconsistent. As one consequence passing of parameters in the
>> URI is broken. Sync the naming of the runtime opts to the QAPI
>> scheme.
>>
>> Please note that this is technically backwards incompatible with the 2.8
>> release, but the 2.8 release is the only version that had the wrong naming.
>> Furthermore release 2.8 suffered from a NULL pointer deference during
>> URI parsing.
> I always love things like this. "This technically breaks X, but
> obviously nobody used X, because X always crashed." Reminds me of how we
> success^W accidentally removed qcow2 encryption.
>
> Technically however this also changes the runtime parameter names, I
> think. Giving them probably did work in 2.8, so it is not compatible
> there. I don't mind very much, though, and you're the maintainer, so
> it's fine with me.
>
> Anyway, I think this patch should also update nfs_refresh_filename().

Thanks, that indeed is missing. Will send a V3.

Peter

Patch

diff --git a/block/nfs.c b/block/nfs.c
index baaecff..464d547 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -359,27 +359,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 +508,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 +545,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);