On Wed, Mar 25, 2015 at 16:29:22 +0100, Ján Tomko wrote:
> On Thu, Mar 19, 2015 at 01:08:22PM -0400, John Ferlan wrote:
> > Add virDomainIOThreadsSet to allow setting the number of IOThreads for
> > a domain.
> >
> > The API supports updating both the --live domain and the --config data.
> >
> > Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> > ---
> > include/libvirt/libvirt-domain.h | 3 ++
> > src/driver-hypervisor.h | 6 ++++
> > src/libvirt-domain.c | 60 ++++++++++++++++++++++++++++++++++++++++
> > src/libvirt_public.syms | 1 +
> > 4 files changed, 70 insertions(+)
> >
>
> This one has been siting there for a while, I'll think out loud just to
> get a discussion going.
>
> > --- a/src/libvirt-domain.c
> > +++ b/src/libvirt-domain.c
> > @@ -8042,6 +8042,66 @@ virDomainPinIOThread(virDomainPtr domain,
> >
> >
> > /**
> > + * virDomainSetIOThreads:
> > + * @domain: pointer to domain object
> > + * @niothreads: the new number of IOThreads for this domain
> > + * @flags: bitwise-OR of virDomainModificationImpact
> > + *
> > + * Dynamically change the number of IOThreads used by the domain.
> > + * Note that this call faiy fail if the underlying virtualization hypervisor
> > + * does not support it or if growing the number is arbitrarily limited.
> > + * This function may require privileged access to the hypervisor.
> > + *
> > + * @flags may include VIR_DOMAIN_AFFECT_LIVE to affect a running
> > + * domain (which may fail if domain is not active), or
> > + * VIR_DOMAIN_AFFECT_CONFIG to affect the next boot via the XML
> > + * description of the domain. Both flags may be set.
> > + * If neither flag is specified (that is, @flags is VIR_DOMAIN_AFFECT_CURRENT),
> > + * then an inactive domain modifies persistent setup, while an active domain
> > + * is hypervisor-dependent on whether just live or both live and persistent
> > + * state is changed.
> > + *
> > + * Not all hypervisors can support all flag combinations.
> > + *
> > + * Returns 0 in case of success, -1 in case of failure.
> > + */
> > +int
> > +virDomainSetIOThreads(virDomainPtr domain,
> > + unsigned int niothreads,
> > + unsigned int flags)
> > +{
>
> This seems ok for the use case where we just give a domain a few
> iothreads and all the disks will share them.
>
> But if you go for one iothread per disk, creating holes in iothread
> indexes could make sense - we allow unplugging disks from the middle.
>
> I can't think of a case where hotplugging a thread with id=6 would be
> useful if there are only 3 threads.
>
> Maybe there should be separate APIs for adding and deleting threads?
I have to agree here. Separate API for adding and removing (or perhaps
just a parameter to switch?) specific threads is the way to go. Why?
Well this function looks like it was copied from virDomainSetVcpus which
I'm currently planning to replace with specific vcpu hotplug api.
The reasons there are the same as Jan pointed out just translated into
the context of vCPUS. At the point where there is no other mapping or
relationship of belonging the API is fine. This was true for vCPUs in
the pre-NUMA era. With NUMA, (or disks IO executed by iothreads in this
case) you have a relationship where a thread (or vCPU) belongs to a dis
(or NUMA node) and you need to be able to select them individually.
>
> Jan
Peter
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list