Re: [PATCH v3 5/9] Implement public API for virDomainSetIOThreads

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

 



On Tue, Feb 17, 2015 at 04:03:54PM -0500, John Ferlan wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1135491
> 
> Add the libvirt API infrastructure to support setting IOThread data.
> For now this is the pinned CPU information, but eventually will also
> support hot plug add/del
> 
> The API will support the LIVE, CONFIG, or CURRENT flags.  If the
> VIR_DOMAIN_IOTHREADS_PIN flag is not provided, it's left up to the
> hypervisor to handle, but when not provided the cpumap/maplen arguments
> are not checked.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  include/libvirt/libvirt-domain.h | 16 ++++++++++
>  src/driver-hypervisor.h          |  8 +++++
>  src/libvirt-domain.c             | 69 ++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  1 +
>  4 files changed, 94 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 9dcc424..e07db16 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1582,11 +1582,27 @@ struct _virDomainIOThreadsInfo {
>      char **resources;                  /* array of resources using IOThread */
>  };
>  
> +/* Flags for controlling virtual IOThread pinning.  */
> +typedef enum {
> +    /* See virDomainModificationImpact for these flags.  */
> +    VIR_DOMAIN_IOTHREADS_CURRENT = VIR_DOMAIN_AFFECT_CURRENT,
> +    VIR_DOMAIN_IOTHREADS_LIVE    = VIR_DOMAIN_AFFECT_LIVE,
> +    VIR_DOMAIN_IOTHREADS_CONFIG  = VIR_DOMAIN_AFFECT_CONFIG,
> +
> +    /* Additionally, these flags may be bitwise-OR'd in.  */
> +    VIR_DOMAIN_IOTHREADS_PIN = (1 << 2), /* thread_id to pin using cpumap */
> +} virDomainIOThreadsFlags;
> +
>  void                 virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr info);
>  
>  int                  virDomainGetIOThreadsInfo(virDomainPtr domain,
>                                                 virDomainIOThreadsInfoPtr **info,
>                                                 unsigned int flags);
> +int                  virDomainSetIOThreads(virDomainPtr domain,
> +                                           unsigned int iothread_val,
> +                                           unsigned char *cpumap,
> +                                           int maplen,
> +                                           unsigned int flags);
>  
>  /**
>   * VIR_USE_CPU:
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 2ce1a51..120d761 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -386,6 +386,13 @@ typedef int
>                                  unsigned int flags);
>  
>  typedef int
> +(*virDrvDomainSetIOThreads)(virDomainPtr domain,
> +                            unsigned int iothread_val,
> +                            unsigned char *cpumap,
> +                            int maplen,
> +                            unsigned int flags);
> +
> +typedef int
>  (*virDrvDomainGetSecurityLabel)(virDomainPtr domain,
>                                  virSecurityLabelPtr seclabel);
>  
> @@ -1260,6 +1267,7 @@ struct _virHypervisorDriver {
>      virDrvDomainGetVcpus domainGetVcpus;
>      virDrvDomainGetMaxVcpus domainGetMaxVcpus;
>      virDrvDomainGetIOThreadsInfo domainGetIOThreadsInfo;
> +    virDrvDomainSetIOThreads domainSetIOThreads;
>      virDrvDomainGetSecurityLabel domainGetSecurityLabel;
>      virDrvDomainGetSecurityLabelList domainGetSecurityLabelList;
>      virDrvNodeGetSecurityModel nodeGetSecurityModel;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index f893b35..bf73773 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -7969,6 +7969,75 @@ virDomainIOThreadsInfoFree(virDomainIOThreadsInfoPtr info)
>  
>  
>  /**
> + * virDomainSetIOThreads:
> + * @domain: a domain object
> + * @iothread_val: either the thread_id to modify or a count of IOThreads
> + *      to be added or removed from the domain depending on the @flags setting
> + * @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
> + *      Each bit set to 1 means that corresponding CPU is usable.
> + *      Bytes are stored in little-endian order: CPU0-7, 8-15...
> + *      In each byte, lowest CPU number is least significant bit.
> + * @maplen: number of bytes in cpumap, from 1 up to size of CPU map in
> + *      underlying virtualization system (Xen...).
> + *      If maplen < size, missing bytes are set to zero.
> + *      If maplen > size, failure code is returned.
> + * @flags: bitwise-OR of supported virDomainIOThreadsFlags
> + *
> + * If the VIR_DOMAIN_IOTHREADS_PIN flag is set, the @iothread_val will be
> + * an existing IOThread to be pinned

I don't really like the sound of this flags & iothread_val usage.
Using flags as a way to multiple different logical operations into
one public API is not very nice design and ends up getting confusing
for developers & ourselves. IMHO we should have a virDomainSetIOThreadsPin()
API for pinning, and another API for adjusting the number of IO threads
to be used.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
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]