|
From: | Pradeep Jagadeesh |
Subject: | Re: [Qemu-devel] [PATCH v3 2/4] fsdev: QMP interface for throttling |
Date: | Fri, 5 May 2017 13:09:14 +0200 |
User-agent: | Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 |
On 5/4/2017 5:19 PM, Eric Blake wrote:
OK, but here the id is only with * (Optional), others are not. Still need them to set?On 05/04/2017 10:12 AM, Pradeep Jagadeesh wrote:+ IOThrottle throttle = { + .has_id = true, + .id = (char *) qdict_get_str(qdict, "id"), + .bps = qdict_get_int(qdict, "bps"), + .bps_rd = qdict_get_int(qdict, "bps_rd"), + .bps_wr = qdict_get_int(qdict, "bps_wr"), + .iops = qdict_get_int(qdict, "iops"), + .iops_rd = qdict_get_int(qdict, "iops_rd"), + .iops_wr = qdict_get_int(qdict, "iops_wr"),Again, don't you need to be setting .has_bps=true and so on?Same as above. I have not set any max burst values here, because I wanted to keep it in line with the block device. May be there is a room to enable these max values in both in future.I don't think you were getting the point of my question. Since 'bps' is an optional member of struct IOThrottle, you have to set 'has_bps' at the same time to make it obvious that 'bps' should affect things.I need some more clarifications here. I did not understand what do you mean by an optional parameter? You mean I need to set "has_*" for all the parameters?Yes, any optional parameter (one named '*foo' in the .json file) has a counterpart has_foo boolean, which should be set to true when the parameter is in use (and thus making it obvious when it is set to false that it is not in use).
I understand that you aren't setting 'bps_max', nor it's counterpart 'has_bps_max', and that's not what I was asking about.+++ b/qapi/iothrottle.json @@ -3,6 +3,7 @@ ## # == QAPI IOThrottle definitions ## +## # @IOThrottle:This looks like a spurious changeYou mean the below sentence is not required?No, the above addition of a second ## is not required.If I remove that I will get the compilation error. I think the parser needs that.If the parser chokes without it, then that implies patch 1 is broken, and the fix should be squashed there for full bisectability. Either way, it looks like a spurious change when it is occurring in patch 3.
Yes, I reordered the patches and fixed this issue. -Pradeep
[Prev in Thread] | Current Thread | [Next in Thread] |