[v6,0/2] allow blockdev-add for NFS

Commit Message

Peter Lieven Jan. 19, 2017, 3:47 p.m. UTC

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 ;-)):



Peter

Patch

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);