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

Re: [libvirt] [PATCH 01/40] Remove xen driver checks for priv->handle < 0



Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
>
> The Xen hypervisor driver checks for 'priv->handle < 0' and
> returns -1, but without raising any error. Fortunately this
> code will never be executed, since the main Xen driver always
> checks 'priv->opened[XEN_UNIFIED_HYPERVISOR_OFFSET]' prior
> to invoking any hypervisor API. Just the redundant checks
>   

s/Just the/Just remove the/

> for priv->handle
>
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  src/xen/xen_hypervisor.c | 98 ++++++++----------------------------------------
>  1 file changed, 16 insertions(+), 82 deletions(-)
>   

ACK.

Regards,
Jim

> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index 9dbbe07..d9941ec 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -1165,11 +1165,6 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
>      char *schedulertype = NULL;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("priv->handle invalid"));
> -        return NULL;
> -    }
>      if (domain->id < 0) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
>                         "%s", _("domain is not running"));
> @@ -1240,11 +1235,7 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>  {
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("priv->handle invalid"));
> -        return -1;
> -    }
> +
>      if (domain->id < 0) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
>                         "%s", _("domain is not running"));
> @@ -1353,11 +1344,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>                                         NULL) < 0)
>          return -1;
>  
> -    if (priv->handle < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("priv->handle invalid"));
> -        return -1;
> -    }
>      if (domain->id < 0) {
>          virReportError(VIR_ERR_OPERATION_INVALID,
>                         "%s", _("domain is not running"));
> @@ -2209,7 +2195,7 @@ xenHypervisorOpen(virConnectPtr conn,
>      virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
>  
>      if (xenHypervisorInitialize() < 0)
> -        return -1;
> +        return VIR_DRV_OPEN_ERROR;
>  
>      priv->handle = -1;
>  
> @@ -2221,7 +2207,7 @@ xenHypervisorOpen(virConnectPtr conn,
>  
>      priv->handle = ret;
>  
> -    return 0;
> +    return VIR_DRV_OPEN_SUCCESS;
>  }
>  
>  /**
> @@ -2238,9 +2224,6 @@ xenHypervisorClose(virConnectPtr conn)
>      int ret;
>      xenUnifiedPrivatePtr priv = conn->privateData;
>  
> -    if (priv->handle < 0)
> -        return -1;
> -
>      ret = VIR_CLOSE(priv->handle);
>      if (ret < 0)
>          return -1;
> @@ -2259,12 +2242,8 @@ xenHypervisorClose(virConnectPtr conn)
>   * Returns 0 in case of success, -1 in case of error
>   */
>  int
> -xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer)
> +xenHypervisorGetVersion(virConnectPtr conn ATTRIBUTE_UNUSED, unsigned long *hvVer)
>  {
> -    xenUnifiedPrivatePtr priv = conn->privateData;
> -
> -    if (priv->handle < 0)
> -        return -1;
>      *hvVer = (hv_versions.hv >> 16) * 1000000 + (hv_versions.hv & 0xFFFF) * 1000;
>      return 0;
>  }
> @@ -2769,9 +2748,6 @@ xenHypervisorNumOfDomains(virConnectPtr conn)
>      int maxids = last_maxids;
>      xenUnifiedPrivatePtr priv = conn->privateData;
>  
> -    if (priv->handle < 0)
> -        return -1;
> -
>   retry:
>      if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
>          virReportOOMError();
> @@ -2823,9 +2799,6 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids)
>      int ret, nbids, i;
>      xenUnifiedPrivatePtr priv = conn->privateData;
>  
> -    if (priv->handle < 0)
> -        return -1;
> -
>      if (maxids == 0)
>          return 0;
>  
> @@ -2866,12 +2839,6 @@ xenHypervisorDomainGetOSType(virDomainPtr dom)
>      xen_getdomaininfo dominfo;
>      char *ostype = NULL;
>  
> -    if (priv->handle < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("domain shut off or invalid"));
> -        return NULL;
> -    }
> -
>      /* HV's earlier than 3.1.0 don't include the HVM flags in guests status*/
>      if (hv_versions.hypervisor < 2 ||
>          hv_versions.dom_interface < 4) {
> @@ -2911,9 +2878,6 @@ xenHypervisorHasDomain(virConnectPtr conn, int id)
>      xenUnifiedPrivatePtr priv = conn->privateData;
>      xen_getdomaininfo dominfo;
>  
> -    if (priv->handle < 0)
> -        return 0;
> -
>      XEN_GETDOMAININFO_CLEAR(dominfo);
>  
>      if (virXen_getdomaininfo(priv->handle, id, &dominfo) < 0)
> @@ -2933,9 +2897,6 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, int id)
>      virDomainPtr ret;
>      char *name;
>  
> -    if (priv->handle < 0)
> -        return NULL;
> -
>      XEN_GETDOMAININFO_CLEAR(dominfo);
>  
>      if (virXen_getdomaininfo(priv->handle, id, &dominfo) < 0)
> @@ -2967,9 +2928,6 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid)
>      char *name;
>      int maxids = 100, nids, i, id;
>  
> -    if (priv->handle < 0)
> -        return NULL;
> -
>   retry:
>      if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
>          virReportOOMError();
> @@ -3030,13 +2988,9 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid)
>   * Returns the maximum of CPU defined by Xen.
>   */
>  int
> -xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED)
> +xenHypervisorGetMaxVcpus(virConnectPtr conn ATTRIBUTE_UNUSED,
> +                         const char *type ATTRIBUTE_UNUSED)
>  {
> -    xenUnifiedPrivatePtr priv = conn->privateData;
> -
> -    if (priv->handle < 0)
> -        return -1;
> -
>      return MAX_VIRT_CPUS;
>  }
>  
> @@ -3057,9 +3011,6 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
>      xen_getdomaininfo dominfo;
>      int ret;
>  
> -    if (priv->handle < 0)
> -        return 0;
> -
>      if (kb_per_pages == 0) {
>          kb_per_pages = sysconf(_SC_PAGESIZE) / 1024;
>          if (kb_per_pages <= 0)
> @@ -3089,9 +3040,7 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id)
>  static unsigned long long ATTRIBUTE_NONNULL(1)
>  xenHypervisorGetMaxMemory(virDomainPtr domain)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return 0;
>  
>      return xenHypervisorGetDomMaxMemory(domain->conn, domain->id);
> @@ -3121,9 +3070,6 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>              kb_per_pages = 4;
>      }
>  
> -    if (priv->handle < 0)
> -        return -1;
> -
>      memset(info, 0, sizeof(virDomainInfo));
>      XEN_GETDOMAININFO_CLEAR(dominfo);
>  
> @@ -3189,9 +3135,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info)
>  int
>  xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
> -
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      return xenHypervisorGetDomInfo(domain->conn, domain->id, info);
> @@ -3215,12 +3159,11 @@ xenHypervisorGetDomainState(virDomainPtr domain,
>                              int *reason,
>                              unsigned int flags)
>  {
> -    xenUnifiedPrivatePtr priv = domain->conn->privateData;
>      virDomainInfo info;
>  
>      virCheckFlags(0, -1);
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      if (xenHypervisorGetDomInfo(domain->conn, domain->id, &info) < 0)
> @@ -3281,12 +3224,6 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn,
>          return -1;
>      }
>  
> -    if (priv->handle < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("priv->handle invalid"));
> -        return -1;
> -    }
> -
>      memset(&op_sys, 0, sizeof(op_sys));
>      op_sys.cmd = XEN_V2_OP_GETAVAILHEAP;
>  
> @@ -3322,7 +3259,7 @@ xenHypervisorPauseDomain(virDomainPtr domain)
>      int ret;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      ret = virXen_pausedomain(priv->handle, domain->id);
> @@ -3345,7 +3282,7 @@ xenHypervisorResumeDomain(virDomainPtr domain)
>      int ret;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      ret = virXen_unpausedomain(priv->handle, domain->id);
> @@ -3374,7 +3311,7 @@ xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags)
>  
>      virCheckFlags(0, -1);
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      ret = virXen_destroydomain(priv->handle, domain->id);
> @@ -3398,7 +3335,7 @@ xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory)
>      int ret;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      ret = virXen_setmaxmem(priv->handle, domain->id, memory);
> @@ -3424,7 +3361,7 @@ xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus)
>      int ret;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0 || domain->id < 0 || nvcpus < 1)
> +    if (domain->id < 0 || nvcpus < 1)
>          return -1;
>  
>      ret = virXen_setmaxvcpus(priv->handle, domain->id, nvcpus);
> @@ -3452,7 +3389,7 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu,
>      int ret;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0 || domain->id < 0)
> +    if (domain->id < 0)
>          return -1;
>  
>      ret = virXen_setvcpumap(priv->handle, domain->id, vcpu,
> @@ -3494,7 +3431,7 @@ xenHypervisorGetVcpus(virDomainPtr domain,
>      virVcpuInfoPtr ipt;
>      int nbinfo, i;
>  
> -    if (priv->handle < 0 || domain->id < 0 || sizeof(cpumap_t) & 7) {
> +    if (domain->id < 0 || sizeof(cpumap_t) & 7) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>                         _("domain shut off or invalid"));
>          return -1;
> @@ -3556,9 +3493,6 @@ xenHypervisorGetVcpuMax(virDomainPtr domain)
>      int maxcpu;
>      xenUnifiedPrivatePtr priv = domain->conn->privateData;
>  
> -    if (priv->handle < 0)
> -        return -1;
> -
>      /* inactive domain */
>      if (domain->id < 0) {
>          maxcpu = MAX_VIRT_CPUS;
>   


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