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

[Erik Skultety]

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 at redhat.com>
> ---
>  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.

Erik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20161025/43eef3e5/attachment-0001.sig>