Re: [libvirt] [PATCH v4 4/6] qemu: Add bps_max and friends QMP suport

On 30.09.2014 16:09, Matthias Gatto wrote: > > Detect if the the qemu binary currently in use suport the bps_max option, > If yes add it to the command, if not, just ignore the options. > > Signed-off-by: Matthias Gatto <matthias.gatto(a)outscale.com&gt; > --- > src/qemu/qemu_monitor_json.c | 59 > +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 48 insertions(+), 11 deletions(-) > > diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c > index a8759dd..bef7b5b 100644 > --- a/src/qemu/qemu_monitor_json.c > +++ b/src/qemu/qemu_monitor_json.c > @@ -4029,6 +4029,12 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon, > } > > > +#define GET_THROTTLE_STATS_OPTIONAL(FIELD, STORE) > \ > + if (virJSONValueObjectGetNumberUlong(inserted, > \ > + FIELD, > \ > + &reply->STORE) < 0) { > \ > + reply->STORE = 0; > \ > + } Well, this is going to be called if-and-only-if qemu supports the new flags. Shouldn't we error out here rather than silently hiding a bug? > #define GET_THROTTLE_STATS(FIELD, STORE) > \ > if (virJSONValueObjectGetNumberUlong(inserted, > \ > FIELD, > \ > @@ -4050,7 +4056,6 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr > result, > size_t i; > bool found = false; > > - (void)supportMaxOptions; > io_throttle = virJSONValueObjectGet(result, "return"); > > if (!io_throttle || io_throttle->type != VIR_JSON_TYPE_ARRAY) { > @@ -4096,6 +4101,16 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr > result, > GET_THROTTLE_STATS("iops", total_iops_sec); > GET_THROTTLE_STATS("iops_rd", read_iops_sec); > GET_THROTTLE_STATS("iops_wr", write_iops_sec); > + if (supportMaxOptions) > + { > + GET_THROTTLE_STATS_OPTIONAL("bps_max", total_bytes_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("bps_rd_max", > read_bytes_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("bps_wr_max", > write_bytes_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("iops_max", total_iops_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("iops_rd_max", > read_iops_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("iops_wr_max", > write_iops_sec_max); > + GET_THROTTLE_STATS_OPTIONAL("iops_size", size_iops_sec); This should be GET_THROTTLE_STAT() then. > + } > > break; > } > @@ -4112,6 +4127,7 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr > result, > return ret; > } > #undef GET_THROTTLE_STATS > +#undef GET_THROTTLE_STATS_OPTIONAL > > int qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, > const char *device, > @@ -4122,16 +4138,37 @@ int > qemuMonitorJSONSetBlockIoThrottle(qemuMonitorPtr mon, > virJSONValuePtr cmd = NULL; > virJSONValuePtr result = NULL; > > - (void)supportMaxOptions; > - cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", > - "s:device", device, > - "U:bps", info->total_bytes_sec, > - "U:bps_rd", info->read_bytes_sec, > - "U:bps_wr", info->write_bytes_sec, > - "U:iops", info->total_iops_sec, > - "U:iops_rd", info->read_iops_sec, > - "U:iops_wr", info->write_iops_sec, > - NULL); > + if (supportMaxOptions) > + { > + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", > + "s:device", device, > + "U:bps", info->total_bytes_sec, > + "U:bps_rd", > info->read_bytes_sec, > + "U:bps_wr", > info->write_bytes_sec, > + "U:iops", info->total_iops_sec, > + "U:iops_rd", > info->read_iops_sec, > + "U:iops_wr", > info->write_iops_sec, > + "U:bps_max", > info->total_bytes_sec_max, > + "U:bps_rd_max", > info->read_bytes_sec_max, > + "U:bps_wr_max", > info->write_bytes_sec_max, > + "U:iops_max", > info->total_iops_sec_max, > + "U:iops_rd_max", > info->read_iops_sec_max, > + "U:iops_wr_max", > info->write_iops_sec_max, > + "U:iops_size", > info->size_iops_sec, > + NULL); > + } > + else > + { > + cmd = qemuMonitorJSONMakeCommand("block_set_io_throttle", > + "s:device", device, > + "U:bps", info->total_bytes_sec, > + "U:bps_rd", > info->read_bytes_sec, > + "U:bps_wr", > info->write_bytes_sec, > + "U:iops", info->total_iops_sec, > + "U:iops_rd", > info->read_iops_sec, > + "U:iops_wr", > info->write_iops_sec, > + NULL); > + } > if (!cmd) > return -1; > > Also, 'make syntax-check' fails here. Michal