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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> ---
>  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

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]