On 10/07/2018 03:00 PM, John Ferlan wrote:
> Create a new API that will allow an adjustment of IOThread
> polling parameters for the specified IOThread. These parameters
> will not be saved in the guest XML. Currently the only parameters
> supported will allow the hypervisor to adjust the parameters used
> to limit and alter the scope of the polling interval. The polling
> interval allows the IOThread to spend more or less time processing
> in the guest.
>
> Based on code originally posted by Pavel Hrdina <phrdina@xxxxxxxxxx>
> to add virDomainAddIOThreadParams and virDomainModIOThreadParams.
> Modification of those changes to use virDomainSetIOThreadParams
> instead and remove concepts related to saving the data in guest
> XML as well as the way to specifically enable the polling parameters.
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> include/libvirt/libvirt-domain.h | 44 ++++++++++++++++++++
> src/driver-hypervisor.h | 8 ++++
> src/libvirt-domain.c | 71 ++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 5 +++
> src/remote/remote_driver.c | 1 +
> src/remote/remote_protocol.x | 21 +++++++++-
> src/remote_protocol-structs | 10 +++++
> 7 files changed, 159 insertions(+), 1 deletion(-)
>
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 58fd4bc10c..bf89d0149f 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1911,6 +1911,50 @@ int virDomainDelIOThread(virDomainPtr domain,
> unsigned int iothread_id,
> unsigned int flags);
>
> +/* IOThread set parameters */
> +
> +/**
> + * VIR_DOMAIN_IOTHREAD_POLL_MAX_NS:
> + *
> + * The maximum polling time that can be used by polling algorithm in ns.
> + * The polling time starts at 0 (zero) and is the time spent by the guest
> + * to process IOThread data before returning the CPU to the host. The
> + * polling time will be dynamically modified over time based on the
> + * poll_grow and poll_shrink parameters provided. A value set too large
> + * will cause more CPU time to be allocated the guest. A value set too
> + * small will not provide enough cycles for the guest to process data.
> + * The polling interval is not available for statistical purposes.
> + */
> +# define VIR_DOMAIN_IOTHREAD_POLL_MAX_NS "poll_max_ns"
> +
> +/**
> + * VIR_DOMAIN_IOTHREAD_POLL_GROW:
> + *
> + * This provides a value for the dynamic polling adjustment algorithm to
> + * use to grow its polling interval up to the poll_max_ns value. A value
> + * of 0 (zero) allows the hypervisor to choose its own value. The algorithm
> + * to use for adjustment is hypervisor specific.
> + */
> +# define VIR_DOMAIN_IOTHREAD_POLL_GROW "poll_grow"
> +
> +/**
> + * VIR_DOMAIN_IOTHREAD_POLL_SHRINK:
> + *
> + * This provides a value for the dynamic polling adjustment algorithm to
> + * use to shrink its polling interval when the polling interval exceeds
> + * the poll_max_ns value. A value of 0 (zero) allows the hypervisor to
> + * choose its own value. The algorithm to use for adjustment is hypervisor
> + * specific.
> + */
> +# define VIR_DOMAIN_IOTHREAD_POLL_SHRINK "poll_shrink"
> +
> +int virDomainSetIOThreadParams(virDomainPtr domain,
> + unsigned int iothread_id,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags);
> +
> +
> /**
> * VIR_USE_CPU:
> * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN/OUT)
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index eef31eb1f0..6be3e175ce 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -406,6 +406,13 @@ typedef int
> unsigned int iothread_id,
> unsigned int flags);
>
> +typedef int
> +(*virDrvDomainSetIOThreadParams)(virDomainPtr domain,
> + unsigned int iothread_id,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags);
> +
> typedef int
> (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
> virSecurityLabelPtr seclabel);
> @@ -1407,6 +1414,7 @@ struct _virHypervisorDriver {
> virDrvDomainPinIOThread domainPinIOThread;
> virDrvDomainAddIOThread domainAddIOThread;
> virDrvDomainDelIOThread domainDelIOThread;
> + virDrvDomainSetIOThreadParams domainSetIOThreadParams;
> virDrvDomainGetSecurityLabel domainGetSecurityLabel;
> virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
> virDrvNodeGetSecurityModel nodeGetSecurityModel;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 9fda56d660..ce5de4b208 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -7812,6 +7812,77 @@ virDomainDelIOThread(virDomainPtr domain,
> }
>
>
> +/**
> + * virDomainSetIOThreadParams:
> + * @domain: a domain object
> + * @iothread_id: the specific IOThread ID value to add
> + * @params: pointer to IOThread parameter objects
> + * @nparams: number of IOThread parameters
> + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags
> + *
> + * Dynamically set IOThread parameters to the domain. It is left up to
> + * the underlying virtual hypervisor to determine the valid range for an
> + * @iothread_id, determining whether the @iothread_id already exists, and
> + * determining the validity of the provided param values.
> + *
> + * See VIR_DOMAIN_IOTHREAD_* for detailed description of accepted IOThread
> + * parameters.
> + *
> + * Since the purpose of this API is to dynamically modify the IOThread
> + * @flags should only include the VIR_DOMAIN_AFFECT_CURRENT and/or
> + * VIR_DOMAIN_AFFECT_LIVE virDomainMemoryModFlags. Setting other flags
> + * may cause errors from the hypervisor.
> + *
> + * Note that this call can fail if the underlying virtualization hypervisor
> + * does not support it or does not support setting the provided values.
> + *
> + * This function requires privileged access to the hypervisor.
> + *
> + * Returns 0 in case of success, -1 in case of failure.
> + */
> +int
> +virDomainSetIOThreadParams(virDomainPtr domain,
> + unsigned int iothread_id,
> + virTypedParameterPtr params,
> + int nparams,
> + unsigned int flags)
> +{
> + virConnectPtr conn;
> +
> + VIR_DOMAIN_DEBUG(domain, "iothread_id=%u, params=%p, nparams=%d, flags=0x%x",
> + iothread_id, params, nparams, flags);
> + VIR_TYPED_PARAMS_DEBUG(params, nparams);
> +
> + virResetLastError();
> +
> + virCheckDomainReturn(domain, -1);
> + conn = domain->conn;
> +
> + virCheckReadOnlyGoto(conn->flags, error);
> + virCheckNonNegativeArgGoto(nparams, error);
> + if (nparams)
> + virCheckNonNullArgGoto(params, error);
So this allows call like this:
virDomainSetIOThreadParams(.params = NULL, .nparams = 0);
(yeah, weird mixture of C and C++ call, but you get the idea)
I don't think it makes sense to allow it.
The checks should be as follows IMO:
virCheckPositiveArgGoto();
virCheckNonNullArgGoto();
> +
> + if (virTypedParameterValidateSet(conn, params, nparams) < 0)
> + goto error;
> +
> + if (conn->driver->domainSetIOThreadParams) {
> + int ret;
> + ret = conn->driver->domainSetIOThreadParams(domain, iothread_id,
> + params, nparams, flags);
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> +
> + error:
> + virDispatchError(domain->conn);
> + return -1;
> +}
ACK
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
