All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices
@ 2017-01-24  9:24 Pradeep Jagadeesh
  2017-01-24  9:24 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
  2017-02-01 14:44 ` [Qemu-devel] [PATCH 0/2 " Alberto Garcia
  0 siblings, 2 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-24  9:24 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Greg Kurz
  Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel, Eric Blake

This patch set adds the IO throttling functionality to fsdev/9p devices.
So far cgroups were used for throttling IO opertions on the fsdev/9p devices.
It is difficult to use cgroups for throttling because we have to set up 
cgroups externally before we start the qemu process.
Qemu provides the throttling apis for implementing the throttling. 
Block devices already make use of these APIs for throtting the IO operations.
So, we use the same APIs to enable the throttling functionality for
fsdevices.As of now the feature is enabled only on 9p-local driver.

This feature can be used as shown in the below example:

-fsdev local,id=sdb1,path=PATH_TO_DEVICE,security_model=none,writeout=immediate,
 throttling.bps-read=4194304,throttling.bps-write=4194304  -device
 virtio-9p-pci,fsdev=sdb1,mount_tag=sdb1

The main advantages are:
 
- Easy to use because the throttling options are part of qemu cli options
- Provides a uniform way of using throttling options across block and
    fsdev/9p devices
- No need to setup cgroup to provide throttling functionality for the fsdev
  devices.
- Removes the redundant throttling code that was present in block and fsdev
  files

Missing features:

-QMP support
-Throttling support for other fsdev/9p drivers.

Thanks,
Pradeep

Pradeep Jagadeesh (2):
  fsdev: add IO throttle support to fsdev devices
  throttle: removed duplicate throtlle code from block and fsdev files

 blockdev.c                      |  81 ++-------------------------
 fsdev/Makefile.objs             |   2 +-
 fsdev/file-op-9p.h              |   3 +
 fsdev/qemu-fsdev-opts.c         |   3 +
 fsdev/qemu-fsdev-throttle.c     | 118 ++++++++++++++++++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.h     |  39 +++++++++++++
 hw/9pfs/9p-local.c              |   8 +++
 hw/9pfs/9p.c                    |   5 ++
 hw/9pfs/cofile.c                |   2 +
 include/qemu/throttle-options.h |  92 +++++++++++++++++++++++++++++++
 10 files changed, 275 insertions(+), 78 deletions(-)
 create mode 100644 fsdev/qemu-fsdev-throttle.c
 create mode 100644 fsdev/qemu-fsdev-throttle.h
 create mode 100644 include/qemu/throttle-options.h

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-01-24  9:24 [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
@ 2017-01-24  9:24 ` Pradeep Jagadeesh
  2017-02-01 15:08   ` Alberto Garcia
  2017-02-01 14:44 ` [Qemu-devel] [PATCH 0/2 " Alberto Garcia
  1 sibling, 1 reply; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-01-24  9:24 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Greg Kurz; +Cc: Pradeep Jagadeesh, Alberto Garcia, qemu-devel

This patchset adds the io throttle support for the 9p-local driver.
For now this functionality can be used only through qemu cli options.
QMP interface and support to other 9p drivers need further extensions.
To make it simple for other 9p drivers, the throttle code has been put in
separate files.

Signed-off-by: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
---
 fsdev/Makefile.objs         |   2 +-
 fsdev/file-op-9p.h          |   3 ++
 fsdev/qemu-fsdev-opts.c     |  77 ++++++++++++++++++++++++++++-
 fsdev/qemu-fsdev-throttle.c | 118 ++++++++++++++++++++++++++++++++++++++++++++
 fsdev/qemu-fsdev-throttle.h |  39 +++++++++++++++
 hw/9pfs/9p-local.c          |   8 +++
 hw/9pfs/9p.c                |   5 ++
 hw/9pfs/cofile.c            |   2 +
 8 files changed, 252 insertions(+), 2 deletions(-)
 create mode 100644 fsdev/qemu-fsdev-throttle.c
 create mode 100644 fsdev/qemu-fsdev-throttle.h

diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 1b120a4..659df6e 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -5,7 +5,7 @@ common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
 else
 common-obj-y = qemu-fsdev-dummy.o
 endif
-common-obj-y += qemu-fsdev-opts.o
+common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
 
 # Toplevel always builds this; targets without virtio will put it in
 # common-obj-y
diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index a56dc84..0844a40 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -17,6 +17,7 @@
 #include <dirent.h>
 #include <utime.h>
 #include <sys/vfs.h>
+#include "qemu-fsdev-throttle.h"
 
 #define SM_LOCAL_MODE_BITS    0600
 #define SM_LOCAL_DIR_MODE_BITS    0700
@@ -74,6 +75,7 @@ typedef struct FsDriverEntry {
     char *path;
     int export_flags;
     FileOperations *ops;
+    FsThrottle fst;
 } FsDriverEntry;
 
 typedef struct FsContext
@@ -83,6 +85,7 @@ typedef struct FsContext
     int export_flags;
     struct xattr_operations **xops;
     struct extended_ops exops;
+    FsThrottle *fst;
     /* fs driver specific data */
     void *private;
 } FsContext;
diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
index 1dd8c7a..385423f0 100644
--- a/fsdev/qemu-fsdev-opts.c
+++ b/fsdev/qemu-fsdev-opts.c
@@ -37,8 +37,83 @@ static QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "sock_fd",
             .type = QEMU_OPT_NUMBER,
+        }, {
+            .name = "throttling.iops-total",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit total I/O operations per second",
+        }, {
+            .name = "throttling.iops-read",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit read operations per second",
+        }, {
+            .name = "throttling.iops-write",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit write operations per second",
+        }, {
+            .name = "throttling.bps-total",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit total bytes per second",
+        }, {
+            .name = "throttling.bps-read",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit read bytes per second",
+        }, {
+            .name = "throttling.bps-write",
+            .type = QEMU_OPT_NUMBER,
+            .help = "limit write bytes per second",
+        }, {
+            .name = "throttling.iops-total-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "I/O operations burst",
+        }, {
+            .name = "throttling.iops-read-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "I/O operations read burst",
+        }, {
+            .name = "throttling.iops-write-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "I/O operations write burst",
+        }, {
+            .name = "throttling.bps-total-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes burst",
+        }, {
+            .name = "throttling.bps-read-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes read burst",
+        }, {
+            .name = "throttling.bps-write-max",
+            .type = QEMU_OPT_NUMBER,
+            .help = "total bytes write burst",
+        }, {
+            .name = "throttling.iops-total-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-total-max burst period, in seconds",
+        }, {
+            .name = "throttling.iops-read-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-read-max burst period, in seconds",
+        }, {
+            .name = "throttling.iops-write-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the iops-write-max burst period, in seconds",
+        }, {
+            .name = "throttling.bps-total-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-total-max burst period, in seconds",
+        }, {
+            .name = "throttling.bps-read-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-read-max burst period, in seconds",
+        }, {
+            .name = "throttling.bps-write-max-length",
+            .type = QEMU_OPT_NUMBER,
+            .help = "length of the bps-write-max burst period, in seconds",
+        }, {
+            .name = "throttling.iops-size",
+            .type = QEMU_OPT_NUMBER,
+            .help = "when limiting by iops max size of an I/O in bytes",
         },
-
         { /*End of list */ }
     },
 };
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
new file mode 100644
index 0000000..feb9af3
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -0,0 +1,118 @@
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ *
+ * See the COPYING file in the top-level directory for details.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "qemu-fsdev-throttle.h"
+#include "qemu/iov.h"
+
+static void fsdev_throttle_read_timer_cb(void *opaque)
+{
+    FsThrottle *fst = opaque;
+    qemu_co_enter_next(&fst->throttled_reqs[false]);
+}
+
+static void fsdev_throttle_write_timer_cb(void *opaque)
+{
+    FsThrottle *fst = opaque;
+    qemu_co_enter_next(&fst->throttled_reqs[true]);
+}
+
+void fsdev_throttle_parse_opts(QemuOpts *opts, FsThrottle *fst, Error **errp)
+{
+    throttle_config_init(&fst->cfg);
+    fst->cfg.buckets[THROTTLE_BPS_TOTAL].avg =
+        qemu_opt_get_number(opts, "throttling.bps-total", 0);
+    fst->cfg.buckets[THROTTLE_BPS_READ].avg  =
+        qemu_opt_get_number(opts, "throttling.bps-read", 0);
+    fst->cfg.buckets[THROTTLE_BPS_WRITE].avg =
+        qemu_opt_get_number(opts, "throttling.bps-write", 0);
+    fst->cfg.buckets[THROTTLE_OPS_TOTAL].avg =
+        qemu_opt_get_number(opts, "throttling.iops-total", 0);
+    fst->cfg.buckets[THROTTLE_OPS_READ].avg =
+        qemu_opt_get_number(opts, "throttling.iops-read", 0);
+    fst->cfg.buckets[THROTTLE_OPS_WRITE].avg =
+        qemu_opt_get_number(opts, "throttling.iops-write", 0);
+
+    fst->cfg.buckets[THROTTLE_BPS_TOTAL].max =
+        qemu_opt_get_number(opts, "throttling.bps-total-max", 0);
+    fst->cfg.buckets[THROTTLE_BPS_READ].max  =
+        qemu_opt_get_number(opts, "throttling.bps-read-max", 0);
+    fst->cfg.buckets[THROTTLE_BPS_WRITE].max =
+        qemu_opt_get_number(opts, "throttling.bps-write-max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_TOTAL].max =
+        qemu_opt_get_number(opts, "throttling.iops-total-max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_READ].max =
+        qemu_opt_get_number(opts, "throttling.iops-read-max", 0);
+    fst->cfg.buckets[THROTTLE_OPS_WRITE].max =
+        qemu_opt_get_number(opts, "throttling.iops-write-max", 0);
+
+    fst->cfg.buckets[THROTTLE_BPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, "throttling.bps-total-max-length", 1);
+    fst->cfg.buckets[THROTTLE_BPS_READ].burst_length  =
+        qemu_opt_get_number(opts, "throttling.bps-read-max-length", 1);
+    fst->cfg.buckets[THROTTLE_BPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, "throttling.bps-write-max-length", 1);
+    fst->cfg.buckets[THROTTLE_OPS_TOTAL].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-total-max-length", 1);
+    fst->cfg.buckets[THROTTLE_OPS_READ].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-read-max-length", 1);
+    fst->cfg.buckets[THROTTLE_OPS_WRITE].burst_length =
+        qemu_opt_get_number(opts, "throttling.iops-write-max-length", 1);
+    fst->cfg.op_size =
+        qemu_opt_get_number(opts, "throttling.iops-size", 0);
+
+    throttle_is_valid(&fst->cfg, errp);
+}
+
+void fsdev_throttle_init(FsThrottle *fst)
+{
+    if (throttle_enabled(&fst->cfg)) {
+        throttle_init(&fst->ts);
+        throttle_timers_init(&fst->tt,
+                             qemu_get_aio_context(),
+                             QEMU_CLOCK_REALTIME,
+                             fsdev_throttle_read_timer_cb,
+                             fsdev_throttle_write_timer_cb,
+                             fst);
+        throttle_config(&fst->ts, &fst->tt, &fst->cfg);
+        qemu_co_queue_init(&fst->throttled_reqs[0]);
+        qemu_co_queue_init(&fst->throttled_reqs[1]);
+    }
+}
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *fst, bool is_write,
+                                            struct iovec *iov, int iovcnt)
+{
+    if (throttle_enabled(&fst->cfg)) {
+        if (throttle_schedule_timer(&fst->ts, &fst->tt, is_write) ||
+            !qemu_co_queue_empty(&fst->throttled_reqs[is_write])) {
+            qemu_co_queue_wait(&fst->throttled_reqs[is_write]);
+        }
+
+        throttle_account(&fst->ts, is_write, iov_size(iov, iovcnt));
+
+        if (!qemu_co_queue_empty(&fst->throttled_reqs[is_write]) &&
+            !throttle_schedule_timer(&fst->ts, &fst->tt, is_write)) {
+            qemu_co_queue_next(&fst->throttled_reqs[is_write]);
+        }
+    }
+}
+
+void fsdev_throttle_cleanup(FsThrottle *fst)
+{
+    if (throttle_enabled(&fst->cfg)) {
+        throttle_timers_destroy(&fst->tt);
+    }
+}
diff --git a/fsdev/qemu-fsdev-throttle.h b/fsdev/qemu-fsdev-throttle.h
new file mode 100644
index 0000000..e418643
--- /dev/null
+++ b/fsdev/qemu-fsdev-throttle.h
@@ -0,0 +1,39 @@
+/*
+ * Fsdev Throttle
+ *
+ * Copyright (C) 2016 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ *
+ * See the COPYING file in the top-level directory for details.
+ *
+ */
+
+#ifndef _FSDEV_THROTTLE_H
+#define _FSDEV_THROTTLE_H
+
+#include "block/aio.h"
+#include "qemu/main-loop.h"
+#include "qemu/coroutine.h"
+#include "qapi/error.h"
+#include "qemu/throttle.h"
+
+typedef struct FsThrottle {
+    ThrottleState ts;
+    ThrottleTimers tt;
+    ThrottleConfig cfg;
+    CoQueue      throttled_reqs[2];
+} FsThrottle;
+
+void fsdev_throttle_parse_opts(QemuOpts *, FsThrottle *, Error **);
+
+void fsdev_throttle_init(FsThrottle *);
+
+void coroutine_fn fsdev_co_throttle_request(FsThrottle *, bool ,
+                                            struct iovec *, int);
+
+void fsdev_throttle_cleanup(FsThrottle *);
+#endif /* _FSDEV_THROTTLE_H */
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 845675e..828348d 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1209,6 +1209,7 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
 {
     const char *sec_model = qemu_opt_get(opts, "security_model");
     const char *path = qemu_opt_get(opts, "path");
+    Error *err = NULL;
 
     if (!sec_model) {
         error_report("Security model not specified, local fs needs security model");
@@ -1237,6 +1238,13 @@ static int local_parse_opts(QemuOpts *opts, struct FsDriverEntry *fse)
         error_report("fsdev: No path specified");
         return -1;
     }
+
+    fsdev_throttle_parse_opts(opts, &fse->fst, &err);
+    if (err) {
+        error_reportf_err(err, "Throttle configuration is not valid: ");
+        return -1;
+    }
+
     fse->path = g_strdup(path);
 
     return 0;
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index fa58877..920eb05 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3520,6 +3520,10 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
         error_setg(errp, "share path %s is not a directory", fse->path);
         goto out;
     }
+
+    s->ctx.fst = &fse->fst;
+    fsdev_throttle_init(s->ctx.fst);
+
     v9fs_path_free(&path);
 
     rc = 0;
@@ -3528,6 +3532,7 @@ out:
         if (s->ops && s->ops->cleanup && s->ctx.private) {
             s->ops->cleanup(&s->ctx);
         }
+        fsdev_throttle_cleanup(s->ctx.fst);
         g_free(s->tag);
         g_free(s->ctx.fs_root);
         v9fs_path_free(&path);
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 120e267..88791bc 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -247,6 +247,7 @@ int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
+    fsdev_co_throttle_request(s->ctx.fst, true, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->pwritev(&s->ctx, &fidp->fs, iov, iovcnt, offset);
@@ -266,6 +267,7 @@ int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
     if (v9fs_request_cancelled(pdu)) {
         return -EINTR;
     }
+    fsdev_co_throttle_request(s->ctx.fst, false, iov, iovcnt);
     v9fs_co_run_in_worker(
         {
             err = s->ops->preadv(&s->ctx, &fidp->fs, iov, iovcnt, offset);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-01-24  9:24 [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
  2017-01-24  9:24 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-01 14:44 ` Alberto Garcia
  2017-02-03  9:41   ` Pradeep Jagadeesh
  1 sibling, 1 reply; 8+ messages in thread
From: Alberto Garcia @ 2017-02-01 14:44 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
  Cc: Pradeep Jagadeesh, qemu-devel, Eric Blake

On Tue 24 Jan 2017 10:24:06 AM CET, Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> Pradeep Jagadeesh (2):
>   fsdev: add IO throttle support to fsdev devices
>   throttle: removed duplicate throtlle code from block and fsdev files

I cannot find the second patch in this thread (v14), did you forget to
send it?

https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04913.html

Berto

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-01-24  9:24 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
@ 2017-02-01 15:08   ` Alberto Garcia
  2017-02-03  9:43     ` Pradeep Jagadeesh
  0 siblings, 1 reply; 8+ messages in thread
From: Alberto Garcia @ 2017-02-01 15:08 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
  Cc: Pradeep Jagadeesh, qemu-devel

Hello, and sorry for being late reviewing these patches :)

On Tue 24 Jan 2017 10:24:07 AM CET, Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:

> This patchset adds the io throttle support for the 9p-local driver.
> For now this functionality can be used only through qemu cli options.
> QMP interface and support to other 9p drivers need further extensions.
> To make it simple for other 9p drivers, the throttle code has been put
> in separate files.

Compared to the previous patch that I reviewed, you removed the
fsdev_throttle_cleanup() call from v9fs_device_unrealize_common(). Why
did you do that?

I miss the list of changes in the cover letter.

> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -3520,6 +3520,10 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
>          error_setg(errp, "share path %s is not a directory", fse->path);
>          goto out;
>      }
> +
> +    s->ctx.fst = &fse->fst;
> +    fsdev_throttle_init(s->ctx.fst);
> +
>      v9fs_path_free(&path);
>  
>      rc = 0;
> @@ -3528,6 +3532,7 @@ out:
>          if (s->ops && s->ops->cleanup && s->ctx.private) {
>              s->ops->cleanup(&s->ctx);
>          }
> +        fsdev_throttle_cleanup(s->ctx.fst);
>          g_free(s->tag);
>          g_free(s->ctx.fs_root);
>          v9fs_path_free(&path);

You also added the fsdev_throttle_cleanup() here, but look at the code:

       fsdev_throttle_init(s->ctx.fst);

       v9fs_path_free(&path);

       rc = 0;
   out:
       if (rc) {
           /* ... */
           fsdev_throttle_cleanup(s->ctx.fst);
           /* ... */
       }

If rc != 0 you don't need to do the clean up, because there was never an
initialization.

Berto

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-02-01 14:44 ` [Qemu-devel] [PATCH 0/2 " Alberto Garcia
@ 2017-02-03  9:41   ` Pradeep Jagadeesh
  2017-02-03  9:47     ` Greg Kurz
  2017-02-03  9:48     ` Alberto Garcia
  0 siblings, 2 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03  9:41 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
  Cc: qemu-devel, Eric Blake

On 2/1/2017 3:44 PM, Alberto Garcia wrote:
> On Tue 24 Jan 2017 10:24:06 AM CET, Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>
>> Pradeep Jagadeesh (2):
>>   fsdev: add IO throttle support to fsdev devices
>>   throttle: removed duplicate throtlle code from block and fsdev files
>
> I cannot find the second patch in this thread (v14), did you forget to
> send it?
>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04913.html

Sorry for the late reply.

I did send all the both patch files and the cover letter.
Don't know how the patch missed in that link.

I have received independent e-mails for each patch, with version 14.
Am I missing something here?

Regards,
Pradeep


>
> Berto
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-02-01 15:08   ` Alberto Garcia
@ 2017-02-03  9:43     ` Pradeep Jagadeesh
  0 siblings, 0 replies; 8+ messages in thread
From: Pradeep Jagadeesh @ 2017-02-03  9:43 UTC (permalink / raw)
  To: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz; +Cc: qemu-devel

On 2/1/2017 4:08 PM, Alberto Garcia wrote:
> Hello, and sorry for being late reviewing these patches :)
>
> On Tue 24 Jan 2017 10:24:07 AM CET, Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
>
>> This patchset adds the io throttle support for the 9p-local driver.
>> For now this functionality can be used only through qemu cli options.
>> QMP interface and support to other 9p drivers need further extensions.
>> To make it simple for other 9p drivers, the throttle code has been put
>> in separate files.
>
> Compared to the previous patch that I reviewed, you removed the
> fsdev_throttle_cleanup() call from v9fs_device_unrealize_common(). Why
> did you do that?
>
I actually missed while merging the code to newer version. Instead of 
putting inside the unrealize it went some other function.

> I miss the list of changes in the cover letter

>> --- a/hw/9pfs/9p.c
>> +++ b/hw/9pfs/9p.c
>> @@ -3520,6 +3520,10 @@ int v9fs_device_realize_common(V9fsState *s, Error **errp)
>>          error_setg(errp, "share path %s is not a directory", fse->path);
>>          goto out;
>>      }
>> +
>> +    s->ctx.fst = &fse->fst;
>> +    fsdev_throttle_init(s->ctx.fst);
>> +
>>      v9fs_path_free(&path);
>>
>>      rc = 0;
>> @@ -3528,6 +3532,7 @@ out:
>>          if (s->ops && s->ops->cleanup && s->ctx.private) {
>>              s->ops->cleanup(&s->ctx);
>>          }
>> +        fsdev_throttle_cleanup(s->ctx.fst);
>>          g_free(s->tag);
>>          g_free(s->ctx.fs_root);
>>          v9fs_path_free(&path);
>
> You also added the fsdev_throttle_cleanup() here, but look at the code:
>
>        fsdev_throttle_init(s->ctx.fst);
>
>        v9fs_path_free(&path);
>
>        rc = 0;
>    out:
>        if (rc) {
>            /* ... */
>            fsdev_throttle_cleanup(s->ctx.fst);
>            /* ... */
>        }
>
> If rc != 0 you don't need to do the clean up, because there was never an
> initialization.
Fixed it, it was because of merging issue as I answered above.

-Pradeep
>
> Berto
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-02-03  9:41   ` Pradeep Jagadeesh
@ 2017-02-03  9:47     ` Greg Kurz
  2017-02-03  9:48     ` Alberto Garcia
  1 sibling, 0 replies; 8+ messages in thread
From: Greg Kurz @ 2017-02-03  9:47 UTC (permalink / raw)
  To: Pradeep Jagadeesh
  Cc: Alberto Garcia, Pradeep Jagadeesh, Aneesh Kumar K.V, qemu-devel,
	Eric Blake

On Fri, 3 Feb 2017 10:41:33 +0100
Pradeep Jagadeesh <pradeep.jagadeesh@huawei.com> wrote:

> On 2/1/2017 3:44 PM, Alberto Garcia wrote:
> > On Tue 24 Jan 2017 10:24:06 AM CET, Pradeep Jagadeesh <pradeepkiruvale@gmail.com> wrote:
> >  
> >> Pradeep Jagadeesh (2):
> >>   fsdev: add IO throttle support to fsdev devices
> >>   throttle: removed duplicate throtlle code from block and fsdev files  
> >
> > I cannot find the second patch in this thread (v14), did you forget to
> > send it?
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04913.html  
> 
> Sorry for the late reply.
> 
> I did send all the both patch files and the cover letter.
> Don't know how the patch missed in that link.
> 
> I have received independent e-mails for each patch, with version 14.
> Am I missing something here?
> 

Hi,

Patch 2/2 still doesn't appear in the archives:

https://lists.gnu.org/archive/html/qemu-devel/2017-01/threads.html#04913

And I haven't received it either...

Cheers.

--
Greg

> Regards,
> Pradeep
> 
> 
> >
> > Berto
> >  
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices
  2017-02-03  9:41   ` Pradeep Jagadeesh
  2017-02-03  9:47     ` Greg Kurz
@ 2017-02-03  9:48     ` Alberto Garcia
  1 sibling, 0 replies; 8+ messages in thread
From: Alberto Garcia @ 2017-02-03  9:48 UTC (permalink / raw)
  To: Pradeep Jagadeesh, Pradeep Jagadeesh, Aneesh Kumar K.V, Greg Kurz
  Cc: qemu-devel, Eric Blake

On Fri 03 Feb 2017 10:41:33 AM CET, Pradeep Jagadeesh wrote:

>> I cannot find the second patch in this thread (v14), did you forget
>> to send it?
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg04913.html
>
> I did send all the both patch files and the cover letter.
> Don't know how the patch missed in that link.

I haven't received anything, but since patch 1 needs to be corrected you
can send both again in v15.

Regards,

Berto

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-02-03  9:49 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-24  9:24 [Qemu-devel] [PATCH 0/2 v14] fsdev: add IO throttle support to fsdev devices Pradeep Jagadeesh
2017-01-24  9:24 ` [Qemu-devel] [PATCH 1/2 " Pradeep Jagadeesh
2017-02-01 15:08   ` Alberto Garcia
2017-02-03  9:43     ` Pradeep Jagadeesh
2017-02-01 14:44 ` [Qemu-devel] [PATCH 0/2 " Alberto Garcia
2017-02-03  9:41   ` Pradeep Jagadeesh
2017-02-03  9:47     ` Greg Kurz
2017-02-03  9:48     ` Alberto Garcia

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.