Re: [libvirt] [PATCH v5 4/5] qemu: Add support to pin IOThreads to specific CPU
On 03/10/2015 09:51 AM, Ján Tomko wrote:
On Fri, Mar 06, 2015 at 09:05:44AM -0500, John Ferlan wrote:
<...snip...>
> +
> + /* pinning to all physical cpus means resetting,
It doesn't.
By default the iothreads inherit the pinning from the domain's cpumask.
A completely clear bitmap would be a better value to mean resetting,
since it makes no sense otherwise. But getting the cpumask in that case
won't be that easy.
So again - this is taken from qemuDomainPinVcpuFlags() - if it's invalid
here, then it's invalid there as well.
Is your objection the comment? Should it be striken or restated?
> + * so check if we can reset setting.
> + */
> + if (virBitmapIsAllSet(pcpumap))
> + doReset = true;
> +
> + if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> +
> + if (priv->iothreadpids == NULL) {
> + virReportError(VIR_ERR_OPERATION_INVALID,
> + "%s", _("IOThread affinity is not
supported"));
> + goto endjob;
> + }
> +
> + if (iothread_id > priv->niothreadpids) {
> + virReportError(VIR_ERR_INVALID_ARG,
> + _("iothread value out of range %d > %d"),
> + iothread_id, priv->niothreadpids);
> + goto endjob;
> + }
> +
> + if (vm->def->cputune.iothreadspin) {
> + /* The VcpuPinDefCopy works for IOThreads too */
Maybe it should be renamed to something like virDomainPinDefCopy then?
Perhaps, but that seems outside the scope of these changes. The 'reuse'
of virDomainVcpuPinDefPtr by the IOThreads code was an "optimization"
that could also then be generalized I suppose (eg change from
virDomainVcpuPinDefPtr to virDomainPinDefPtr), but that's a much more
invasive change which would seemingly require change the structure
"vcpuid" value to just "id".
John