blockdev: handle error on block latency histogram set error

Commit Message

zhenwei pi Nov. 5, 2018, 3:04 a.m. UTC

Function block_latency_histogram_set may return error, but qapi ignore this.
This can be reproduced easily by qmp command:
virsh qemu-monitor-command INSTANCE '{"execute":"x-block-latency-histogram-set",
"arguments":{"device":"drive-virtio-disk1","boundaries":[10,200,40]}}'
In fact this command does not work, but we still get success result.

qmp_x_block_latency_histogram_set is a batch setting API, report error ASAP.

Signed-off-by: zhenwei pi <[e-mail]>
---
 blockdev.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

Comments

Kevin Wolf Nov. 6, 2018, 4:52 p.m. UTC | #1

Am 05.11.2018 um 04:04 hat zhenwei pi geschrieben:
> Function block_latency_histogram_set may return error, but qapi ignore this.
> This can be reproduced easily by qmp command:
> virsh qemu-monitor-command INSTANCE '{"execute":"x-block-latency-histogram-set",
> "arguments":{"device":"drive-virtio-disk1","boundaries":[10,200,40]}}'
> In fact this command does not work, but we still get success result.
> 
> qmp_x_block_latency_histogram_set is a batch setting API, report error ASAP.
> 
> Signed-off-by: zhenwei pi <[e-mail]>

Thanks, applied to the block branch.

Kevin

Patch

diff --git a/blockdev.c b/blockdev.c
index a8755bd..03b1aa3 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -4377,6 +4377,7 @@  void qmp_x_block_latency_histogram_set(
 {
     BlockBackend *blk = blk_by_name(device);
     BlockAcctStats *stats;
+    int ret;
 
     if (!blk) {
         error_setg(errp, "Device '%s' not found", device);
@@ -4392,21 +4393,33 @@  void qmp_x_block_latency_histogram_set(
     }
 
     if (has_boundaries || has_boundaries_read) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_READ,
             has_boundaries_read ? boundaries_read : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set read boundaries fail", device);
+            return;
+        }
     }
 
     if (has_boundaries || has_boundaries_write) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_WRITE,
             has_boundaries_write ? boundaries_write : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set write boundaries fail", device);
+            return;
+        }
     }
 
     if (has_boundaries || has_boundaries_flush) {
-        block_latency_histogram_set(
+        ret = block_latency_histogram_set(
             stats, BLOCK_ACCT_FLUSH,
             has_boundaries_flush ? boundaries_flush : boundaries);
+        if (ret) {
+            error_setg(errp, "Device '%s' set flush boundaries fail", device);
+            return;
+        }
     }
 }