Re: [PATCH 1/7] Implement public API for virDomainIOThreadsSet

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

 



On Wed, Mar 25, 2015 at 03:22:03PM -0400, John Ferlan wrote:
> 
> 
> On 03/25/2015 11:37 AM, Peter Krempa wrote:
> > 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.
> > 
> 
> hmmm...  well yes, the model is the Vcpu algorithms and specifically
> SetVcpus.  Allowing unordered IOThread id's will require some rework of
> other algorithms.
> 
> Talking through this ...
> 
> If I start with 0 IOThreads and want 2, is it easier to "SetIOThreads"
> once or "AddIOThread" twice?
> 
> Similarly, if I have 2 IOThreads and want 0, is it easier to
> SetIOThreads once or "DelIOThread" twice?
> 
> If I start with 2 IOThreads, I add 2 IOThreads, then want to remove
> IOThread 3, then does that mean this has similar logic to Vcpus where I
> have to introduce a 'current' value.  Then at (re)startup, I have to
> determine which IOThreads are assigned disks in order to determine which
> of the 3 current to start. The add iothread to disk logic would need
> adjustment to know that IOThread#3 doesn't exist. It's easy to start one
> in the middle with its own number, I just wonder the 'real world' use
> case (not talking the varying array of test matrix patterns that need to
> be validated). Hence the KISS method of managing IOThreads.

Right, so holes in iothreads don't make much sense if we're specifying
them in the domain XML as:
<iothreads>n</iothreads>
And if we ever need to invent an API that does that, new XML will be
needed as well.

It was just a thought that came to mind when looking at the
iothread_id returned in the virDomainIOThreadInfo struct.

Jan

Attachment: signature.asc
Description: Digital 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]