Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-08-07 Thread Markus Armbruster
Pradeep Jagadeesh  writes:

> On 7/7/2017 8:14 AM, Markus Armbruster wrote:
>> Pradeep Jagadeesh  writes:
>>
>>> These patches provide the qmp interface, to query the io throttle
>>> status of the all fsdev devices that are present in a vm.
>>> also, it provides an interface to set the io throttle parameters of a
>>> fsdev to a required value. some of the patches also remove the duplicate
>>> code that was present in block and fsdev files.
>>>
>>> Pradeep Jagadeesh (6):
>>>   throttle: factor out duplicate code
>>>   qmp: Create IOThrottle structure
>>>   throttle: move out function to reuse the code
>>>   hmp: create a throttle initialization function for code reusability
>>>   fsdev: hmp interface for throttling
>>>   fsdev: QMP interface for throttling
>>>
>>>  Makefile|   4 ++
>>>  blockdev.c  |  97 ++---
>>>  fsdev/qemu-fsdev-dummy.c|  10 
>>>  fsdev/qemu-fsdev-throttle.c | 118 
>>> ++--
>>>  fsdev/qemu-fsdev-throttle.h |  13 +
>>>  fsdev/qemu-fsdev.c  |  37 +
>>>  hmp-commands-info.hx|  18 ++
>>>  hmp-commands.hx |  19 +++
>>>  hmp.c   |  81 +--
>>>  hmp.h   |   4 ++
>>>  include/qemu/throttle-options.h |   7 +++
>>>  include/qemu/throttle.h |   4 +-
>>>  include/qemu/typedefs.h |   1 +
>>>  monitor.c   |   5 ++
>>>  qapi-schema.json|   3 +
>>>  qapi/block-core.json|  76 +-
>>>  qapi/fsdev.json |  84 
>>>  qapi/iothrottle.json|  88 ++
>>>  qmp.c   |  14 +
>>>  util/throttle.c | 110 +
>>>  20 files changed, 577 insertions(+), 216 deletions(-)
>>>  create mode 100644 qapi/fsdev.json
>>>  create mode 100644 qapi/iothrottle.json
>>
>> No test coverage?
> I wanted to upstream these first then I am planning to write the tests.

Feels backwards to me :)



Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-08-07 Thread Pradeep Jagadeesh

On 7/7/2017 8:14 AM, Markus Armbruster wrote:

Pradeep Jagadeesh  writes:


These patches provide the qmp interface, to query the io throttle
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. some of the patches also remove the duplicate
code that was present in block and fsdev files.

Pradeep Jagadeesh (6):
  throttle: factor out duplicate code
  qmp: Create IOThrottle structure
  throttle: move out function to reuse the code
  hmp: create a throttle initialization function for code reusability
  fsdev: hmp interface for throttling
  fsdev: QMP interface for throttling

 Makefile|   4 ++
 blockdev.c  |  97 ++---
 fsdev/qemu-fsdev-dummy.c|  10 
 fsdev/qemu-fsdev-throttle.c | 118 ++--
 fsdev/qemu-fsdev-throttle.h |  13 +
 fsdev/qemu-fsdev.c  |  37 +
 hmp-commands-info.hx|  18 ++
 hmp-commands.hx |  19 +++
 hmp.c   |  81 +--
 hmp.h   |   4 ++
 include/qemu/throttle-options.h |   7 +++
 include/qemu/throttle.h |   4 +-
 include/qemu/typedefs.h |   1 +
 monitor.c   |   5 ++
 qapi-schema.json|   3 +
 qapi/block-core.json|  76 +-
 qapi/fsdev.json |  84 
 qapi/iothrottle.json|  88 ++
 qmp.c   |  14 +
 util/throttle.c | 110 +
 20 files changed, 577 insertions(+), 216 deletions(-)
 create mode 100644 qapi/fsdev.json
 create mode 100644 qapi/iothrottle.json


No test coverage?

I wanted to upstream these first then I am planning to write the tests.

-Pradeep








Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-14 Thread Pradeep Jagadeesh

On 7/14/2017 4:26 PM, Manos Pitsidianakis wrote:

On Fri, Jul 14, 2017 at 03:15:06PM +0200, Pradeep Jagadeesh wrote:

Hi Manos,

Thanks for sharing the link to your code patch.

On 7/14/2017 2:22 PM, Manos Pitsidianakis wrote:

Hello Pradeep, you might be interested in my work on refactoring the
block layer's throttling interface in my series:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04191.html

Sure will have a look.

In this series you copy the existing legacy interface we want to get rid
of. I think it will be easier work for you to use the changes introduced
in my patches when they are merged.

So, should I wait till they are in?. Actually my throttle patches for
fsdev are already upstream (2.9). Now I am just introducing the
qmp/hmp interfaces for the same.


If you plan on using ThrottleGroups, probably yes, I think, because we'd
have duplicate interfaces afterwards instead of a unified one. You'd
have to introduce a qmp/hmp command to set fsdev throttle now and after
you use throttle groups it will become obsolete.

I am not using ThrottleGroups in near future.

-Pradeep




Have you thought about using throttle groups? It'd mean many devices
sharing the same limits. This way the interfaces can be unified.  Please
read docs/throttle.txt and see if it would be useful for you.


I have read about the throttle group, last year when I was
implementing the throttle feature for fsdev.My open source work
depends on the projects I/my group work on. So, when I have more
bandwidth work on those, I will surely take it up. Throttle group
option may be useful feature even in case of fsdev devices.

Regards,
Pradeep







Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-14 Thread Manos Pitsidianakis

On Fri, Jul 14, 2017 at 03:15:06PM +0200, Pradeep Jagadeesh wrote:

Hi Manos,

Thanks for sharing the link to your code patch.

On 7/14/2017 2:22 PM, Manos Pitsidianakis wrote:

Hello Pradeep, you might be interested in my work on refactoring the
block layer's throttling interface in my series:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04191.html

Sure will have a look.

In this series you copy the existing legacy interface we want to get rid
of. I think it will be easier work for you to use the changes introduced
in my patches when they are merged.
So, should I wait till they are in?. Actually my throttle patches for 
fsdev are already upstream (2.9). Now I am just introducing the 
qmp/hmp interfaces for the same.


If you plan on using ThrottleGroups, probably yes, I think, because we'd 
have duplicate interfaces afterwards instead of a unified one. You'd 
have to introduce a qmp/hmp command to set fsdev throttle now and after 
you use throttle groups it will become obsolete.



Have you thought about using throttle groups? It'd mean many devices
sharing the same limits. This way the interfaces can be unified.  Please
read docs/throttle.txt and see if it would be useful for you.


I have read about the throttle group, last year when I was 
implementing the throttle feature for fsdev.My open source work 
depends on the projects I/my group work on. So, when I have more 
bandwidth work on those, I will surely take it up. Throttle group 
option may be useful feature even in case of fsdev devices.


Regards,
Pradeep




signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-14 Thread Pradeep Jagadeesh

Hi Manos,

Thanks for sharing the link to your code patch.

On 7/14/2017 2:22 PM, Manos Pitsidianakis wrote:

Hello Pradeep, you might be interested in my work on refactoring the
block layer's throttling interface in my series:
https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04191.html

Sure will have a look.

In this series you copy the existing legacy interface we want to get rid
of. I think it will be easier work for you to use the changes introduced
in my patches when they are merged.
So, should I wait till they are in?. Actually my throttle patches for 
fsdev are already upstream (2.9). Now I am just introducing the qmp/hmp 
interfaces for the same.

Have you thought about using throttle groups? It'd mean many devices
sharing the same limits. This way the interfaces can be unified.  Please
read docs/throttle.txt and see if it would be useful for you.


I have read about the throttle group, last year when I was implementing 
the throttle feature for fsdev.My open source work depends on the 
projects I/my group work on. So, when I have more bandwidth work on 
those, I will surely take it up. Throttle group option may be useful 
feature even in case of fsdev devices.


Regards,
Pradeep




Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-14 Thread Manos Pitsidianakis
Hello Pradeep, you might be interested in my work on refactoring the 
block layer's throttling interface in my series:

https://lists.nongnu.org/archive/html/qemu-devel/2017-07/msg04191.html

In this series you copy the existing legacy interface we want to get rid 
of. I think it will be easier work for you to use the changes introduced 
in my patches when they are merged. 

Have you thought about using throttle groups? It'd mean many devices 
sharing the same limits. This way the interfaces can be unified.  Please 
read docs/throttle.txt and see if it would be useful for you.


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-07 Thread Pradeep Jagadeesh

On 7/7/2017 8:14 AM, Markus Armbruster wrote:

Pradeep Jagadeesh  writes:


These patches provide the qmp interface, to query the io throttle
status of the all fsdev devices that are present in a vm.
also, it provides an interface to set the io throttle parameters of a
fsdev to a required value. some of the patches also remove the duplicate
code that was present in block and fsdev files.

Pradeep Jagadeesh (6):
  throttle: factor out duplicate code
  qmp: Create IOThrottle structure
  throttle: move out function to reuse the code
  hmp: create a throttle initialization function for code reusability
  fsdev: hmp interface for throttling
  fsdev: QMP interface for throttling

 Makefile|   4 ++
 blockdev.c  |  97 ++---
 fsdev/qemu-fsdev-dummy.c|  10 
 fsdev/qemu-fsdev-throttle.c | 118 ++--
 fsdev/qemu-fsdev-throttle.h |  13 +
 fsdev/qemu-fsdev.c  |  37 +
 hmp-commands-info.hx|  18 ++
 hmp-commands.hx |  19 +++
 hmp.c   |  81 +--
 hmp.h   |   4 ++
 include/qemu/throttle-options.h |   7 +++
 include/qemu/throttle.h |   4 +-
 include/qemu/typedefs.h |   1 +
 monitor.c   |   5 ++
 qapi-schema.json|   3 +
 qapi/block-core.json|  76 +-
 qapi/fsdev.json |  84 
 qapi/iothrottle.json|  88 ++
 qmp.c   |  14 +
 util/throttle.c | 110 +
 20 files changed, 577 insertions(+), 216 deletions(-)
 create mode 100644 qapi/fsdev.json
 create mode 100644 qapi/iothrottle.json


No test coverage?
Not yet, I am yet to write the tests. I can do it only after I finish 
this work.


-Pradeep







Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-06 Thread Markus Armbruster
PS: Sorry for the late review, got a bit overwhelmed...



Re: [Qemu-devel] [PATCH v7 0/6] fsdev: qmp interface for io throttling

2017-07-06 Thread Markus Armbruster
Pradeep Jagadeesh  writes:

> These patches provide the qmp interface, to query the io throttle 
> status of the all fsdev devices that are present in a vm.
> also, it provides an interface to set the io throttle parameters of a
> fsdev to a required value. some of the patches also remove the duplicate
> code that was present in block and fsdev files. 
>
> Pradeep Jagadeesh (6):
>   throttle: factor out duplicate code
>   qmp: Create IOThrottle structure
>   throttle: move out function to reuse the code
>   hmp: create a throttle initialization function for code reusability
>   fsdev: hmp interface for throttling
>   fsdev: QMP interface for throttling
>
>  Makefile|   4 ++
>  blockdev.c  |  97 ++---
>  fsdev/qemu-fsdev-dummy.c|  10 
>  fsdev/qemu-fsdev-throttle.c | 118 
> ++--
>  fsdev/qemu-fsdev-throttle.h |  13 +
>  fsdev/qemu-fsdev.c  |  37 +
>  hmp-commands-info.hx|  18 ++
>  hmp-commands.hx |  19 +++
>  hmp.c   |  81 +--
>  hmp.h   |   4 ++
>  include/qemu/throttle-options.h |   7 +++
>  include/qemu/throttle.h |   4 +-
>  include/qemu/typedefs.h |   1 +
>  monitor.c   |   5 ++
>  qapi-schema.json|   3 +
>  qapi/block-core.json|  76 +-
>  qapi/fsdev.json |  84 
>  qapi/iothrottle.json|  88 ++
>  qmp.c   |  14 +
>  util/throttle.c | 110 +
>  20 files changed, 577 insertions(+), 216 deletions(-)
>  create mode 100644 qapi/fsdev.json
>  create mode 100644 qapi/iothrottle.json

No test coverage?