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

Re: [libvirt] [PATCH] domiftune: clean up previous patches



On Mon, Jan 02, 2012 at 02:37:03PM -0700, Eric Blake wrote:
> Most severe here is a latent (but currently untriggered) memory leak
> if any hypervisor ever adds a string interface property; the
> remainder are mainly cosmetic.
> 
> * include/libvirt/libvirt.h.in (VIR_DOMAIN_BANDWIDTH_*): Move
> macros closer to interface that uses them, and document type.
> * src/libvirt.c (virDomainSetInterfaceParameters)
> (virDomainGetInterfaceParameters): Formatting tweaks.
> * daemon/remote.c (remoteDispatchDomainGetInterfaceParameters):
> Avoid memory leak.
> * src/libvirt_public.syms (LIBVIRT_0.9.9): Sort lines.
> * src/libvirt_private.syms (domain_conf.h): Likewise.
> * src/qemu/qemu_driver.c (qemuDomainSetInterfaceParameters): Fix
> comments, break long lines.
> ---
>  daemon/remote.c              |    5 +-
>  include/libvirt/libvirt.h.in |  104 +++++++++++++++++++++++-------------------
>  src/libvirt.c                |   21 ++++++--
>  src/libvirt_private.syms     |   10 ++--
>  src/libvirt_public.syms      |    4 +-
>  src/qemu/qemu_driver.c       |   24 ++++++----
>  6 files changed, 97 insertions(+), 71 deletions(-)
> 
> diff --git a/daemon/remote.c b/daemon/remote.c
> index b7ba321..a28a754 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -1,7 +1,7 @@
>  /*
>   * remote.c: handlers for RPC method calls
>   *
> - * Copyright (C) 2007-2011 Red Hat, Inc.
> + * Copyright (C) 2007-2012 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -3495,9 +3495,10 @@ success:
>  cleanup:
>      if (rv < 0)
>          virNetMessageSaveError(rerr);
> +    virTypedParameterArrayClear(params, nparams);
> +    VIR_FREE(params);
>      if (dom)
>          virDomainFree(dom);
> -    VIR_FREE(params);
>      return rv;
>  }
> 
> diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
> index f59c3b1..ad6fcce 100644
> --- a/include/libvirt/libvirt.h.in
> +++ b/include/libvirt/libvirt.h.in
> @@ -4,7 +4,7 @@
>   * Description: Provides the interfaces of the libvirt library to handle
>   *              virtualized domains
>   *
> - * Copy:  Copyright (C) 2005-2006, 2010-2011 Red Hat, Inc.
> + * Copy:  Copyright (C) 2005-2006, 2010-2012 Red Hat, Inc.
>   *
>   * See COPYING.LIB for the License of this software
>   *
> @@ -645,48 +645,6 @@ typedef virTypedParameter *virTypedParameterPtr;
>   */
>  #define VIR_DOMAIN_SCHEDULER_SHARES "shares"
> 
> -/**
> - * VIR_DOMAIN_BANDWIDTH_IN_AVERAGE:
> - *
> - * Macro represents the inbound average of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_IN_AVERAGE "inbound.average"
> -
> -/**
> - * VIR_DOMAIN_BANDWIDTH_IN_PEAK:
> - *
> - * Macro represents the inbound peak of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_IN_PEAK "inbound.peak"
> -
> -/**
> - * VIR_DOMAIN_BANDWIDTH_IN_BURST:
> - *
> - * Macro represents the inbound burst of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_IN_BURST "inbound.burst"
> -
> -/**
> - * VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE:
> - *
> - * Macro represents the outbound average of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE "outbound.average"
> -
> -/**
> - * VIR_DOMAIN_BANDWIDTH_OUT_PEAK:
> - *
> - * Macro represents the outbound peak of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_OUT_PEAK "outbound.peak"
> -
> -/**
> - * VIR_DOMAIN_BANDWIDTH_OUT_BURST:
> - *
> - * Macro represents the outbound burst of NIC bandwidth.
> - */
> -#define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> -
>  /*
>   * Fetch scheduler parameters, caller allocates 'params' field of size 'nparams'
>   */
> @@ -1488,6 +1446,51 @@ int                     virDomainInterfaceStats (virDomainPtr dom,
>                                                   const char *path,
>                                                   virDomainInterfaceStatsPtr stats,
>                                                   size_t size);
> +
> +/* Management of interface parameters */
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_IN_AVERAGE:
> + *
> + * Macro represents the inbound average of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_IN_AVERAGE "inbound.average"
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_IN_PEAK:
> + *
> + * Macro represents the inbound peak of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_IN_PEAK "inbound.peak"
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_IN_BURST:
> + *
> + * Macro represents the inbound burst of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_IN_BURST "inbound.burst"
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE:
> + *
> + * Macro represents the outbound average of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE "outbound.average"
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_OUT_PEAK:
> + *
> + * Macro represents the outbound peak of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_OUT_PEAK "outbound.peak"
> +
> +/**
> + * VIR_DOMAIN_BANDWIDTH_OUT_BURST:
> + *
> + * Macro represents the outbound burst of NIC bandwidth, as a uint.
> + */
> +#define VIR_DOMAIN_BANDWIDTH_OUT_BURST "outbound.burst"
> +
>  int                     virDomainSetInterfaceParameters (virDomainPtr dom,
>                                                          const char *device,
>                                                          virTypedParameterPtr params,
> @@ -1496,10 +1499,9 @@ int                     virDomainGetInterfaceParameters (virDomainPtr dom,
>                                                          const char *device,
>                                                          virTypedParameterPtr params,
>                                                          int *nparams, unsigned int flags);
> -int                     virDomainMemoryStats (virDomainPtr dom,
> -                                              virDomainMemoryStatPtr stats,
> -                                              unsigned int nr_stats,
> -                                              unsigned int flags);
> +
> +/* Management of domain block devices */
> +
>  int                     virDomainBlockPeek (virDomainPtr dom,
>                                              const char *disk,
>                                              unsigned long long offset,
> @@ -1546,7 +1548,15 @@ int                     virDomainGetBlockInfo(virDomainPtr dom,
>                                                virDomainBlockInfoPtr info,
>                                                unsigned int flags);
> 
> +/* Management of domain memory */
> +
> +int                     virDomainMemoryStats (virDomainPtr dom,
> +                                              virDomainMemoryStatPtr stats,
> +                                              unsigned int nr_stats,
> +                                              unsigned int flags);
> +
>  /* Memory peeking flags. */
> +
>  typedef enum {
>    VIR_MEMORY_VIRTUAL              = 1, /* addresses are virtual addresses */
>    VIR_MEMORY_PHYSICAL             = 2, /* addresses are physical addresses */
> diff --git a/src/libvirt.c b/src/libvirt.c
> index 9507902..feb3ca6 100644
> --- a/src/libvirt.c
> +++ b/src/libvirt.c
> @@ -2,7 +2,7 @@
>   * libvirt.c: Main interfaces for the libvirt library to handle virtualization
>   *           domains from a process running in domain 0
>   *
> - * Copyright (C) 2005-2006, 2008-2011 Red Hat, Inc.
> + * Copyright (C) 2005-2006, 2008-2012 Red Hat, Inc.
>   *
>   * See COPYING.LIB for the License of this software
>   *
> @@ -7084,7 +7084,12 @@ error:
>   *          less than the number of parameters supported)
>   * @flags: bitwise-OR of virDomainModificationImpact
>   *
> - * Currently this function sets bandwidth parameters of interface.
> + * Change a subset or all parameters of interface; currently this
> + * includes bandwidth parameters.  The value of @flags should be
> + * either VIR_DOMAIN_AFFECT_CURRENT, or a bitwise-or of values from
> + * VIR_DOMAIN_AFFECT_LIVE and VIR_DOMAIN_AFFECT_CURRENT, although
> + * hypervisors vary in which flags are supported.
> + *
>   * This function may require privileged access to the hypervisor.
>   *
>   * Returns -1 in case of error, 0 in case of success.
> @@ -7122,7 +7127,9 @@ virDomainSetInterfaceParameters(virDomainPtr domain,
> 
>      if (conn->driver->domainSetInterfaceParameters) {
>          int ret;
> -        ret = conn->driver->domainSetInterfaceParameters(domain, device, params, nparams, flags);
> +        ret = conn->driver->domainSetInterfaceParameters(domain, device,
> +                                                         params, nparams,
> +                                                         flags);
>          if (ret < 0)
>              goto error;
>          return ret;
> @@ -7141,8 +7148,8 @@ error:
>   * @device: the interface name or mac address
>   * @params: pointer to interface parameter objects
>   *          (return value, allocated by the caller)
> - * @nparams: pointer to number of interface parameter
> - * @flags: one of virDomainModificationImpact
> + * @nparams: pointer to number of interface parameter; input and output
> + * @flags: bitwise-OR of virDomainModificationImpact and virTypedParameterFlags
>   *
>   * Get all interface parameters. On input, @nparams gives the size of
>   * the @params array; on output, @nparams gives how many slots were
> @@ -7193,7 +7200,9 @@ virDomainGetInterfaceParameters(virDomainPtr domain,
> 
>      if (conn->driver->domainGetInterfaceParameters) {
>          int ret;
> -        ret = conn->driver->domainGetInterfaceParameters (domain, device, params, nparams, flags);
> +        ret = conn->driver->domainGetInterfaceParameters(domain, device,
> +                                                         params, nparams,
> +                                                         flags);
>          if (ret < 0)
>              goto error;
>          return ret;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 216a5a5..ac2c52e 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -260,10 +260,10 @@ virDomainConfigFile;
>  virDomainControllerDefFree;
>  virDomainControllerInsert;
>  virDomainControllerInsertPreAlloced;
> -virDomainControllerModelUSBTypeFromString;
> -virDomainControllerModelUSBTypeToString;
>  virDomainControllerModelSCSITypeFromString;
>  virDomainControllerModelSCSITypeToString;
> +virDomainControllerModelUSBTypeFromString;
> +virDomainControllerModelUSBTypeToString;
>  virDomainControllerTypeToString;
>  virDomainCpuSetFormat;
>  virDomainCpuSetParse;
> @@ -308,7 +308,6 @@ virDomainDiskSnapshotTypeFromString;
>  virDomainDiskSnapshotTypeToString;
>  virDomainDiskTypeFromString;
>  virDomainDiskTypeToString;
> -virDomainNetFind;
>  virDomainFSDefFree;
>  virDomainFSTypeFromString;
>  virDomainFSTypeToString;
> @@ -366,12 +365,13 @@ virDomainLoadAllConfigs;
>  virDomainMemballoonModelTypeFromString;
>  virDomainMemballoonModelTypeToString;
>  virDomainNetDefFree;
> +virDomainNetFind;
>  virDomainNetGetActualBandwidth;
>  virDomainNetGetActualBridgeName;
>  virDomainNetGetActualDirectDev;
>  virDomainNetGetActualDirectMode;
> -virDomainNetGetActualType;
>  virDomainNetGetActualDirectVirtPortProfile;
> +virDomainNetGetActualType;
>  virDomainNetIndexByMac;
>  virDomainNetInsert;
>  virDomainNetRemoveByMac;
> @@ -462,9 +462,9 @@ virDomainVideoDefaultRAM;
>  virDomainVideoDefaultType;
>  virDomainVideoTypeFromString;
>  virDomainVideoTypeToString;
> +virDomainVirtTypeToString;
>  virDomainVirtioEventIdxTypeFromString;
>  virDomainVirtioEventIdxTypeToString;
> -virDomainVirtTypeToString;
>  virDomainWatchdogActionTypeFromString;
>  virDomainWatchdogActionTypeToString;
>  virDomainWatchdogModelTypeFromString;
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index ea6d562..4ca7216 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -510,10 +510,10 @@ LIBVIRT_0.9.8 {
> 
>  LIBVIRT_0.9.9 {
>      global:
> -        virDomainGetNumaParameters;
> -        virDomainSetNumaParameters;
>          virDomainGetInterfaceParameters;
> +        virDomainGetNumaParameters;
>          virDomainSetInterfaceParameters;
> +        virDomainSetNumaParameters;
>  } LIBVIRT_0.9.8;
> 
>  # .... define new API here using predicted next version number ....
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e93fe87..ad592d6 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1,7 +1,7 @@
>  /*
>   * qemu_driver.c: core driver methods for managing qemu guests
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2012 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -7920,7 +7920,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_AVERAGE)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth average tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth average tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7928,7 +7929,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_PEAK)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth peak tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth peak tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7936,7 +7938,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_IN_BURST)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth burst tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth burst tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7944,7 +7947,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_AVERAGE)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth average tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth average tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7952,7 +7956,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_PEAK)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth peak tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth peak tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7960,7 +7965,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          } else if (STREQ(param->field, VIR_DOMAIN_BANDWIDTH_OUT_BURST)) {
>              if (param->type != VIR_TYPED_PARAM_UINT) {
>                  qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> -                                _("invalid type for bandwidth burst tunable, expected a 'unsigned int'"));
> +                                _("invalid type for bandwidth burst tunable, "
> +                                  "expected an 'unsigned int'"));
>                  goto cleanup;
>              }
> 
> @@ -7973,9 +7979,9 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>          }
>      }
> 
> -    /* average is mandatory, peak and burst is optional. So if no
> +    /* average is mandatory, peak and burst are optional. So if no
>       * average is given, we free inbound/outbound here which causes
> -     * inbound/outbound won't be set. */
> +     * inbound/outbound to not be set. */
>      if (!bandwidth->in->average) {
>          VIR_FREE(bandwidth->in);
>          bandwidth->in = NULL;
> -- 
> 1.7.7.4
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list

ACK.

-- 
Thanks,
Hu Tao


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