Re: [libvirt] [PATCH v2 04/12] qemu: Introduce qemuDomainSetBlockIoTuneSetDefaults

On Thu, Oct 06, 2016 at 06:38:52PM -0400, John Ferlan wrote: > Create a helper to set the bytes/iops iotune default values based on > the current qemu setting > > Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; > --- > src/qemu/qemu_driver.c | 66 ++++++++++++++++++++++++++++++-------------------- > 1 file changed, 40 insertions(+), 26 deletions(-) > > diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c > index 78e917e..fcce3f0 100644 > --- a/src/qemu/qemu_driver.c > +++ b/src/qemu/qemu_driver.c > @@ -17286,6 +17286,43 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, > return ret; > } > > + > +/* If the user didn't specify bytes limits, inherit previous values; > + * likewise if the user didn't specify iops limits. */ > +static void > +qemuDomainSetBlockIoTuneSetDefaults(virDomainBlockIoTuneInfoPtr newinfo, > + virDomainBlockIoTuneInfoPtr oldinfo, > + bool set_bytes, > + bool set_iops, > + bool set_bytes_max, > + bool set_iops_max, > + bool set_size_iops) > +{ Could this be a macro instead? 5 booleans, it just seems a tiny bit odd.

> + if (!set_bytes) { > + newinfo->total_bytes_sec = oldinfo->total_bytes_sec; > + newinfo->read_bytes_sec = oldinfo->read_bytes_sec; > + newinfo->write_bytes_sec = oldinfo->write_bytes_sec; > + } And then you'll end up just with snippet like ^^this. Or you could use bitwise OR'd flags instead of the booleans, as it scales better (in terms of number of arguments), though I'm not very fond of that even though it works. ACK with the adjustment, but see 5/12 as well before pushing.