Re: [PATCH 12/23] Replace use of custom macros with virReportError in the Xen drivers

[Peter Krempa <pkrempa@xxxxxxxxxx>]

On 07/18/12 20:40, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Update the legacy Xen drivers to use virReportError instead of
> the statsError, virXenInotifyError, virXenStoreError,
> virXendError, xenUnifiedError, xenXMError custom macros
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>   cfg.mk                   |    6 -
>   src/xen/block_stats.c    |   54 +++---
>   src/xen/xen_driver.c     |  114 ++++++------
>   src/xen/xen_hypervisor.c |  211 +++++++++++----------
>   src/xen/xen_inotify.c    |   48 +++--
>   src/xen/xend_internal.c  |  461 +++++++++++++++++++++++-----------------------
>   src/xen/xm_internal.c    |  134 +++++++-------
>   src/xen/xs_internal.c    |   40 ++--
>   8 files changed, 514 insertions(+), 554 deletions(-)
>
> diff --git a/cfg.mk b/cfg.mk

ACK to changes in this file

> diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c

ACK to changes in this file.

> diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
> index b4ec579..21ea07d 100644
> --- a/src/xen/xen_hypervisor.c
> +++ b/src/xen/xen_hypervisor.c
> @@ -874,11 +874,6 @@ struct xenUnifiedDriver xenHypervisorDriver = {
>       .xenDomainSetSchedulerParameters = xenHypervisorSetSchedulerParameters,
>   };
>
> -#define virXenError(code, ...)                                             \
> -        if (in_init == 0)                                                  \
> -            virReportErrorHelper(VIR_FROM_XEN, code, __FILE__,             \
> -                                 __FUNCTION__, __LINE__, __VA_ARGS__)
> -

This error reporting macro is different from the other ones you nuked in 
the previous patches. This one does not print the error if the global 
static variable "in_init" in xen_hypervisor.c is non-zero.

Skimming through the code in xenHypervisorInit the program flow may 
leave the function without reseting in_init to 0 and thus suppressing 
error reporting from all parts of this file. Also in_init is compared 
only in that macro, so removing the use of that makes that variable 
pointless. I don't know if suppressing errors during init of xen driver 
is needed but either way, this part wil need tweaking.

NACK to changes in this file until it gets cleared.

>   /**
>    * xenHypervisorDoV0Op:
>    * @handle: the handle to the Xen hypervisor
> @@ -901,18 +896,18 @@ xenHypervisorDoV0Op(int handle, xen_op_v0 * op)
>       hc.arg[0] = (unsigned long) op;
>
>       if (lock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " locking");
> +        virReportError(VIR_ERR_XEN_CALL, " locking");
>           return -1;
>       }
>
>       ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc);
>       if (ret < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " ioctl %d",
> -                    xen_ioctl_hypercall_cmd);
> +        virReportError(VIR_ERR_XEN_CALL, " ioctl %d",
> +                       xen_ioctl_hypercall_cmd);
>       }
>
>       if (unlock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " releasing");
> +        virReportError(VIR_ERR_XEN_CALL, " releasing");
>           ret = -1;
>       }
>
> @@ -943,18 +938,18 @@ xenHypervisorDoV1Op(int handle, xen_op_v1* op)
>       hc.arg[0] = (unsigned long) op;
>
>       if (lock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " locking");
> +        virReportError(VIR_ERR_XEN_CALL, " locking");
>           return -1;
>       }
>
>       ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc);
>       if (ret < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " ioctl %d",
> -                    xen_ioctl_hypercall_cmd);
> +        virReportError(VIR_ERR_XEN_CALL, " ioctl %d",
> +                       xen_ioctl_hypercall_cmd);
>       }
>
>       if (unlock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " releasing");
> +        virReportError(VIR_ERR_XEN_CALL, " releasing");
>           ret = -1;
>       }
>
> @@ -986,18 +981,18 @@ xenHypervisorDoV2Sys(int handle, xen_op_v2_sys* op)
>       hc.arg[0] = (unsigned long) op;
>
>       if (lock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " locking");
> +        virReportError(VIR_ERR_XEN_CALL, " locking");
>           return -1;
>       }
>
>       ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc);
>       if (ret < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " sys ioctl %d",
> -                                            xen_ioctl_hypercall_cmd);
> +        virReportError(VIR_ERR_XEN_CALL, " sys ioctl %d",
> +                       xen_ioctl_hypercall_cmd);
>       }
>
>       if (unlock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " releasing");
> +        virReportError(VIR_ERR_XEN_CALL, " releasing");
>           ret = -1;
>       }
>
> @@ -1029,18 +1024,18 @@ xenHypervisorDoV2Dom(int handle, xen_op_v2_dom* op)
>       hc.arg[0] = (unsigned long) op;
>
>       if (lock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " locking");
> +        virReportError(VIR_ERR_XEN_CALL, " locking");
>           return -1;
>       }
>
>       ret = ioctl(handle, xen_ioctl_hypercall_cmd, (unsigned long) &hc);
>       if (ret < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " ioctl %d",
> -                    xen_ioctl_hypercall_cmd);
> +        virReportError(VIR_ERR_XEN_CALL, " ioctl %d",
> +                       xen_ioctl_hypercall_cmd);
>       }
>
>       if (unlock_pages(op, sizeof(dom0_op_t)) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " releasing");
> +        virReportError(VIR_ERR_XEN_CALL, " releasing");
>           ret = -1;
>       }
>
> @@ -1069,7 +1064,7 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids,
>
>       if (lock_pages(XEN_GETDOMAININFOLIST_DATA(dominfos),
>                 XEN_GETDOMAININFO_SIZE * maxids) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " locking");
> +        virReportError(VIR_ERR_XEN_CALL, " locking");
>           return -1;
>       }
>       if (hv_versions.hypervisor > 1) {
> @@ -1124,7 +1119,7 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids,
>       }
>       if (unlock_pages(XEN_GETDOMAININFOLIST_DATA(dominfos),
>                   XEN_GETDOMAININFO_SIZE * maxids) < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, " release");
> +        virReportError(VIR_ERR_XEN_CALL, " release");
>           ret = -1;
>       }
>       return ret;
> @@ -1161,20 +1156,20 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
>       xenUnifiedPrivatePtr priv;
>
>       if (domain->conn == NULL) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("domain or conn is NULL"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("domain or conn is NULL"));
>           return NULL;
>       }
>
>       priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>       if (priv->handle < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("priv->handle invalid"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("priv->handle invalid"));
>           return NULL;
>       }
>       if (domain->id < 0) {
> -        virXenError(VIR_ERR_OPERATION_INVALID,
> -                    "%s", _("domain is not running"));
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
>           return NULL;
>       }
>
> @@ -1184,8 +1179,8 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams)
>        * TODO: check on Xen 3.0.3
>        */
>       if (hv_versions.dom_interface < 5) {
> -        virXenError(VIR_ERR_NO_XEN, "%s",
> -                    _("unsupported in dom interface < 5"));
> +        virReportError(VIR_ERR_NO_XEN, "%s",
> +                       _("unsupported in dom interface < 5"));
>           return NULL;
>       }
>
> @@ -1242,20 +1237,20 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>       xenUnifiedPrivatePtr priv;
>
>       if (domain->conn == NULL) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("domain or conn is NULL"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("domain or conn is NULL"));
>           return -1;
>       }
>
>       priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>       if (priv->handle < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("priv->handle invalid"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("priv->handle invalid"));
>           return -1;
>       }
>       if (domain->id < 0) {
> -        virXenError(VIR_ERR_OPERATION_INVALID,
> -                    "%s", _("domain is not running"));
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
>           return -1;
>       }
>
> @@ -1265,8 +1260,8 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>        * TODO: check on Xen 3.0.3
>        */
>       if (hv_versions.dom_interface < 5) {
> -        virXenError(VIR_ERR_NO_XEN, "%s",
> -                    _("unsupported in dom interface < 5"));
> +        virReportError(VIR_ERR_NO_XEN, "%s",
> +                       _("unsupported in dom interface < 5"));
>           return -1;
>       }
>
> @@ -1284,8 +1279,8 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>           switch (op_sys.u.getschedulerid.sched_id){
>               case XEN_SCHEDULER_SEDF:
>                   if (*nparams < XEN_SCHED_SEDF_NPARAM) {
> -                    virXenError(VIR_ERR_INVALID_ARG,
> -                                "%s", _("Invalid parameter count"));
> +                    virReportError(VIR_ERR_INVALID_ARG,
> +                                   "%s", _("Invalid parameter count"));
>                       return -1;
>                   }
>
> @@ -1319,9 +1314,9 @@ xenHypervisorGetSchedulerParameters(virDomainPtr domain,
>                       *nparams = XEN_SCHED_CRED_NPARAM;
>                   break;
>               default:
> -                virXenError(VIR_ERR_INVALID_ARG,
> -                            _("Unknown scheduler %d"),
> -                            op_sys.u.getschedulerid.sched_id);
> +                virReportError(VIR_ERR_INVALID_ARG,
> +                               _("Unknown scheduler %d"),
> +                               op_sys.u.getschedulerid.sched_id);
>                   return -1;
>           }
>       }
> @@ -1348,8 +1343,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>       char buf[256];
>
>       if (domain->conn == NULL) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("domain or conn is NULL"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("domain or conn is NULL"));
>           return -1;
>       }
>
> @@ -1368,13 +1363,13 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>
>       priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
>       if (priv->handle < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("priv->handle invalid"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("priv->handle invalid"));
>           return -1;
>       }
>       if (domain->id < 0) {
> -        virXenError(VIR_ERR_OPERATION_INVALID,
> -                    "%s", _("domain is not running"));
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
>           return -1;
>       }
>
> @@ -1384,8 +1379,8 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>        * TODO: check on Xen 3.0.3
>        */
>       if (hv_versions.dom_interface < 5) {
> -        virXenError(VIR_ERR_NO_XEN, "%s",
> -                    _("unsupported in dom interface < 5"));
> +        virReportError(VIR_ERR_NO_XEN, "%s",
> +                       _("unsupported in dom interface < 5"));
>           return -1;
>       }
>
> @@ -1423,18 +1418,18 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>                   if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_WEIGHT)) {
>                       val = params[i].value.ui;
>                       if ((val < 1) || (val > USHRT_MAX)) {
> -                        virXenError(VIR_ERR_INVALID_ARG,
> -                                    _("Credit scheduler weight parameter (%d) "
> -                                      "is out of range (1-65535)"), val);
> +                        virReportError(VIR_ERR_INVALID_ARG,
> +                                       _("Credit scheduler weight parameter (%d) "
> +                                         "is out of range (1-65535)"), val);
>                           return -1;
>                       }
>                       op_dom.u.getschedinfo.u.credit.weight = val;
>                   } else if (STREQ(params[i].field, VIR_DOMAIN_SCHEDULER_CAP)) {
>                       val = params[i].value.ui;
>                       if (val >= USHRT_MAX) {
> -                        virXenError(VIR_ERR_INVALID_ARG,
> -                                    _("Credit scheduler cap parameter (%d) is "
> -                                      "out of range (0-65534)"), val);
> +                        virReportError(VIR_ERR_INVALID_ARG,
> +                                       _("Credit scheduler cap parameter (%d) is "
> +                                         "out of range (0-65534)"), val);
>                           return -1;
>                       }
>                       op_dom.u.getschedinfo.u.credit.cap = val;
> @@ -1447,9 +1442,9 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
>               break;
>           }
>           default:
> -            virXenError(VIR_ERR_INVALID_ARG,
> -                        _("Unknown scheduler %d"),
> -                        op_sys.u.getschedulerid.sched_id);
> +            virReportError(VIR_ERR_INVALID_ARG,
> +                           _("Unknown scheduler %d"),
> +                           op_sys.u.getschedulerid.sched_id);
>               return -1;
>           }
>       }
> @@ -1474,8 +1469,8 @@ xenHypervisorDomainBlockStats (virDomainPtr dom,
>       xenUnifiedUnlock(priv);
>       return ret;
>   #else
> -    virXenError(VIR_ERR_OPERATION_INVALID, "%s",
> -                _("block statistics not supported on this platform"));
> +    virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                   _("block statistics not supported on this platform"));
>       return -1;
>   #endif
>   }
> @@ -1499,20 +1494,20 @@ xenHypervisorDomainInterfaceStats (virDomainPtr dom,
>        * domain.
>        */
>       if (sscanf(path, "vif%d.%d", &rqdomid, &device) != 2) {
> -        virXenError(VIR_ERR_INVALID_ARG, "%s",
> -                    _("invalid path, should be vif<domid>.<n>."));
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("invalid path, should be vif<domid>.<n>."));
>           return -1;
>       }
>       if (rqdomid != dom->id) {
> -        virXenError(VIR_ERR_INVALID_ARG, "%s",
> -                    _("invalid path, vif<domid> should match this domain ID"));
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("invalid path, vif<domid> should match this domain ID"));
>           return -1;
>       }
>
>       return linuxDomainInterfaceStats(path, stats);
>   #else
> -    virXenError(VIR_ERR_OPERATION_INVALID, "%s",
> -                _("/proc/net/dev: Interface not found"));
> +    virReportError(VIR_ERR_OPERATION_INVALID, "%s",
> +                   _("/proc/net/dev: Interface not found"));
>       return -1;
>   #endif
>   }
> @@ -1748,7 +1743,7 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu,
>           xen_op_v2_dom op;
>
>           if (lock_pages(cpumap, maplen) < 0) {
> -            virXenError(VIR_ERR_XEN_CALL, " locking");
> +            virReportError(VIR_ERR_XEN_CALL, " locking");
>               return -1;
>           }
>           memset(&op, 0, sizeof(op));
> @@ -1783,7 +1778,7 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu,
>           VIR_FREE(new);
>
>           if (unlock_pages(cpumap, maplen) < 0) {
> -            virXenError(VIR_ERR_XEN_CALL, " release");
> +            virReportError(VIR_ERR_XEN_CALL, " release");
>               ret = -1;
>           }
>       } else {
> @@ -1880,7 +1875,7 @@ virXen_getvcpusinfo(int handle, int id, unsigned int vcpu, virVcpuInfoPtr ipt,
>           }
>           if ((cpumap != NULL) && (maplen > 0)) {
>               if (lock_pages(cpumap, maplen) < 0) {
> -                virXenError(VIR_ERR_XEN_CALL, " locking");
> +                virReportError(VIR_ERR_XEN_CALL, " locking");
>                   return -1;
>               }
>               memset(cpumap, 0, maplen);
> @@ -1898,7 +1893,7 @@ virXen_getvcpusinfo(int handle, int id, unsigned int vcpu, virVcpuInfoPtr ipt,
>               }
>               ret = xenHypervisorDoV2Dom(handle, &op);
>               if (unlock_pages(cpumap, maplen) < 0) {
> -                virXenError(VIR_ERR_XEN_CALL, " release");
> +                virReportError(VIR_ERR_XEN_CALL, " release");
>                   ret = -1;
>               }
>           }
> @@ -1998,7 +1993,7 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions)
>           char error[100];
>           regerror (errcode, &flags_hvm_rec, error, sizeof(error));
>           regfree (&flags_hvm_rec);
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s", error);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", error);
>           in_init = 0;
>           return -1;
>       }
> @@ -2008,7 +2003,7 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions)
>           regerror (errcode, &flags_pae_rec, error, sizeof(error));
>           regfree (&flags_pae_rec);
>           regfree (&flags_hvm_rec);
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s", error);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", error);
>           in_init = 0;
>           return -1;
>       }
> @@ -2019,7 +2014,7 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions)
>           regfree (&xen_cap_rec);
>           regfree (&flags_pae_rec);
>           regfree (&flags_hvm_rec);
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s", error);
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", error);
>           in_init = 0;
>           return -1;
>       }
> @@ -2079,8 +2074,8 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions)
>        */
>
>       hv_versions.hypervisor = -1;
> -    virXenError(VIR_ERR_XEN_CALL, " ioctl %lu",
> -                (unsigned long) IOCTL_PRIVCMD_HYPERCALL);
> +    virReportError(VIR_ERR_XEN_CALL, " ioctl %lu",
> +                   (unsigned long) IOCTL_PRIVCMD_HYPERCALL);
>       VIR_FORCE_CLOSE(fd);
>       in_init = 0;
>       return -1;
> @@ -2184,8 +2179,8 @@ xenHypervisorInit(struct xenHypervisorVersions *override_versions)
>
>       VIR_DEBUG("Failed to find any Xen hypervisor method");
>       hv_versions.hypervisor = -1;
> -    virXenError(VIR_ERR_XEN_CALL, " ioctl %lu",
> -                (unsigned long)IOCTL_PRIVCMD_HYPERCALL);
> +    virReportError(VIR_ERR_XEN_CALL, " ioctl %lu",
> +                   (unsigned long)IOCTL_PRIVCMD_HYPERCALL);
>       VIR_FORCE_CLOSE(fd);
>       in_init = 0;
>       VIR_FREE(ipt);
> @@ -2226,7 +2221,7 @@ xenHypervisorOpen(virConnectPtr conn,
>
>       ret = open(XEN_HYPERVISOR_SOCKET, O_RDWR);
>       if (ret < 0) {
> -        virXenError(VIR_ERR_NO_XEN, "%s", XEN_HYPERVISOR_SOCKET);
> +        virReportError(VIR_ERR_NO_XEN, "%s", XEN_HYPERVISOR_SOCKET);
>           return -1;
>       }
>
> @@ -2925,30 +2920,30 @@ xenHypervisorDomainGetOSType (virDomainPtr dom)
>
>       priv = (xenUnifiedPrivatePtr) dom->conn->privateData;
>       if (priv->handle < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("domain shut off or invalid"));
> +        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) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("unsupported in dom interface < 4"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("unsupported in dom interface < 4"));
>           return NULL;
>       }
>
>       XEN_GETDOMAININFO_CLEAR(dominfo);
>
>       if (virXen_getdomaininfo(priv->handle, dom->id, &dominfo) < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("cannot get domain details"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("cannot get domain details"));
>           return NULL;
>       }
>
>       if (XEN_GETDOMAININFO_DOMAIN(dominfo) != dom->id) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("cannot get domain details"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("cannot get domain details"));
>           return NULL;
>       }
>
> @@ -3347,21 +3342,21 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
>       xenUnifiedPrivatePtr priv;
>
>       if (conn == NULL) {
> -        virXenError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument"));
> +        virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument"));
>           return -1;
>       }
>
>       priv = conn->privateData;
>
>       if (priv->nbNodeCells < 0) {
> -        virXenError(VIR_ERR_XEN_CALL, "%s",
> -                    _("cannot determine actual number of cells"));
> +        virReportError(VIR_ERR_XEN_CALL, "%s",
> +                       _("cannot determine actual number of cells"));
>           return -1;
>       }
>
>       if ((maxCells < 1) || (startCell >= priv->nbNodeCells)) {
> -        virXenError(VIR_ERR_INVALID_ARG, "%s",
> -                    _("invalid argument"));
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("invalid argument"));
>           return -1;
>       }
>
> @@ -3369,14 +3364,14 @@ xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *free
>        * Support only hv_versions.sys_interface >=4
>        */
>       if (hv_versions.sys_interface < SYS_IFACE_MIN_VERS_NUMA) {
> -        virXenError(VIR_ERR_XEN_CALL, "%s",
> -                    _("unsupported in sys interface < 4"));
> +        virReportError(VIR_ERR_XEN_CALL, "%s",
> +                       _("unsupported in sys interface < 4"));
>           return -1;
>       }
>
>       if (priv->handle < 0) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("priv->handle invalid"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("priv->handle invalid"));
>           return -1;
>       }
>
> @@ -3617,13 +3612,13 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>       if (priv->handle < 0 || (domain->id < 0) ||
>           (info == NULL) || (maxinfo < 1) ||
>           (sizeof(cpumap_t) & 7)) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("domain shut off or invalid"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("domain shut off or invalid"));
>           return -1;
>       }
>       if ((cpumaps != NULL) && (maplen < 1)) {
> -        virXenError(VIR_ERR_INVALID_ARG, "%s",
> -                    _("invalid argument"));
> +        virReportError(VIR_ERR_INVALID_ARG, "%s",
> +                       _("invalid argument"));
>           return -1;
>       }
>       /* first get the number of virtual CPUs in this domain */
> @@ -3632,8 +3627,8 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>                                  &dominfo);
>
>       if ((ret < 0) || (XEN_GETDOMAININFO_DOMAIN(dominfo) != domain->id)) {
> -        virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                    _("cannot get domain details"));
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("cannot get domain details"));
>           return -1;
>       }
>       nbinfo = XEN_GETDOMAININFO_CPUCOUNT(dominfo) + 1;
> @@ -3649,16 +3644,16 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo,
>                                         (unsigned char *)VIR_GET_CPUMAP(cpumaps, maplen, i),
>                                         maplen);
>               if (ret < 0) {
> -                virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                            _("cannot get VCPUs info"));
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("cannot get VCPUs info"));
>                   return -1;
>               }
>           } else {
>               ret = virXen_getvcpusinfo(priv->handle, domain->id, i,
>                                         ipt, NULL, 0);
>               if (ret < 0) {
> -                virXenError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                            _("cannot get VCPUs info"));
> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                               _("cannot get VCPUs info"));
>                   return -1;
>               }
>           }


> diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c

ACK to changes in this file.

> diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c

ACK to changes in this file.

> diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c

ACK to changes in this file.

> diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c

ACK to changes in this file.

Apart from the problematic macro in src/xen/xen_hypervisor.c and some 
pre-existing virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__) this 
patch looks fine, but I have to give:

NACK until src/xen/xen_hypervisor.c gets clarified.

Peter

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list