[libvirt] [PATCH 00/11] Revisit xen driver Coverity cleanup changes

This is a rework of a couple of patches. Initially the changes were to just remove some Coverity warnings; however, it was pointed out that each of the xen drivers could use some cleanup since they were duplicating checks in libvirt.c, see: https://www.redhat.com/archives/libvir-list/2013-January/msg01038.html https://www.redhat.com/archives/libvir-list/2013-January/msg01057.html While I was at it - I cleaned up some of the function headers. Additionally, a revisit of a change to add Coverity false positive tags to the v0 & v1 xen_hypervisor cpumap manipulation, see: https://www.redhat.com/archives/libvir-list/2013-January/msg01527.html and followups. John Ferlan (11): xs: Remove redundant validity checks xs: Code cleanup inotify: Code cleanup xm: Remove redundant validity checks xm: Clean up xend: Remove redundant validity checks xend: Fix a memory leak found by Coverity xend: Clean up hypervisor: Remove redundant validity checks hypervisor: Clean up hypervisor: Revisit Coverity issues regarding cpumap src/xen/xen_hypervisor.c | 220 ++++++++-------------------------- src/xen/xen_inotify.c | 48 ++++---- src/xen/xend_internal.c | 303 ++++++++++++++--------------------------------- src/xen/xm_internal.c | 195 ++++++++++++++---------------- src/xen/xs_internal.c | 239 +++++++++++-------------------------- 5 files changed, 325 insertions(+), 680 deletions(-) -- 1.7.11.7

Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xs_internal.c | 139 ++++++++++---------------------------------------- 1 file changed, 28 insertions(+), 111 deletions(-) diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 9308522..573c0c6 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -1,7 +1,7 @@ /* * xs_internal.c: access to Xen Store * - * Copyright (C) 2006, 2009-2012 Red Hat, Inc. + * Copyright (C) 2006, 2009-2013 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 @@ -86,12 +86,8 @@ static char ** virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb) { - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return NULL; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL || path == NULL || nb == NULL) return NULL; @@ -113,12 +109,8 @@ virDomainDoStoreQuery(virConnectPtr conn, int domid, const char *path) { char s[256]; unsigned int len = 0; - xenUnifiedPrivatePtr priv; - - if (!conn) - return NULL; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -143,16 +135,11 @@ virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) { char s[256]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = + (xenUnifiedPrivatePtr) domain->conn->privateData; int ret = -1; - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xshandle == NULL) - return -1; - if (domain->conn->flags & VIR_CONNECT_RO) + if (priv->xshandle == NULL || domain->conn->flags & VIR_CONNECT_RO) return -1; snprintf(s, 255, "/local/domain/%d/%s", domain->id, path); @@ -178,12 +165,9 @@ virDomainGetVM(virDomainPtr domain) char *vm; char query[200]; unsigned int len; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return NULL; + xenUnifiedPrivatePtr priv = + (xenUnifiedPrivatePtr) domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -212,12 +196,8 @@ virDomainGetVMInfo(virDomainPtr domain, const char *vm, const char *name) char s[256]; char *ret = NULL; unsigned int len = 0; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return NULL; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -326,14 +306,7 @@ xenStoreOpen(virConnectPtr conn, int xenStoreClose(virConnectPtr conn) { - xenUnifiedPrivatePtr priv; - - if (conn == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (xenStoreRemoveWatch(conn, "@introduceDomain", "introduceDomain") < 0) { VIR_DEBUG("Warning, could not remove @introduceDomain watch"); @@ -377,21 +350,9 @@ xenStoreGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) char *tmp, **tmp2; unsigned int nb_vcpus; char request[200]; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return -1; - - if ((domain == NULL) || (domain->conn == NULL) || (info == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xshandle == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->id == -1) + if (priv->xshandle == NULL || domain->id == -1) return -1; tmp = virDomainDoStoreQuery(domain->conn, domain->id, "running"); @@ -482,8 +443,7 @@ xenStoreDomainSetMemory(virDomainPtr domain, unsigned long memory) int ret; char value[20]; - if ((domain == NULL) || (domain->conn == NULL) || - (memory < 1024 * MIN_XEN_GUEST_SIZE)) { + if (memory < 1024 * MIN_XEN_GUEST_SIZE) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -512,14 +472,11 @@ xenStoreDomainGetMaxMemory(virDomainPtr domain) { char *tmp; unsigned long long ret = 0; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return ret; if (domain->id == -1) return 0; - priv = domain->conn->privateData; xenUnifiedLock(priv); tmp = virDomainDoStoreQuery(domain->conn, domain->id, "memory/target"); if (tmp != NULL) { @@ -545,14 +502,8 @@ xenStoreNumOfDomains(virConnectPtr conn) char **idlist = NULL, *endptr; int i, ret = -1, realnum = 0; long id; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; @@ -632,16 +583,9 @@ out: int xenStoreListDomains(virConnectPtr conn, int *ids, int maxids) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; int ret; - if ((conn == NULL) || (ids == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - - priv = (xenUnifiedPrivatePtr) conn->privateData; - xenUnifiedLock(priv); ret = xenStoreDoListDomains(conn, priv, ids, maxids); xenUnifiedUnlock(priv); @@ -668,14 +612,8 @@ xenStoreLookupByName(virConnectPtr conn, const char *name) char prop[200], *tmp; int found = 0; struct xend_domain *xenddomain = NULL; - xenUnifiedPrivatePtr priv; - - if ((conn == NULL) || (name == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -732,19 +670,14 @@ int xenStoreDomainShutdown(virDomainPtr domain) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } if (domain->id == -1 || domain->id == 0) return -1; /* * this is very hackish, the domU kernel probes for a special * node in the xenstore and launch the shutdown command if found. */ - priv = domain->conn->privateData; xenUnifiedLock(priv); ret = virDomainDoStoreWrite(domain, "control/shutdown", "poweroff"); xenUnifiedUnlock(priv); @@ -766,21 +699,17 @@ int xenStoreDomainReboot(virDomainPtr domain, unsigned int flags) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virCheckFlags(0, -1); - if ((domain == NULL) || (domain->conn == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } if (domain->id == -1 || domain->id == 0) return -1; /* * this is very hackish, the domU kernel probes for a special * node in the xenstore and launch the shutdown command if found. */ - priv = domain->conn->privateData; + xenUnifiedLock(priv); ret = virDomainDoStoreWrite(domain, "control/shutdown", "reboot"); xenUnifiedUnlock(priv); @@ -800,14 +729,10 @@ static char * xenStoreDomainGetOSType(virDomainPtr domain) { char *vm, *str = NULL; - if ((domain == NULL) || (domain->conn == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } - vm = virDomainGetVM(domain); if (vm) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = + (xenUnifiedPrivatePtr) domain->conn->privateData; xenUnifiedLock(priv); str = virDomainGetVMInfo(domain, vm, "image/ostype"); xenUnifiedUnlock(priv); @@ -904,12 +829,10 @@ xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) { char dir[80], path[128], **list = NULL, *val = NULL; unsigned int len, i, num; char *ret = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (id < 0) return NULL; - - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; if (mac == NULL) @@ -962,12 +885,10 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) { char dir[80], path[128], **list = NULL, *val = NULL; unsigned int devlen, len, i, num; char *ret = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (id < 0) return NULL; - - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; if (dev == NULL) @@ -1046,12 +967,10 @@ xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) char dir[80], path[128], **list = NULL, *val = NULL; unsigned int len, i, num; char *ret = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (id < 0) return NULL; - - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; if (bdf == NULL) @@ -1087,10 +1006,9 @@ xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) char *xenStoreDomainGetName(virConnectPtr conn, int id) { char prop[200]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; unsigned int len; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -1107,12 +1025,11 @@ int xenStoreDomainGetUUID(virConnectPtr conn, int id, unsigned char *uuid) { char prop[200]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; unsigned int len; char *uuidstr; int ret = 0; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return -1; @@ -1335,7 +1252,7 @@ int xenStoreDomainIntroduced(virConnectPtr conn, int *new_domids; int nread; - xenUnifiedPrivatePtr priv = opaque; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) opaque; retry: new_domain_cnt = xenStoreNumOfDomains(conn); -- 1.7.11.7

On 2013年01月31日 02:51, John Ferlan wrote:
Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xs_internal.c | 139 ++++++++++---------------------------------------- 1 file changed, 28 insertions(+), 111 deletions(-)
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 9308522..573c0c6 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -1,7 +1,7 @@ /* * xs_internal.c: access to Xen Store * - * Copyright (C) 2006, 2009-2012 Red Hat, Inc. + * Copyright (C) 2006, 2009-2013 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 @@ -86,12 +86,8 @@ static char ** virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb) { - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return NULL; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL || path == NULL || nb == NULL) return NULL;
@@ -113,12 +109,8 @@ virDomainDoStoreQuery(virConnectPtr conn, int domid, const char *path) { char s[256]; unsigned int len = 0; - xenUnifiedPrivatePtr priv; - - if (!conn) - return NULL; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->xshandle == NULL) return NULL;
@@ -143,16 +135,11 @@ virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) { char s[256]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = + (xenUnifiedPrivatePtr) domain->conn->privateData; int ret = -1;
- if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xshandle == NULL) - return -1; - if (domain->conn->flags& VIR_CONNECT_RO) + if (priv->xshandle == NULL || domain->conn->flags& VIR_CONNECT_RO) return -1;
snprintf(s, 255, "/local/domain/%d/%s", domain->id, path); @@ -178,12 +165,9 @@ virDomainGetVM(virDomainPtr domain) char *vm; char query[200]; unsigned int len; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECTED_DOMAIN(domain)) - return NULL; + xenUnifiedPrivatePtr priv = + (xenUnifiedPrivatePtr) domain->conn->privateData;
ACK with changing lines like this into 1 line.

Housekeeping kind of stuff. --- src/xen/xs_internal.c | 108 +++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 63 deletions(-) diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 573c0c6..f823dfc 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -83,14 +83,10 @@ struct xenUnifiedDriver xenStoreDriver = { * Returns a string which must be freed by the caller or NULL in case of error */ static char ** -virConnectDoStoreList(virConnectPtr conn, const char *path, - unsigned int *nb) +virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->xshandle == NULL || path == NULL || nb == NULL) - return NULL; - return xs_directory(priv->xshandle, 0, path, nb); } @@ -131,12 +127,10 @@ virDomainDoStoreQuery(virConnectPtr conn, int domid, const char *path) * Returns 0 in case of success, -1 in case of failure */ static int -virDomainDoStoreWrite(virDomainPtr domain, const char *path, - const char *value) +virDomainDoStoreWrite(virDomainPtr domain, const char *path, const char *value) { char s[256]; - xenUnifiedPrivatePtr priv = - (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; int ret = -1; if (priv->xshandle == NULL || domain->conn->flags & VIR_CONNECT_RO) @@ -165,8 +159,7 @@ virDomainGetVM(virDomainPtr domain) char *vm; char query[200]; unsigned int len; - xenUnifiedPrivatePtr priv = - (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (priv->xshandle == NULL) return NULL; @@ -402,9 +395,7 @@ xenStoreGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) * Returns 0 in case of success, -1 in case of error. */ int -xenStoreDomainGetState(virDomainPtr domain, - int *state, - int *reason, +xenStoreDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { char *running; @@ -540,7 +531,8 @@ out: * Returns the number of domain found or -1 in case of error */ static int -xenStoreDoListDomains(virConnectPtr conn, xenUnifiedPrivatePtr priv, int *ids, int maxids) +xenStoreDoListDomains(virConnectPtr conn, xenUnifiedPrivatePtr priv, int *ids, + int maxids) { char **idlist = NULL, *endptr; unsigned int num, i; @@ -726,7 +718,8 @@ xenStoreDomainReboot(virDomainPtr domain, unsigned int flags) * freed by the caller. */ static char * -xenStoreDomainGetOSType(virDomainPtr domain) { +xenStoreDomainGetOSType(virDomainPtr domain) +{ char *vm, *str = NULL; vm = virDomainGetVM(domain); @@ -755,7 +748,9 @@ xenStoreDomainGetOSType(virDomainPtr domain) { * * Returns the port number, -1 in case of error */ -int xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) { +int +xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) +{ char *tmp; int ret = -1; @@ -785,7 +780,9 @@ int xenStoreDomainGetVNCPort(virConnectPtr conn, int domid) { * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -char * xenStoreDomainGetConsolePath(virConnectPtr conn, int domid) { +char * +xenStoreDomainGetConsolePath(virConnectPtr conn, int domid) +{ return virDomainDoStoreQuery(conn, domid, "console/tty"); } @@ -804,7 +801,9 @@ char * xenStoreDomainGetConsolePath(virConnectPtr conn, int domid) { * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -char * xenStoreDomainGetSerialConsolePath(virConnectPtr conn, int domid) { +char * +xenStoreDomainGetSerialConsolePath(virConnectPtr conn, int domid) +{ return virDomainDoStoreQuery(conn, domid, "serial/0/tty"); } @@ -825,17 +824,14 @@ char * xenStoreDomainGetSerialConsolePath(virConnectPtr conn, int domid) { * freed by the caller. */ char * -xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) { +xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) +{ char dir[80], path[128], **list = NULL, *val = NULL; unsigned int len, i, num; char *ret = NULL; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (id < 0) - return NULL; - if (priv->xshandle == NULL) - return NULL; - if (mac == NULL) + if (id < 0 || priv->xshandle == NULL || mac == NULL) return NULL; snprintf(dir, sizeof(dir), "/local/domain/0/backend/vif/%d", id); @@ -881,17 +877,14 @@ xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) { * freed by the caller. */ char * -xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) { +xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) +{ char dir[80], path[128], **list = NULL, *val = NULL; unsigned int devlen, len, i, num; char *ret = NULL; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (id < 0) - return NULL; - if (priv->xshandle == NULL) - return NULL; - if (dev == NULL) + if (id < 0 || priv->xshandle == NULL || dev == NULL) return NULL; devlen = strlen(dev); if (devlen <= 0) @@ -969,11 +962,7 @@ xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) char *ret = NULL; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (id < 0) - return NULL; - if (priv->xshandle == NULL) - return NULL; - if (bdf == NULL) + if (id < 0 || priv->xshandle == NULL || bdf == NULL) return NULL; snprintf(dir, sizeof(dir), "/local/domain/0/backend/pci/%d", id); @@ -1003,8 +992,9 @@ xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -char *xenStoreDomainGetName(virConnectPtr conn, - int id) { +char * +xenStoreDomainGetName(virConnectPtr conn, int id) +{ char prop[200]; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; unsigned int len; @@ -1021,9 +1011,9 @@ char *xenStoreDomainGetName(virConnectPtr conn, * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -int xenStoreDomainGetUUID(virConnectPtr conn, - int id, - unsigned char *uuid) { +int +xenStoreDomainGetUUID(virConnectPtr conn, int id, unsigned char *uuid) +{ char prop[200]; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; unsigned int len; @@ -1066,11 +1056,9 @@ xenStoreWatchListFree(xenStoreWatchListPtr list) * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -int xenStoreAddWatch(virConnectPtr conn, - const char *path, - const char *token, - xenStoreWatchCallback cb, - void *opaque) +int +xenStoreAddWatch(virConnectPtr conn, const char *path, const char *token, + xenStoreWatchCallback cb, void *opaque) { xenStoreWatchPtr watch = NULL; int n; @@ -1133,9 +1121,8 @@ int xenStoreAddWatch(virConnectPtr conn, * The caller must hold the lock on the privateData * associated with the 'conn' parameter. */ -int xenStoreRemoveWatch(virConnectPtr conn, - const char *path, - const char *token) +int +xenStoreRemoveWatch(virConnectPtr conn, const char *path, const char *token) { int i; xenStoreWatchListPtr list; @@ -1182,8 +1169,7 @@ int xenStoreRemoveWatch(virConnectPtr conn, } static xenStoreWatchPtr -xenStoreFindWatch(xenStoreWatchListPtr list, - const char *path, +xenStoreFindWatch(xenStoreWatchListPtr list, const char *path, const char *token) { int i; @@ -1196,10 +1182,8 @@ xenStoreFindWatch(xenStoreWatchListPtr list, } static void -xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED, - int fd ATTRIBUTE_UNUSED, - int events, - void *data) +xenStoreWatchEvent(int watch ATTRIBUTE_UNUSED, int fd ATTRIBUTE_UNUSED, + int events, void *data) { char **event; char *path; @@ -1242,10 +1226,9 @@ cleanup: * * The lock on 'priv' is held when calling this */ -int xenStoreDomainIntroduced(virConnectPtr conn, - const char *path ATTRIBUTE_UNUSED, - const char *token ATTRIBUTE_UNUSED, - void *opaque) +int +xenStoreDomainIntroduced(virConnectPtr conn, const char *path ATTRIBUTE_UNUSED, + const char *token ATTRIBUTE_UNUSED, void *opaque) { int i, j, found, missing = 0, retries = 20; int new_domain_cnt; @@ -1323,10 +1306,9 @@ retry: * * The lock on 'priv' is held when calling this */ -int xenStoreDomainReleased(virConnectPtr conn, - const char *path ATTRIBUTE_UNUSED, - const char *token ATTRIBUTE_UNUSED, - void *opaque) +int +xenStoreDomainReleased(virConnectPtr conn, const char *path ATTRIBUTE_UNUSED, + const char *token ATTRIBUTE_UNUSED, void *opaque) { int i, j, found, removed, retries = 20; int new_domain_cnt; -- 1.7.11.7

On 2013年01月31日 02:51, John Ferlan wrote:
Housekeeping kind of stuff. --- src/xen/xs_internal.c | 108 +++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 63 deletions(-)
diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 573c0c6..f823dfc 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -83,14 +83,10 @@ struct xenUnifiedDriver xenStoreDriver = { * Returns a string which must be freed by the caller or NULL in case of error */ static char ** -virConnectDoStoreList(virConnectPtr conn, const char *path, - unsigned int *nb) +virConnectDoStoreList(virConnectPtr conn, const char *path, unsigned int *nb) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- if (priv->xshandle == NULL || path == NULL || nb == NULL) - return NULL; -
virConnectDoStoreList is internal helper, and these arguments are guaranteed not NULL. So ACK.

Clean up some function headers --- src/xen/xen_inotify.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index 2043c74..94803b2 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -4,7 +4,7 @@ * /etc/xen * /var/lib/xend/domains * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -49,9 +49,9 @@ struct xenUnifiedDriver xenInotifyDriver = { }; static int -xenInotifyXenCacheLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyXenCacheLookup(virConnectPtr conn, const char *filename, + char **name, unsigned char *uuid) +{ xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; @@ -73,7 +73,8 @@ xenInotifyXenCacheLookup(virConnectPtr conn, static int xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, - char **name, unsigned char *uuid) { + char **name, unsigned char *uuid) +{ int i; virDomainPtr dom; const char *uuid_str; @@ -129,9 +130,9 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, } static int -xenInotifyDomainLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyDomainLookup(virConnectPtr conn, const char *filename, char **name, + unsigned char *uuid) +{ xenUnifiedPrivatePtr priv = conn->privateData; if (priv->useXenConfigCache) return xenInotifyXenCacheLookup(conn, filename, name, uuid); @@ -140,9 +141,9 @@ xenInotifyDomainLookup(virConnectPtr conn, } static virDomainEventPtr -xenInotifyDomainEventFromFile(virConnectPtr conn, - const char *filename, - int type, int detail) { +xenInotifyDomainEventFromFile(virConnectPtr conn, const char *filename, + int type, int detail) +{ virDomainEventPtr event; char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -156,8 +157,8 @@ xenInotifyDomainEventFromFile(virConnectPtr conn, } static int -xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; const char *uuidstr = fname + strlen(XEND_DOMAINS_DIR) + 1; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -193,8 +194,8 @@ xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, } static int -xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, const char *fname) +{ char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; xenUnifiedPrivatePtr priv = conn->privateData; @@ -217,8 +218,8 @@ xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, } static int -xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheRemoveFile(conn, fname) : @@ -226,8 +227,8 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, } static int -xenInotifyAddDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheAddFile(conn, fname) : @@ -235,10 +236,8 @@ xenInotifyAddDomainConfigInfo(virConnectPtr conn, } static void -xenInotifyEvent(int watch ATTRIBUTE_UNUSED, - int fd, - int events ATTRIBUTE_UNUSED, - void *data) +xenInotifyEvent(int watch ATTRIBUTE_UNUSED, int fd, + int events ATTRIBUTE_UNUSED, void *data) { char buf[1024]; char fname[1024]; @@ -341,8 +340,7 @@ cleanup: * Returns 0 or -1 in case of error. */ virDrvOpenStatus -xenInotifyOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenInotifyOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { DIR *dh; -- 1.7.11.7

On 2013年01月31日 02:51, John Ferlan wrote:
Clean up some function headers --- src/xen/xen_inotify.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index 2043c74..94803b2 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -4,7 +4,7 @@ * /etc/xen * /var/lib/xend/domains * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -49,9 +49,9 @@ struct xenUnifiedDriver xenInotifyDriver = { };
static int -xenInotifyXenCacheLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyXenCacheLookup(virConnectPtr conn, const char *filename, + char **name, unsigned char *uuid) +{ xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry;
@@ -73,7 +73,8 @@ xenInotifyXenCacheLookup(virConnectPtr conn,
static int xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, - char **name, unsigned char *uuid) { + char **name, unsigned char *uuid) +{ int i; virDomainPtr dom; const char *uuid_str; @@ -129,9 +130,9 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, }
static int -xenInotifyDomainLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyDomainLookup(virConnectPtr conn, const char *filename, char **name, + unsigned char *uuid)
Thing like this is personal habit from my p.o.v, someone likes even write it like below: xenInotifyDomainLookup(virConnectPtr conn, const char *filename, char **name, unsigned char *uuid) { }
+{ xenUnifiedPrivatePtr priv = conn->privateData; if (priv->useXenConfigCache) return xenInotifyXenCacheLookup(conn, filename, name, uuid); @@ -140,9 +141,9 @@ xenInotifyDomainLookup(virConnectPtr conn, }
static virDomainEventPtr -xenInotifyDomainEventFromFile(virConnectPtr conn, - const char *filename, - int type, int detail) { +xenInotifyDomainEventFromFile(virConnectPtr conn, const char *filename, + int type, int detail) +{ virDomainEventPtr event; char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -156,8 +157,8 @@ xenInotifyDomainEventFromFile(virConnectPtr conn, }
static int -xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; const char *uuidstr = fname + strlen(XEND_DOMAINS_DIR) + 1; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -193,8 +194,8 @@ xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, }
static int -xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, const char *fname) +{ char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; xenUnifiedPrivatePtr priv = conn->privateData; @@ -217,8 +218,8 @@ xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, }
static int -xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheRemoveFile(conn, fname) : @@ -226,8 +227,8 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, }
static int -xenInotifyAddDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheAddFile(conn, fname) : @@ -235,10 +236,8 @@ xenInotifyAddDomainConfigInfo(virConnectPtr conn, }
static void -xenInotifyEvent(int watch ATTRIBUTE_UNUSED, - int fd, - int events ATTRIBUTE_UNUSED, - void *data) +xenInotifyEvent(int watch ATTRIBUTE_UNUSED, int fd, + int events ATTRIBUTE_UNUSED, void *data) { char buf[1024]; char fname[1024]; @@ -341,8 +340,7 @@ cleanup: * Returns 0 or -1 in case of error. */ virDrvOpenStatus -xenInotifyOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenInotifyOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { DIR *dh;

Osier Yang wrote:
On 2013年01月31日 02:51, John Ferlan wrote:
Clean up some function headers --- src/xen/xen_inotify.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index 2043c74..94803b2 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -4,7 +4,7 @@ * /etc/xen * /var/lib/xend/domains * - * Copyright (C) 2010-2011 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2008 VirtualIron * * This library is free software; you can redistribute it and/or @@ -49,9 +49,9 @@ struct xenUnifiedDriver xenInotifyDriver = { };
static int -xenInotifyXenCacheLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyXenCacheLookup(virConnectPtr conn, const char *filename, + char **name, unsigned char *uuid) +{ xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry;
@@ -73,7 +73,8 @@ xenInotifyXenCacheLookup(virConnectPtr conn,
static int xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, - char **name, unsigned char *uuid) { + char **name, unsigned char *uuid) +{ int i; virDomainPtr dom; const char *uuid_str; @@ -129,9 +130,9 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, const char *filename, }
static int -xenInotifyDomainLookup(virConnectPtr conn, - const char *filename, - char **name, unsigned char *uuid) { +xenInotifyDomainLookup(virConnectPtr conn, const char *filename, char **name, + unsigned char *uuid)
Thing like this is personal habit from my p.o.v, someone likes even write it like below:
xenInotifyDomainLookup(virConnectPtr conn, const char *filename, char **name, unsigned char *uuid) { }
Yep, there doesn't seem to be a consistent pattern throughout the sources. One thing that is consistent is '{' starting on a new line, so ACK to those changes. Regards, Jim
+{ xenUnifiedPrivatePtr priv = conn->privateData; if (priv->useXenConfigCache) return xenInotifyXenCacheLookup(conn, filename, name, uuid); @@ -140,9 +141,9 @@ xenInotifyDomainLookup(virConnectPtr conn, }
static virDomainEventPtr -xenInotifyDomainEventFromFile(virConnectPtr conn, - const char *filename, - int type, int detail) { +xenInotifyDomainEventFromFile(virConnectPtr conn, const char *filename, + int type, int detail) +{ virDomainEventPtr event; char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -156,8 +157,8 @@ xenInotifyDomainEventFromFile(virConnectPtr conn, }
static int -xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; const char *uuidstr = fname + strlen(XEND_DOMAINS_DIR) + 1; unsigned char uuid[VIR_UUID_BUFLEN]; @@ -193,8 +194,8 @@ xenInotifyXendDomainsDirRemoveEntry(virConnectPtr conn, }
static int -xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, - const char *fname) { +xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, const char *fname) +{ char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; xenUnifiedPrivatePtr priv = conn->privateData; @@ -217,8 +218,8 @@ xenInotifyXendDomainsDirAddEntry(virConnectPtr conn, }
static int -xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheRemoveFile(conn, fname) : @@ -226,8 +227,8 @@ xenInotifyRemoveDomainConfigInfo(virConnectPtr conn, }
static int -xenInotifyAddDomainConfigInfo(virConnectPtr conn, - const char *fname) { +xenInotifyAddDomainConfigInfo(virConnectPtr conn, const char *fname) +{ xenUnifiedPrivatePtr priv = conn->privateData; return priv->useXenConfigCache ? xenXMConfigCacheAddFile(conn, fname) : @@ -235,10 +236,8 @@ xenInotifyAddDomainConfigInfo(virConnectPtr conn, }
static void -xenInotifyEvent(int watch ATTRIBUTE_UNUSED, - int fd, - int events ATTRIBUTE_UNUSED, - void *data) +xenInotifyEvent(int watch ATTRIBUTE_UNUSED, int fd, + int events ATTRIBUTE_UNUSED, void *data) { char buf[1024]; char fname[1024]; @@ -341,8 +340,7 @@ cleanup: * Returns 0 or -1 in case of error. */ virDrvOpenStatus -xenInotifyOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenInotifyOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { DIR *dh;
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xm_internal.c | 76 ++++++++++++--------------------------------------- 1 file changed, 17 insertions(+), 59 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 003e8f7..1c1db54 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -481,7 +481,8 @@ int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -527,7 +528,7 @@ char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) /* Flags checked by virDomainDefFormat */ - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } @@ -560,7 +561,7 @@ int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) { xenXMConfCachePtr entry; int ret = -1; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -605,7 +606,7 @@ int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { xenXMConfCachePtr entry; int ret = -1; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -648,7 +649,7 @@ unsigned long long xenXMDomainGetMaxMemory(virDomainPtr domain) { xenXMConfCachePtr entry; unsigned long long ret = 0; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return 0; } @@ -696,7 +697,7 @@ xenXMDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -780,7 +781,7 @@ xenXMDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -829,8 +830,7 @@ int xenXMDomainPinVcpu(virDomainPtr domain, xenXMConfCachePtr entry; int ret = -1; - if (domain == NULL || domain->conn == NULL || domain->name == NULL - || cpumap == NULL || maplen < 1 || maplen > (int)sizeof(cpumap_t)) { + if (domain->name == NULL || maplen > (int)sizeof(cpumap_t)) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -876,21 +876,11 @@ int xenXMDomainPinVcpu(virDomainPtr domain, * Find an inactive domain based on its name */ virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; virDomainPtr ret = NULL; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return NULL; - } - if (domname == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } - - priv = conn->privateData; xenUnifiedLock(priv); if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) @@ -933,20 +923,10 @@ static int xenXMDomainSearchForUUID(const void *payload, const void *name ATTRIB */ virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; virDomainPtr ret = NULL; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return NULL; - } - if (uuid == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } - - priv = conn->privateData; xenUnifiedLock(priv); if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) @@ -974,12 +954,10 @@ cleanup: int xenXMDomainCreate(virDomainPtr domain) { char *sexpr; int ret = -1; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv= (xenUnifiedPrivatePtr) domain->conn->privateData; const char *filename; xenXMConfCachePtr entry; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id != -1) return -1; @@ -1036,14 +1014,6 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) xenXMConfCachePtr entry = NULL; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return NULL; - } - if (xml == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } if (conn->flags & VIR_CONNECT_RO) return NULL; @@ -1177,7 +1147,7 @@ int xenXMDomainUndefine(virDomainPtr domain) { xenXMConfCachePtr entry; int ret = -1; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1249,16 +1219,10 @@ static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, * are currently running */ int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = conn->privateData; struct xenXMListIteratorContext ctx; int i, ret = -1; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - priv = conn->privateData; xenUnifiedLock(priv); if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) @@ -1295,15 +1259,9 @@ cleanup: * based on number running */ int xenXMNumOfDefinedDomains(virConnectPtr conn) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = conn->privateData; int ret = -1; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - priv = conn->privateData; xenUnifiedLock(priv); if (!xenInotifyActive(conn) && xenXMConfigCacheRefresh(conn) < 0) @@ -1343,7 +1301,7 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1440,7 +1398,7 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if ((!domain) || (!domain->conn) || (!domain->name) || (!xml)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } -- 1.7.11.7

On 2013年01月31日 02:51, John Ferlan wrote:
Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xm_internal.c | 76 ++++++++++++--------------------------------------- 1 file changed, 17 insertions(+), 59 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 003e8f7..1c1db54 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -481,7 +481,8 @@ int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + + if (domain->name == NULL) {
IMHO this checking can also be removed. Domain name is mandatory when parsing. So I don't think it can be NULL.

Clean up some function headers. --- src/xen/xm_internal.c | 119 +++++++++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 44 deletions(-) diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 1c1db54..912de3f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -124,7 +124,10 @@ struct xenXMConfigReaperData { }; /* Remove any configs which were not refreshed recently */ -static int xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUSED, const void *data) { +static int +xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUSED, + const void *data) +{ const struct xenXMConfigReaperData *args = data; xenXMConfCachePtr entry = (xenXMConfCachePtr)payload; @@ -143,7 +146,8 @@ static int xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUS static virDomainDefPtr -xenXMConfigReadFile(virConnectPtr conn, const char *filename) { +xenXMConfigReadFile(virConnectPtr conn, const char *filename) +{ virConfPtr conf; virDomainDefPtr def; xenUnifiedPrivatePtr priv = conn->privateData; @@ -158,7 +162,9 @@ xenXMConfigReadFile(virConnectPtr conn, const char *filename) { } static int -xenXMConfigSaveFile(virConnectPtr conn, const char *filename, virDomainDefPtr def) { +xenXMConfigSaveFile(virConnectPtr conn, const char *filename, + virDomainDefPtr def) +{ virConfPtr conf; xenUnifiedPrivatePtr priv = conn->privateData; int ret; @@ -177,8 +183,7 @@ xenXMConfigSaveFile(virConnectPtr conn, const char *filename, virDomainDefPtr de * calling this function */ int -xenXMConfigCacheRemoveFile(virConnectPtr conn, - const char *filename) +xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename) { xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; @@ -309,7 +314,9 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) * Caller must hold the lock on 'conn->privateData' before * calling this function */ -int xenXMConfigCacheRefresh(virConnectPtr conn) { +int +xenXMConfigCacheRefresh(virConnectPtr conn) +{ xenUnifiedPrivatePtr priv = conn->privateData; DIR *dh; struct dirent *ent; @@ -411,8 +418,7 @@ int xenXMConfigCacheRefresh(virConnectPtr conn) { * every few seconds */ virDrvOpenStatus -xenXMOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { xenUnifiedPrivatePtr priv = conn->privateData; @@ -442,7 +448,9 @@ xenXMOpen(virConnectPtr conn, * Free the cached config files associated with this * connection */ -int xenXMClose(virConnectPtr conn) { +int +xenXMClose(virConnectPtr conn) +{ xenUnifiedPrivatePtr priv = conn->privateData; virHashFree(priv->nameConfigMap); @@ -455,9 +463,7 @@ int xenXMClose(virConnectPtr conn) { * Since these are all offline domains, the state is always SHUTOFF. */ int -xenXMDomainGetState(virDomainPtr domain, - int *state, - int *reason, +xenXMDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { virCheckFlags(0, -1); @@ -477,7 +483,9 @@ xenXMDomainGetState(virDomainPtr domain, * Since these are all offline domains, we only return info about * VCPUs and memory. */ -int xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { +int +xenXMDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) +{ xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; @@ -519,7 +527,8 @@ error: * Turn a config record into a lump of XML describing the * domain, suitable for later feeding for virDomainCreateXML */ -char *xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) +char * +xenXMDomainGetXMLDesc(virDomainPtr domain, unsigned int flags) { xenUnifiedPrivatePtr priv; const char *filename; @@ -555,7 +564,9 @@ cleanup: /* * Update amount of memory in the config file */ -int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) { +int +xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) +{ xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; @@ -565,11 +576,8 @@ int xenXMDomainSetMemory(virDomainPtr domain, unsigned long memory) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - if (domain->conn->flags & VIR_CONNECT_RO) - return -1; - if (domain->id != -1) - return -1; - if (memory < 1024 * MIN_XEN_GUEST_SIZE) + if (domain->conn->flags & VIR_CONNECT_RO || domain->id != -1 || + memory < 1024 * MIN_XEN_GUEST_SIZE) return -1; priv = domain->conn->privateData; @@ -600,7 +608,9 @@ cleanup: /* * Update maximum memory limit in config */ -int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { +int +xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) +{ xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; @@ -610,9 +620,7 @@ int xenXMDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - if (domain->conn->flags & VIR_CONNECT_RO) - return -1; - if (domain->id != -1) + if (domain->conn->flags & VIR_CONNECT_RO || domain->id != -1) return -1; priv = domain->conn->privateData; @@ -643,7 +651,9 @@ cleanup: /* * Get max memory limit from config */ -unsigned long long xenXMDomainGetMaxMemory(virDomainPtr domain) { +unsigned long long +xenXMDomainGetMaxMemory(virDomainPtr domain) +{ xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; @@ -821,9 +831,9 @@ cleanup: * * Returns 0 for success; -1 (with errno) on error */ -int xenXMDomainPinVcpu(virDomainPtr domain, - unsigned int vcpu ATTRIBUTE_UNUSED, - unsigned char *cpumap, int maplen) +int +xenXMDomainPinVcpu(virDomainPtr domain, unsigned int vcpu ATTRIBUTE_UNUSED, + unsigned char *cpumap, int maplen) { xenUnifiedPrivatePtr priv; const char *filename; @@ -875,7 +885,9 @@ int xenXMDomainPinVcpu(virDomainPtr domain, /* * Find an inactive domain based on its name */ -virDomainPtr xenXMDomainLookupByName(virConnectPtr conn, const char *domname) { +virDomainPtr +xenXMDomainLookupByName(virConnectPtr conn, const char *domname) +{ xenUnifiedPrivatePtr priv = conn->privateData; const char *filename; xenXMConfCachePtr entry; @@ -908,7 +920,10 @@ cleanup: /* * Hash table iterator to search for a domain based on UUID */ -static int xenXMDomainSearchForUUID(const void *payload, const void *name ATTRIBUTE_UNUSED, const void *data) { +static int +xenXMDomainSearchForUUID(const void *payload, const void *name ATTRIBUTE_UNUSED, + const void *data) +{ const unsigned char *wantuuid = (const unsigned char *)data; const xenXMConfCachePtr entry = (const xenXMConfCachePtr)payload; @@ -921,8 +936,9 @@ static int xenXMDomainSearchForUUID(const void *payload, const void *name ATTRIB /* * Find an inactive domain based on its UUID */ -virDomainPtr xenXMDomainLookupByUUID(virConnectPtr conn, - const unsigned char *uuid) { +virDomainPtr +xenXMDomainLookupByUUID(virConnectPtr conn, const unsigned char *uuid) +{ xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; virDomainPtr ret = NULL; @@ -951,7 +967,9 @@ cleanup: /* * Start a domain from an existing defined config file */ -int xenXMDomainCreate(virDomainPtr domain) { +int +xenXMDomainCreate(virDomainPtr domain) +{ char *sexpr; int ret = -1; xenUnifiedPrivatePtr priv= (xenUnifiedPrivatePtr) domain->conn->privateData; @@ -1004,7 +1022,8 @@ int xenXMDomainCreate(virDomainPtr domain) { * Create a config file for a domain, based on an XML * document describing its config */ -virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) +virDomainPtr +xenXMDomainDefineXML(virConnectPtr conn, const char *xml) { virDomainPtr ret; char *filename = NULL; @@ -1141,7 +1160,9 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) /* * Delete a domain from disk */ -int xenXMDomainUndefine(virDomainPtr domain) { +int +xenXMDomainUndefine(virDomainPtr domain) +{ xenUnifiedPrivatePtr priv; const char *filename; xenXMConfCachePtr entry; @@ -1192,7 +1213,8 @@ struct xenXMListIteratorContext { char ** names; }; -static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { +static void +xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) { struct xenXMListIteratorContext *ctx = data; virDomainPtr dom = NULL; @@ -1218,7 +1240,9 @@ static void xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, * List all defined domains, filtered to remove any which * are currently running */ -int xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { +int +xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) +{ xenUnifiedPrivatePtr priv = conn->privateData; struct xenXMListIteratorContext ctx; int i, ret = -1; @@ -1258,7 +1282,9 @@ cleanup: * Return the maximum number of defined domains - not filtered * based on number running */ -int xenXMNumOfDefinedDomains(virConnectPtr conn) { +int +xenXMNumOfDefinedDomains(virConnectPtr conn) +{ xenUnifiedPrivatePtr priv = conn->privateData; int ret = -1; @@ -1387,7 +1413,8 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const char *xml, */ static int xenXMDomainDetachDeviceFlags(virDomainPtr domain, const char *xml, - unsigned int flags) { + unsigned int flags) +{ const char *filename = NULL; xenXMConfCachePtr entry = NULL; virDomainDeviceDefPtr dev = NULL; @@ -1497,7 +1524,8 @@ xenXMDomainBlockPeek(virDomainPtr dom ATTRIBUTE_UNUSED, } -static char *xenXMAutostartLinkName(virDomainPtr dom) +static char * +xenXMAutostartLinkName(virDomainPtr dom) { char *ret; if (virAsprintf(&ret, "/etc/xen/auto/%s", dom->name) < 0) @@ -1505,7 +1533,8 @@ static char *xenXMAutostartLinkName(virDomainPtr dom) return ret; } -static char *xenXMDomainConfigName(virDomainPtr dom) +static char * +xenXMDomainConfigName(virDomainPtr dom) { char *ret; if (virAsprintf(&ret, "/etc/xen/%s", dom->name) < 0) @@ -1513,7 +1542,8 @@ static char *xenXMDomainConfigName(virDomainPtr dom) return ret; } -int xenXMDomainGetAutostart(virDomainPtr dom, int *autostart) +int +xenXMDomainGetAutostart(virDomainPtr dom, int *autostart) { char *linkname = xenXMAutostartLinkName(dom); char *config = xenXMDomainConfigName(dom); @@ -1541,7 +1571,8 @@ cleanup: } -int xenXMDomainSetAutostart(virDomainPtr dom, int autostart) +int +xenXMDomainSetAutostart(virDomainPtr dom, int autostart) { char *linkname = xenXMAutostartLinkName(dom); char *config = xenXMDomainConfigName(dom); -- 1.7.11.7

On 2013年01月31日 02:51, John Ferlan wrote:
Clean up some function headers. --- src/xen/xm_internal.c | 119 +++++++++++++++++++++++++++++++------------------- 1 file changed, 75 insertions(+), 44 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 1c1db54..912de3f 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1,7 +1,7 @@ /* * xm_internal.h: helper routines for dealing with inactive domains * - * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc. + * Copyright (C) 2006-2007, 2009-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -124,7 +124,10 @@ struct xenXMConfigReaperData { };
/* Remove any configs which were not refreshed recently */ -static int xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUSED, const void *data) { +static int +xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUSED, + const void *data) +{ const struct xenXMConfigReaperData *args = data; xenXMConfCachePtr entry = (xenXMConfCachePtr)payload;
@@ -143,7 +146,8 @@ static int xenXMConfigReaper(const void *payload, const void *key ATTRIBUTE_UNUS
static virDomainDefPtr -xenXMConfigReadFile(virConnectPtr conn, const char *filename) { +xenXMConfigReadFile(virConnectPtr conn, const char *filename) +{ virConfPtr conf; virDomainDefPtr def; xenUnifiedPrivatePtr priv = conn->privateData; @@ -158,7 +162,9 @@ xenXMConfigReadFile(virConnectPtr conn, const char *filename) { }
static int -xenXMConfigSaveFile(virConnectPtr conn, const char *filename, virDomainDefPtr def) { +xenXMConfigSaveFile(virConnectPtr conn, const char *filename, + virDomainDefPtr def) +{ virConfPtr conf; xenUnifiedPrivatePtr priv = conn->privateData; int ret; @@ -177,8 +183,7 @@ xenXMConfigSaveFile(virConnectPtr conn, const char *filename, virDomainDefPtr de * calling this function */ int -xenXMConfigCacheRemoveFile(virConnectPtr conn, - const char *filename) +xenXMConfigCacheRemoveFile(virConnectPtr conn, const char *filename) { xenUnifiedPrivatePtr priv = conn->privateData; xenXMConfCachePtr entry; @@ -309,7 +314,9 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) * Caller must hold the lock on 'conn->privateData' before * calling this function */ -int xenXMConfigCacheRefresh(virConnectPtr conn) { +int +xenXMConfigCacheRefresh(virConnectPtr conn) +{ xenUnifiedPrivatePtr priv = conn->privateData; DIR *dh; struct dirent *ent; @@ -411,8 +418,7 @@ int xenXMConfigCacheRefresh(virConnectPtr conn) { * every few seconds */ virDrvOpenStatus -xenXMOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenXMOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { xenUnifiedPrivatePtr priv = conn->privateData; @@ -442,7 +448,9 @@ xenXMOpen(virConnectPtr conn, * Free the cached config files associated with this * connection */ -int xenXMClose(virConnectPtr conn) { +int +xenXMClose(virConnectPtr conn) +{ xenUnifiedPrivatePtr priv = conn->privateData;
virHashFree(priv->nameConfigMap); @@ -455,9 +463,7 @@ int xenXMClose(virConnectPtr conn) { * Since these are all offline domains, the state is always SHUTOFF. */ int -xenXMDomainGetState(virDomainPtr domain, - int *state, - int *reason, +xenXMDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags)
Again, not sure if I like this. All others except things like these are all good.

Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xend_internal.c | 227 ++++++++++++------------------------------------ 1 file changed, 56 insertions(+), 171 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index b03b7bc..a8e276d 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -613,9 +613,6 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) struct sockaddr_un *addr; xenUnifiedPrivatePtr priv; - if ((conn == NULL) || (path == NULL)) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; memset(&priv->addr, 0, sizeof(priv->addr)); priv->addrfamily = AF_UNIX; @@ -650,17 +647,12 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) static int xenDaemonOpen_tcp(virConnectPtr conn, const char *host, const char *port) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; struct addrinfo *res, *r; struct addrinfo hints; int saved_errno = EINVAL; int ret; - if ((conn == NULL) || (host == NULL) || (port == NULL)) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; - priv->addrlen = 0; memset(&priv->addr, 0, sizeof(priv->addr)); @@ -928,14 +920,7 @@ static int xend_detect_config_version(virConnectPtr conn) { struct sexpr *root; const char *value; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; root = sexpr_get(conn, "/xend/node/"); if (root == NULL) @@ -1013,9 +998,6 @@ sexpr_to_xend_domain_info(virDomainPtr domain, const struct sexpr *root, { int vcpus; - if ((root == NULL) || (info == NULL)) - return -1; - info->state = sexpr_to_xend_domain_state(domain, root); info->memory = sexpr_u64(root, "domain/memory") << 10; info->maxMem = sexpr_u64(root, "domain/maxmem") << 10; @@ -1044,10 +1026,6 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) { const char *machine; - - if ((root == NULL) || (info == NULL)) - return -1; - machine = sexpr_node(root, "node/machine"); if (machine == NULL) { info->model[0] = 0; @@ -1200,12 +1178,7 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) unsigned char uuid[VIR_UUID_BUFLEN]; const char *name; const char *tmp; - xenUnifiedPrivatePtr priv; - - if ((conn == NULL) || (root == NULL)) - return NULL; - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (sexpr_uuid(uuid, root, "domain/uuid") < 0) goto error; @@ -1350,7 +1323,7 @@ xenDaemonClose(virConnectPtr conn ATTRIBUTE_UNUSED) int xenDaemonDomainSuspend(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1376,7 +1349,7 @@ xenDaemonDomainSuspend(virDomainPtr domain) int xenDaemonDomainResume(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1403,7 +1376,7 @@ xenDaemonDomainResume(virDomainPtr domain) int xenDaemonDomainShutdown(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1433,7 +1406,7 @@ xenDaemonDomainReboot(virDomainPtr domain, unsigned int flags) { virCheckFlags(0, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1470,7 +1443,7 @@ xenDaemonDomainDestroyFlags(virDomainPtr domain, { virCheckFlags(0, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1498,15 +1471,13 @@ xenDaemonDomainGetOSType(virDomainPtr domain) { char *type; struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return NULL; @@ -1545,8 +1516,7 @@ xenDaemonDomainGetOSType(virDomainPtr domain) int xenDaemonDomainSave(virDomainPtr domain, const char *filename) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (filename == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1583,8 +1553,7 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, { virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (filename == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1616,11 +1585,6 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, int xenDaemonDomainRestore(virConnectPtr conn, const char *filename) { - if ((conn == NULL) || (filename == NULL)) { - /* this should be caught at the interface but ... */ - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } return xend_op(conn, "", "op", "restore", "file", filename, NULL); } @@ -1638,15 +1602,13 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain) { unsigned long long ret = 0; struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return 0; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return 0; @@ -1677,15 +1639,13 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -1714,15 +1674,13 @@ int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -1739,7 +1697,7 @@ xenDaemonDomainFetch(virConnectPtr conn, const char *cpus) { struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def; int id; char * tty; @@ -1752,8 +1710,6 @@ xenDaemonDomainFetch(virConnectPtr conn, if (root == NULL) return NULL; - priv = (xenUnifiedPrivatePtr) conn->privateData; - id = xenGetDomIdFromSxpr(root, priv->xendConfigVersion); xenUnifiedLock(priv); if (sexpr_lookup(root, "domain/image/hvm")) @@ -1791,17 +1747,16 @@ char * xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; virDomainDefPtr def; char *xml; /* Flags checked by virDomainDefFormat */ - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { /* fall-through to the next driver to handle */ @@ -1837,16 +1792,13 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (info == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -1915,11 +1867,6 @@ xenDaemonLookupByName(virConnectPtr conn, const char *domname) struct sexpr *root; virDomainPtr ret = NULL; - if ((conn == NULL) || (domname == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } - root = sexpr_get(conn, "/xend/domain/%s?detail=1", domname); if (root == NULL) goto error; @@ -1946,15 +1893,6 @@ xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { int ret = -1; struct sexpr *root; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - if (info == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - root = sexpr_get(conn, "/xend/node/"); if (root == NULL) return -1; @@ -1979,16 +1917,6 @@ xenDaemonNodeGetTopology(virConnectPtr conn, int ret = -1; struct sexpr *root; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - if (caps == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - root = sexpr_get(conn, "/xend/node/"); if (root == NULL) { return -1; @@ -2017,14 +1945,6 @@ xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer) int major, minor; unsigned long version; - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - if (hvVer == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } root = sexpr_get(conn, "/xend/node/"); if (root == NULL) return -1; @@ -2061,8 +1981,6 @@ xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids) if (maxids == 0) return 0; - if ((ids == NULL) || (maxids < 0)) - goto error; root = sexpr_get(conn, "/xend/domain"); if (root == NULL) goto error; @@ -2168,21 +2086,18 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, unsigned int flags) { char buf[VIR_UUID_BUFLEN]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; int max; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (vcpus < 1)) { + if (vcpus < 1) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) || (flags & VIR_DOMAIN_VCPU_MAXIMUM)) return -2; @@ -2254,16 +2169,14 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; int i, j, ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; virDomainDefPtr def = NULL; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (cpumap == NULL) || (maplen < 1) || (maplen > (int)sizeof(cpumap_t))) { + if (maplen > (int)sizeof(cpumap_t)) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { mapstr[0] = '['; mapstr[1] = 0; @@ -2335,19 +2248,17 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1); - if (domain == NULL || domain->conn == NULL || domain->name == NULL) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - /* If xendConfigVersion is 2, then we can only report _LIVE (and * xm_internal reports _CONFIG). If it is 3, then _LIVE and * _CONFIG are always in sync for a running system. */ @@ -2404,12 +2315,7 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumap; int vcpu, cpu; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (info == NULL) || (maxinfo < 1)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - if (cpumaps != NULL && maplen < 1) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -2568,13 +2474,11 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, int ret; char *sexpr; virDomainPtr dom = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;; virDomainDefPtr def; virCheckFlags(0, NULL); - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (!(def = virDomainDefParseString(priv->caps, xmlDesc, 1 << VIR_DOMAIN_VIRT_XEN, VIR_DOMAIN_XML_INACTIVE))) @@ -2630,7 +2534,7 @@ static int xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2641,13 +2545,11 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -2798,7 +2700,7 @@ int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2809,13 +2711,11 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -2912,7 +2812,7 @@ static int xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char class[8], ref[80]; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; @@ -2922,13 +2822,11 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1); - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3013,9 +2911,9 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, { struct sexpr *root; const char *tmp; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -3023,7 +2921,6 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, /* xm_internal.c (the support for defined domains from /etc/xen * config files used by old Xen) will handle this. */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -3053,9 +2950,9 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); return -1; } @@ -3063,7 +2960,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, /* xm_internal.c (the support for defined domains from /etc/xen * config files used by old Xen) will handle this. */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -3315,15 +3211,15 @@ xenDaemonDomainMigratePerform(virDomainPtr domain, return ret; } -virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { +virDomainPtr +xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) +{ int ret; char *sexpr; virDomainPtr dom; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def; - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return NULL; @@ -3362,17 +3258,15 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { } int xenDaemonDomainCreate(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; int ret; virDomainPtr tmp; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -3391,15 +3285,13 @@ int xenDaemonDomainCreate(virDomainPtr domain) int xenDaemonDomainUndefine(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; @@ -3457,8 +3349,6 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1; - if ((names == NULL) || (maxnames < 0)) - goto error; if (maxnames == 0) return 0; @@ -3509,18 +3399,17 @@ error: static char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; const char *ret = NULL; char *schedulertype = NULL; - if (domain->conn == NULL || domain->name == NULL) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } /* Support only xendConfigVersion >=4 */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3581,19 +3470,18 @@ static int xenDaemonGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int sched_nparam = 0; int ret = -1; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } /* Support only xendConfigVersion >=4 */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3688,20 +3576,19 @@ static int xenDaemonSetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int i; int sched_nparam = 0; int ret = -1; - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } /* Support only xendConfigVersion >=4 and active domains */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3799,7 +3686,7 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root = NULL; int fd = -1, ret = -1; virDomainDefPtr def; @@ -3808,8 +3695,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, int vncport; const char *actual; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -2; /* Decline, allow XM to handle it. */ -- 1.7.11.7

John Ferlan wrote:
Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xend_internal.c | 227 ++++++++++++------------------------------------ 1 file changed, 56 insertions(+), 171 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index b03b7bc..a8e276d 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -613,9 +613,6 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) struct sockaddr_un *addr; xenUnifiedPrivatePtr priv;
- if ((conn == NULL) || (path == NULL)) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; memset(&priv->addr, 0, sizeof(priv->addr)); priv->addrfamily = AF_UNIX; @@ -650,17 +647,12 @@ xenDaemonOpen_unix(virConnectPtr conn, const char *path) static int xenDaemonOpen_tcp(virConnectPtr conn, const char *host, const char *port) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; struct addrinfo *res, *r; struct addrinfo hints; int saved_errno = EINVAL; int ret;
- if ((conn == NULL) || (host == NULL) || (port == NULL)) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; - priv->addrlen = 0; memset(&priv->addr, 0, sizeof(priv->addr));
@@ -928,14 +920,7 @@ static int xend_detect_config_version(virConnectPtr conn) { struct sexpr *root; const char *value; - xenUnifiedPrivatePtr priv; - - if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
root = sexpr_get(conn, "/xend/node/"); if (root == NULL) @@ -1013,9 +998,6 @@ sexpr_to_xend_domain_info(virDomainPtr domain, const struct sexpr *root, { int vcpus;
- if ((root == NULL) || (info == NULL)) - return -1; - info->state = sexpr_to_xend_domain_state(domain, root); info->memory = sexpr_u64(root, "domain/memory") << 10; info->maxMem = sexpr_u64(root, "domain/maxmem") << 10; @@ -1044,10 +1026,6 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) { const char *machine;
- - if ((root == NULL) || (info == NULL)) - return -1; - machine = sexpr_node(root, "node/machine"); if (machine == NULL) { info->model[0] = 0; @@ -1200,12 +1178,7 @@ sexpr_to_domain(virConnectPtr conn, const struct sexpr *root) unsigned char uuid[VIR_UUID_BUFLEN]; const char *name; const char *tmp; - xenUnifiedPrivatePtr priv; - - if ((conn == NULL) || (root == NULL)) - return NULL; - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
if (sexpr_uuid(uuid, root, "domain/uuid") < 0) goto error; @@ -1350,7 +1323,7 @@ xenDaemonClose(virConnectPtr conn ATTRIBUTE_UNUSED) int xenDaemonDomainSuspend(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) {
Same comment here as Osier had in 4/11. domain->name is guaranteed non-NULL right? Regards, Jim
virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1376,7 +1349,7 @@ xenDaemonDomainSuspend(virDomainPtr domain) int xenDaemonDomainResume(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1403,7 +1376,7 @@ xenDaemonDomainResume(virDomainPtr domain) int xenDaemonDomainShutdown(virDomainPtr domain) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1433,7 +1406,7 @@ xenDaemonDomainReboot(virDomainPtr domain, unsigned int flags) { virCheckFlags(0, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1470,7 +1443,7 @@ xenDaemonDomainDestroyFlags(virDomainPtr domain, { virCheckFlags(0, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1498,15 +1471,13 @@ xenDaemonDomainGetOSType(virDomainPtr domain) { char *type; struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return NULL;
@@ -1545,8 +1516,7 @@ xenDaemonDomainGetOSType(virDomainPtr domain) int xenDaemonDomainSave(virDomainPtr domain, const char *filename) { - if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (filename == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1583,8 +1553,7 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, { virCheckFlags(VIR_DUMP_LIVE | VIR_DUMP_CRASH, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (filename == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -1616,11 +1585,6 @@ xenDaemonDomainCoreDump(virDomainPtr domain, const char *filename, int xenDaemonDomainRestore(virConnectPtr conn, const char *filename) { - if ((conn == NULL) || (filename == NULL)) { - /* this should be caught at the interface but ... */ - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } return xend_op(conn, "", "op", "restore", "file", filename, NULL); }
@@ -1638,15 +1602,13 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain) { unsigned long long ret = 0; struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return 0; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return 0;
@@ -1677,15 +1639,13 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -1714,15 +1674,13 @@ int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -1739,7 +1697,7 @@ xenDaemonDomainFetch(virConnectPtr conn, const char *cpus) { struct sexpr *root; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def; int id; char * tty; @@ -1752,8 +1710,6 @@ xenDaemonDomainFetch(virConnectPtr conn, if (root == NULL) return NULL;
- priv = (xenUnifiedPrivatePtr) conn->privateData; - id = xenGetDomIdFromSxpr(root, priv->xendConfigVersion); xenUnifiedLock(priv); if (sexpr_lookup(root, "domain/image/hvm")) @@ -1791,17 +1747,16 @@ char * xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; virDomainDefPtr def; char *xml;
/* Flags checked by virDomainDefFormat */
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; } - priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { /* fall-through to the next driver to handle */ @@ -1837,16 +1792,13 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) || - (info == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -1915,11 +1867,6 @@ xenDaemonLookupByName(virConnectPtr conn, const char *domname) struct sexpr *root; virDomainPtr ret = NULL;
- if ((conn == NULL) || (domname == NULL)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return NULL; - } - root = sexpr_get(conn, "/xend/domain/%s?detail=1", domname); if (root == NULL) goto error; @@ -1946,15 +1893,6 @@ xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { int ret = -1; struct sexpr *root;
- if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - if (info == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - root = sexpr_get(conn, "/xend/node/"); if (root == NULL) return -1; @@ -1979,16 +1917,6 @@ xenDaemonNodeGetTopology(virConnectPtr conn, int ret = -1; struct sexpr *root;
- if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - - if (caps == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - root = sexpr_get(conn, "/xend/node/"); if (root == NULL) { return -1; @@ -2017,14 +1945,6 @@ xenDaemonGetVersion(virConnectPtr conn, unsigned long *hvVer) int major, minor; unsigned long version;
- if (!VIR_IS_CONNECT(conn)) { - virReportError(VIR_ERR_INVALID_CONN, __FUNCTION__); - return -1; - } - if (hvVer == NULL) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } root = sexpr_get(conn, "/xend/node/"); if (root == NULL) return -1; @@ -2061,8 +1981,6 @@ xenDaemonListDomains(virConnectPtr conn, int *ids, int maxids) if (maxids == 0) return 0;
- if ((ids == NULL) || (maxids < 0)) - goto error; root = sexpr_get(conn, "/xend/domain"); if (root == NULL) goto error; @@ -2168,21 +2086,18 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, unsigned int flags) { char buf[VIR_UUID_BUFLEN]; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; int max;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (vcpus < 1)) { + if (vcpus < 1) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if ((domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) || (flags & VIR_DOMAIN_VCPU_MAXIMUM)) return -2; @@ -2254,16 +2169,14 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; int i, j, ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; virDomainDefPtr def = NULL;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (cpumap == NULL) || (maplen < 1) || (maplen > (int)sizeof(cpumap_t))) { + if (maplen > (int)sizeof(cpumap_t)) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) { mapstr[0] = '['; mapstr[1] = 0; @@ -2335,19 +2248,17 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | VIR_DOMAIN_VCPU_MAXIMUM, -1);
- if (domain == NULL || domain->conn == NULL || domain->name == NULL) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - /* If xendConfigVersion is 2, then we can only report _LIVE (and * xm_internal reports _CONFIG). If it is 3, then _LIVE and * _CONFIG are always in sync for a running system. */ @@ -2404,12 +2315,7 @@ xenDaemonDomainGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, unsigned char *cpumap; int vcpu, cpu;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL) - || (info == NULL) || (maxinfo < 1)) { - virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); - return -1; - } - if (cpumaps != NULL && maplen < 1) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -2568,13 +2474,11 @@ xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, int ret; char *sexpr; virDomainPtr dom = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;; virDomainDefPtr def;
virCheckFlags(0, NULL);
- priv = (xenUnifiedPrivatePtr) conn->privateData; - if (!(def = virDomainDefParseString(priv->caps, xmlDesc, 1 << VIR_DOMAIN_VIRT_XEN, VIR_DOMAIN_XML_INACTIVE))) @@ -2630,7 +2534,7 @@ static int xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2641,13 +2545,11 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -2798,7 +2700,7 @@ int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2809,13 +2711,11 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, virCheckFlags(VIR_DOMAIN_DEVICE_MODIFY_LIVE | VIR_DOMAIN_DEVICE_MODIFY_CONFIG, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -2912,7 +2812,7 @@ static int xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; char class[8], ref[80]; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; @@ -2922,13 +2822,11 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml,
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE | VIR_DOMAIN_AFFECT_CONFIG, -1);
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0) { /* Cannot modify live config if domain is inactive */ if (flags & VIR_DOMAIN_DEVICE_MODIFY_LIVE) { @@ -3013,9 +2911,9 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, { struct sexpr *root; const char *tmp; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; } @@ -3023,7 +2921,6 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, /* xm_internal.c (the support for defined domains from /etc/xen * config files used by old Xen) will handle this. */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -3053,9 +2950,9 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); return -1; } @@ -3063,7 +2960,6 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, /* xm_internal.c (the support for defined domains from /etc/xen * config files used by old Xen) will handle this. */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -3315,15 +3211,15 @@ xenDaemonDomainMigratePerform(virDomainPtr domain, return ret; }
-virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { +virDomainPtr +xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) +{ int ret; char *sexpr; virDomainPtr dom; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def;
- priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return NULL;
@@ -3362,17 +3258,15 @@ virDomainPtr xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) { } int xenDaemonDomainCreate(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; int ret; virDomainPtr tmp;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -3391,15 +3285,13 @@ int xenDaemonDomainCreate(virDomainPtr domain)
int xenDaemonDomainUndefine(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
@@ -3457,8 +3349,6 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -1;
- if ((names == NULL) || (maxnames < 0)) - goto error; if (maxnames == 0) return 0;
@@ -3509,18 +3399,17 @@ error: static char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; const char *ret = NULL; char *schedulertype = NULL;
- if (domain->conn == NULL || domain->name == NULL) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return NULL; }
/* Support only xendConfigVersion >=4 */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3581,19 +3470,18 @@ static int xenDaemonGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int sched_nparam = 0; int ret = -1;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
/* Support only xendConfigVersion >=4 */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3688,20 +3576,19 @@ static int xenDaemonSetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int i; int sched_nparam = 0; int ret = -1;
- if ((domain == NULL) || (domain->conn == NULL) || (domain->name == NULL)) { + if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); return -1; }
/* Support only xendConfigVersion >=4 and active domains */ - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->xendConfigVersion < XEND_CONFIG_VERSION_3_1_0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("unsupported in xendConfigVersion < 4")); @@ -3799,7 +3686,7 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; struct sexpr *root = NULL; int fd = -1, ret = -1; virDomainDefPtr def; @@ -3808,8 +3695,6 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, int vncport; const char *actual;
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (domain->id < 0 && priv->xendConfigVersion < XEND_CONFIG_VERSION_3_0_4) return -2; /* Decline, allow XM to handle it. */

Commit id '87b4c10c' moved the VIR_ALLOC_N, but didn't check if 'cpuset' had been allocated on failure. --- src/xen/xend_internal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index a8e276d..51f1f15 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1130,8 +1130,10 @@ sexpr_to_xend_topology(const struct sexpr *root, goto error; } - if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) + if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) { + virBitmapFree(cpuset); goto memory_error; + } for (n = 0, cpu = 0; cpu < numCpus; cpu++) { bool used; -- 1.7.11.7

John Ferlan wrote:
Commit id '87b4c10c' moved the VIR_ALLOC_N, but didn't check if 'cpuset' had been allocated on failure. --- src/xen/xend_internal.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index a8e276d..51f1f15 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1130,8 +1130,10 @@ sexpr_to_xend_topology(const struct sexpr *root, goto error; }
- if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) + if (VIR_ALLOC_N(cpuInfo, numCpus) < 0) { + virBitmapFree(cpuset); goto memory_error; + }
for (n = 0, cpu = 0; cpu < numCpus; cpu++) { bool used;
ACK. Regards, Jim

Clean up some function headers --- src/xen/xend_internal.c | 114 ++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 63 deletions(-) diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 51f1f15..8bf6fc1 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1,7 +1,7 @@ /* * xend_internal.c: access to Xen though the Xen Daemon interface * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * * This file is subject to the terms and conditions of the GNU Lesser General @@ -60,11 +60,8 @@ #define XEND_RCV_BUF_MAX_LEN (256 * 1024) static int -virDomainXMLDevID(virDomainPtr domain, - virDomainDeviceDefPtr dev, - char *class, - char *ref, - int ref_len); +virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, char *class, + char *ref, int ref_len); /** * do_connect: @@ -331,8 +328,7 @@ xend_req(int fd, char **content) * Returns the HTTP return code or -1 in case or error. */ static int ATTRIBUTE_NONNULL(3) -xend_get(virConnectPtr xend, const char *path, - char **content) +xend_get(virConnectPtr xend, const char *path, char **content) { int ret; int s = do_connect(xend); @@ -871,9 +867,7 @@ xenDaemonDomainLookupByName_ids(virConnectPtr xend, const char *domname, * Returns the 0 on success; -1 (with errno) on error */ int -xenDaemonDomainLookupByID(virConnectPtr xend, - int id, - char **domname, +xenDaemonDomainLookupByID(virConnectPtr xend, int id, char **domname, unsigned char *uuid) { const char *name = NULL; @@ -917,7 +911,8 @@ error: static int -xend_detect_config_version(virConnectPtr conn) { +xend_detect_config_version(virConnectPtr conn) +{ struct sexpr *root; const char *value; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; @@ -1085,8 +1080,7 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) * Returns 0 in case of success, -1 in case of error */ static int -sexpr_to_xend_topology(const struct sexpr *root, - virCapsPtr caps) +sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps) { const char *nodeToCpu; const char *cur; @@ -1235,8 +1229,7 @@ error: * Returns 0 in case of success, -1 in case of error. */ virDrvOpenStatus -xenDaemonOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { char *port = NULL; @@ -1440,8 +1433,7 @@ xenDaemonDomainReboot(virDomainPtr domain, unsigned int flags) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainDestroyFlags(virDomainPtr domain, - unsigned int flags) +xenDaemonDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { virCheckFlags(0, -1); @@ -1473,7 +1465,7 @@ xenDaemonDomainGetOSType(virDomainPtr domain) { char *type; struct sexpr *root; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1604,7 +1596,7 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain) { unsigned long long ret = 0; struct sexpr *root; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1641,7 +1633,7 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1676,7 +1668,7 @@ int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1693,9 +1685,7 @@ xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) virDomainDefPtr -xenDaemonDomainFetch(virConnectPtr conn, - int domid, - const char *name, +xenDaemonDomainFetch(virConnectPtr conn, int domid, const char *name, const char *cpus) { struct sexpr *root; @@ -1749,7 +1739,7 @@ char * xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainDefPtr def; char *xml; @@ -1794,7 +1784,7 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1826,9 +1816,7 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) * Returns 0 in case of success, -1 in case of error */ int -xenDaemonDomainGetState(virDomainPtr domain, - int *state, - int *reason, +xenDaemonDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { xenUnifiedPrivatePtr priv = domain->conn->privateData; @@ -1891,7 +1879,8 @@ error: * Returns 0 in case of success and -1 in case of failure. */ int -xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { +xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) +{ int ret = -1; struct sexpr *root; @@ -1914,8 +1903,8 @@ xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { * Returns -1 in case of error, 0 otherwise. */ int -xenDaemonNodeGetTopology(virConnectPtr conn, - virCapsPtr caps) { +xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps) +{ int ret = -1; struct sexpr *root; @@ -2051,7 +2040,8 @@ error: * Returns a new domain object or NULL in case of failure */ virDomainPtr -xenDaemonLookupByID(virConnectPtr conn, int id) { +xenDaemonLookupByID(virConnectPtr conn, int id) +{ char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; virDomainPtr ret; @@ -2088,7 +2078,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, unsigned int flags) { char buf[VIR_UUID_BUFLEN]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; int max; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | @@ -2171,7 +2161,7 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; int i, j, ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainDefPtr def = NULL; if (maplen > (int)sizeof(cpumap_t)) { @@ -2250,7 +2240,7 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -2470,13 +2460,12 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) * Returns a new domain object or NULL in case of failure */ virDomainPtr -xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, - unsigned int flags) +xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { int ret; char *sexpr; virDomainPtr dom = NULL; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def; virCheckFlags(0, NULL); @@ -2536,7 +2525,7 @@ static int xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2702,7 +2691,7 @@ int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2814,7 +2803,7 @@ static int xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char class[8], ref[80]; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; @@ -2908,12 +2897,11 @@ cleanup: } int -xenDaemonDomainGetAutostart(virDomainPtr domain, - int *autostart) +xenDaemonDomainGetAutostart(virDomainPtr domain, int *autostart) { struct sexpr *root; const char *tmp; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -2945,14 +2933,13 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, } int -xenDaemonDomainSetAutostart(virDomainPtr domain, - int autostart) +xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) { struct sexpr *root, *autonode; virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); @@ -3258,9 +3245,10 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) virDomainDefFree(def); return NULL; } -int xenDaemonDomainCreate(virDomainPtr domain) +int +xenDaemonDomainCreate(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; int ret; virDomainPtr tmp; @@ -3285,9 +3273,10 @@ int xenDaemonDomainCreate(virDomainPtr domain) return ret; } -int xenDaemonDomainUndefine(virDomainPtr domain) +int +xenDaemonDomainUndefine(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -3342,7 +3331,9 @@ error: } static int -xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { +xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, + int maxnames) +{ struct sexpr *root = NULL; int i, ret = -1; struct sexpr *_for_i, *node; @@ -3401,7 +3392,7 @@ error: static char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; const char *ret = NULL; char *schedulertype = NULL; @@ -3472,7 +3463,7 @@ static int xenDaemonGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int sched_nparam = 0; @@ -3578,7 +3569,7 @@ static int xenDaemonSetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int i; @@ -3688,7 +3679,7 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root = NULL; int fd = -1, ret = -1; virDomainDefPtr def; @@ -3810,11 +3801,8 @@ struct xenUnifiedDriver xenDaemonDriver = { * Returns 0 in case of success, -1 in case of failure. */ static int -virDomainXMLDevID(virDomainPtr domain, - virDomainDeviceDefPtr dev, - char *class, - char *ref, - int ref_len) +virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, + char *class, char *ref, int ref_len) { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref; -- 1.7.11.7

John Ferlan wrote:
Clean up some function headers --- src/xen/xend_internal.c | 114 ++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 63 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index 51f1f15..8bf6fc1 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -1,7 +1,7 @@ /* * xend_internal.c: access to Xen though the Xen Daemon interface * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <aliguori@us.ibm.com> * * This file is subject to the terms and conditions of the GNU Lesser General @@ -60,11 +60,8 @@ #define XEND_RCV_BUF_MAX_LEN (256 * 1024)
static int -virDomainXMLDevID(virDomainPtr domain, - virDomainDeviceDefPtr dev, - char *class, - char *ref, - int ref_len); +virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, char *class, + char *ref, int ref_len);
Style preference as Osier mentioned...
/** * do_connect: @@ -331,8 +328,7 @@ xend_req(int fd, char **content) * Returns the HTTP return code or -1 in case or error. */ static int ATTRIBUTE_NONNULL(3) -xend_get(virConnectPtr xend, const char *path, - char **content) +xend_get(virConnectPtr xend, const char *path, char **content) { int ret; int s = do_connect(xend); @@ -871,9 +867,7 @@ xenDaemonDomainLookupByName_ids(virConnectPtr xend, const char *domname, * Returns the 0 on success; -1 (with errno) on error */ int -xenDaemonDomainLookupByID(virConnectPtr xend, - int id, - char **domname, +xenDaemonDomainLookupByID(virConnectPtr xend, int id, char **domname, unsigned char *uuid) { const char *name = NULL; @@ -917,7 +911,8 @@ error:
static int -xend_detect_config_version(virConnectPtr conn) { +xend_detect_config_version(virConnectPtr conn) +{
ACK to these changes for consistency's sake.
struct sexpr *root; const char *value; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; @@ -1085,8 +1080,7 @@ sexpr_to_xend_node_info(const struct sexpr *root, virNodeInfoPtr info) * Returns 0 in case of success, -1 in case of error */ static int -sexpr_to_xend_topology(const struct sexpr *root, - virCapsPtr caps) +sexpr_to_xend_topology(const struct sexpr *root, virCapsPtr caps)
I personally prefer one line when it fits within 80 columns :).
{ const char *nodeToCpu; const char *cur; @@ -1235,8 +1229,7 @@ error: * Returns 0 in case of success, -1 in case of error. */ virDrvOpenStatus -xenDaemonOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenDaemonOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { char *port = NULL; @@ -1440,8 +1433,7 @@ xenDaemonDomainReboot(virDomainPtr domain, unsigned int flags) * Returns 0 in case of success, -1 (with errno) in case of error. */ int -xenDaemonDomainDestroyFlags(virDomainPtr domain, - unsigned int flags) +xenDaemonDomainDestroyFlags(virDomainPtr domain, unsigned int flags) { virCheckFlags(0, -1);
@@ -1473,7 +1465,7 @@ xenDaemonDomainGetOSType(virDomainPtr domain) { char *type; struct sexpr *root; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
ACK to these changes as well since this is the style used throughout the sources. Regards, Jim
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1604,7 +1596,7 @@ xenDaemonDomainGetMaxMemory(virDomainPtr domain) { unsigned long long ret = 0; struct sexpr *root; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1641,7 +1633,7 @@ int xenDaemonDomainSetMaxMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1676,7 +1668,7 @@ int xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory) { char buf[1024]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1693,9 +1685,7 @@ xenDaemonDomainSetMemory(virDomainPtr domain, unsigned long memory)
virDomainDefPtr -xenDaemonDomainFetch(virConnectPtr conn, - int domid, - const char *name, +xenDaemonDomainFetch(virConnectPtr conn, int domid, const char *name, const char *cpus) { struct sexpr *root; @@ -1749,7 +1739,7 @@ char * xenDaemonDomainGetXMLDesc(virDomainPtr domain, unsigned int flags, const char *cpus) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainDefPtr def; char *xml;
@@ -1794,7 +1784,7 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -1826,9 +1816,7 @@ xenDaemonDomainGetInfo(virDomainPtr domain, virDomainInfoPtr info) * Returns 0 in case of success, -1 in case of error */ int -xenDaemonDomainGetState(virDomainPtr domain, - int *state, - int *reason, +xenDaemonDomainGetState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { xenUnifiedPrivatePtr priv = domain->conn->privateData; @@ -1891,7 +1879,8 @@ error: * Returns 0 in case of success and -1 in case of failure. */ int -xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { +xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) +{ int ret = -1; struct sexpr *root;
@@ -1914,8 +1903,8 @@ xenDaemonNodeGetInfo(virConnectPtr conn, virNodeInfoPtr info) { * Returns -1 in case of error, 0 otherwise. */ int -xenDaemonNodeGetTopology(virConnectPtr conn, - virCapsPtr caps) { +xenDaemonNodeGetTopology(virConnectPtr conn, virCapsPtr caps) +{ int ret = -1; struct sexpr *root;
@@ -2051,7 +2040,8 @@ error: * Returns a new domain object or NULL in case of failure */ virDomainPtr -xenDaemonLookupByID(virConnectPtr conn, int id) { +xenDaemonLookupByID(virConnectPtr conn, int id) +{ char *name = NULL; unsigned char uuid[VIR_UUID_BUFLEN]; virDomainPtr ret; @@ -2088,7 +2078,7 @@ xenDaemonDomainSetVcpusFlags(virDomainPtr domain, unsigned int vcpus, unsigned int flags) { char buf[VIR_UUID_BUFLEN]; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; int max;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE | @@ -2171,7 +2161,7 @@ xenDaemonDomainPinVcpu(virDomainPtr domain, unsigned int vcpu, { char buf[VIR_UUID_BUFLEN], mapstr[sizeof(cpumap_t) * 64]; int i, j, ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainDefPtr def = NULL;
if (maplen > (int)sizeof(cpumap_t)) { @@ -2250,7 +2240,7 @@ xenDaemonDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags) { struct sexpr *root; int ret; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
virCheckFlags(VIR_DOMAIN_VCPU_LIVE | VIR_DOMAIN_VCPU_CONFIG | @@ -2470,13 +2460,12 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) * Returns a new domain object or NULL in case of failure */ virDomainPtr -xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, - unsigned int flags) +xenDaemonCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags) { int ret; char *sexpr; virDomainPtr dom = NULL; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; virDomainDefPtr def;
virCheckFlags(0, NULL); @@ -2536,7 +2525,7 @@ static int xenDaemonAttachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2702,7 +2691,7 @@ int xenDaemonUpdateDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char *sexpr = NULL; int ret = -1; virDomainDeviceDefPtr dev = NULL; @@ -2814,7 +2803,7 @@ static int xenDaemonDetachDeviceFlags(virDomainPtr domain, const char *xml, unsigned int flags) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; char class[8], ref[80]; virDomainDeviceDefPtr dev = NULL; virDomainDefPtr def = NULL; @@ -2908,12 +2897,11 @@ cleanup: }
int -xenDaemonDomainGetAutostart(virDomainPtr domain, - int *autostart) +xenDaemonDomainGetAutostart(virDomainPtr domain, int *autostart) { struct sexpr *root; const char *tmp; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -2945,14 +2933,13 @@ xenDaemonDomainGetAutostart(virDomainPtr domain, }
int -xenDaemonDomainSetAutostart(virDomainPtr domain, - int autostart) +xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) { struct sexpr *root, *autonode; virBuffer buffer = VIR_BUFFER_INITIALIZER; char *content = NULL; int ret = -1; - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, __FUNCTION__); @@ -3258,9 +3245,10 @@ xenDaemonDomainDefineXML(virConnectPtr conn, const char *xmlDesc) virDomainDefFree(def); return NULL; } -int xenDaemonDomainCreate(virDomainPtr domain) +int +xenDaemonDomainCreate(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; int ret; virDomainPtr tmp;
@@ -3285,9 +3273,10 @@ int xenDaemonDomainCreate(virDomainPtr domain) return ret; }
-int xenDaemonDomainUndefine(virDomainPtr domain) +int +xenDaemonDomainUndefine(virDomainPtr domain) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
if (domain->name == NULL) { virReportError(VIR_ERR_INVALID_ARG, __FUNCTION__); @@ -3342,7 +3331,9 @@ error: }
static int -xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) { +xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, + int maxnames) +{ struct sexpr *root = NULL; int i, ret = -1; struct sexpr *_for_i, *node; @@ -3401,7 +3392,7 @@ error: static char * xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; const char *ret = NULL; char *schedulertype = NULL; @@ -3472,7 +3463,7 @@ static int xenDaemonGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int sched_nparam = 0; @@ -3578,7 +3569,7 @@ static int xenDaemonSetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int nparams) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root; char *sched_type = NULL; int i; @@ -3688,7 +3679,7 @@ xenDaemonDomainBlockPeek(virDomainPtr domain, const char *path, unsigned long long offset, size_t size, void *buffer) { - xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; struct sexpr *root = NULL; int fd = -1, ret = -1; virDomainDefPtr def; @@ -3810,11 +3801,8 @@ struct xenUnifiedDriver xenDaemonDriver = { * Returns 0 in case of success, -1 in case of failure. */ static int -virDomainXMLDevID(virDomainPtr domain, - virDomainDeviceDefPtr dev, - char *class, - char *ref, - int ref_len) +virDomainXMLDevID(virDomainPtr domain, virDomainDeviceDefPtr dev, + char *class, char *ref, int ref_len) { xenUnifiedPrivatePtr priv = domain->conn->privateData; char *xref;

Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xen_hypervisor.c | 164 +++++++++-------------------------------------- 1 file changed, 30 insertions(+), 134 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index bfee56d..2437413 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1161,15 +1161,8 @@ char * xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams) { char *schedulertype = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return NULL; - } - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("priv->handle invalid")); @@ -1242,15 +1235,8 @@ int xenHypervisorGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return -1; - } - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("priv->handle invalid")); @@ -1350,12 +1336,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, xenUnifiedPrivatePtr priv; char buf[256]; - if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return -1; - } - if (nparams == 0) { /* nothing to do, exit early */ return 0; @@ -2252,12 +2232,7 @@ int xenHypervisorClose(virConnectPtr conn) { int ret; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return -1; @@ -2282,12 +2257,9 @@ xenHypervisorClose(virConnectPtr conn) int xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || hvVer == NULL) + if (priv->handle < 0) return -1; *hvVer = (hv_versions.hv >> 16) * 1000000 + (hv_versions.hv & 0xFFFF) * 1000; return 0; @@ -2803,11 +2775,8 @@ xenHypervisorNumOfDomains(virConnectPtr conn) int ret, nbids; static int last_maxids = 2; int maxids = last_maxids; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return -1; @@ -2860,14 +2829,9 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids) { xen_getdomaininfolist dominfos; int ret, nbids, i; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || - (ids == NULL) || (maxids < 0)) + if (priv->handle < 0) return -1; if (maxids == 0) @@ -3084,11 +3048,8 @@ int xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; - if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return -1; @@ -3108,14 +3069,10 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn, unsigned long xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; xen_getdomaininfo dominfo; int ret; - if (conn == NULL) - return 0; - - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return 0; @@ -3148,12 +3105,8 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) static unsigned long long ATTRIBUTE_NONNULL(1) xenHypervisorGetMaxMemory(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) - return 0; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return 0; @@ -3173,7 +3126,7 @@ xenHypervisorGetMaxMemory(virDomainPtr domain) int xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; xen_getdomaininfo dominfo; int ret; uint32_t domain_flags, domain_state, domain_shutdown_cause; @@ -3184,11 +3137,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) kb_per_pages = 4; } - if (conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || info == NULL) + if (priv->handle < 0) return -1; memset(info, 0, sizeof(virDomainInfo)); @@ -3256,14 +3205,9 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) int xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || info == NULL || - (domain->id < 0)) + if (priv->handle < 0 || domain->id < 0) return -1; return xenHypervisorGetDomInfo(domain->conn, domain->id, info); @@ -3287,14 +3231,11 @@ xenHypervisorGetDomainState(virDomainPtr domain, int *reason, unsigned int flags) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainInfo info; virCheckFlags(0, -1); - if (domain->conn == NULL) - return -1; - if (priv->handle < 0 || domain->id < 0) return -1; @@ -3326,19 +3267,13 @@ xenHypervisorGetDomainState(virDomainPtr domain, * Returns the number of entries filled in freeMems, or -1 in case of error. */ int -xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, +xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, int startCell, int maxCells) { xen_op_v2_sys op_sys; int i, j, ret; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument")); - return -1; - } - - priv = conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData; if (priv->nbNodeCells < 0) { virReportError(VIR_ERR_XEN_CALL, "%s", @@ -3400,12 +3335,8 @@ int xenHypervisorPauseDomain(virDomainPtr domain) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1; @@ -3427,12 +3358,8 @@ int xenHypervisorResumeDomain(virDomainPtr domain) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1; @@ -3459,14 +3386,10 @@ xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virCheckFlags(0, -1); - if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1; @@ -3489,12 +3412,8 @@ int xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1; @@ -3519,12 +3438,8 @@ int xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0 || nvcpus < 1) return -1; @@ -3551,14 +3466,9 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || (domain->id < 0) || - (cpumap == NULL) || (maplen < 1)) + if (priv->handle < 0 || domain->id < 0) return -1; ret = virXen_setvcpumap(priv->handle, domain->id, vcpu, @@ -3593,26 +3503,16 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, { xen_getdomaininfo dominfo; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virVcpuInfoPtr ipt; int nbinfo, i; - if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || (domain->id < 0) || - (info == NULL) || (maxinfo < 1) || - (sizeof(cpumap_t) & 7)) { + if (priv->handle < 0 || domain->id < 0 || sizeof(cpumap_t) & 7) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain shut off or invalid")); return -1; } - if ((cpumaps != NULL) && (maplen < 1)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid argument")); - return -1; - } + /* first get the number of virtual CPUs in this domain */ XEN_GETDOMAININFO_CLEAR(dominfo); ret = virXen_getdomaininfo(priv->handle, domain->id, @@ -3667,12 +3567,8 @@ xenHypervisorGetVcpuMax(virDomainPtr domain) xen_getdomaininfo dominfo; int ret; int maxcpu; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; - if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) return -1; -- 1.7.11.7

John Ferlan wrote:
Arguments for driver entry points are checked in libvirt.c, so no need to check again. --- src/xen/xen_hypervisor.c | 164 +++++++++-------------------------------------- 1 file changed, 30 insertions(+), 134 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index bfee56d..2437413 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1161,15 +1161,8 @@ char * xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams) { char *schedulertype = NULL; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return NULL; - } - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("priv->handle invalid")); @@ -1242,15 +1235,8 @@ int xenHypervisorGetSchedulerParameters(virDomainPtr domain, virTypedParameterPtr params, int *nparams) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return -1; - } - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("priv->handle invalid")); @@ -1350,12 +1336,6 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, xenUnifiedPrivatePtr priv; char buf[256];
- if (domain->conn == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("domain or conn is NULL")); - return -1; - } - if (nparams == 0) { /* nothing to do, exit early */ return 0; @@ -2252,12 +2232,7 @@ int xenHypervisorClose(virConnectPtr conn) { int ret; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
Extra space here in the cast.
if (priv->handle < 0) return -1; @@ -2282,12 +2257,9 @@ xenHypervisorClose(virConnectPtr conn) int xenHypervisorGetVersion(virConnectPtr conn, unsigned long *hvVer) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
Same here, along with some other below. ACK with those nits fixed. Regards, Jim
- if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || hvVer == NULL) + if (priv->handle < 0) return -1; *hvVer = (hv_versions.hv >> 16) * 1000000 + (hv_versions.hv & 0xFFFF) * 1000; return 0; @@ -2803,11 +2775,8 @@ xenHypervisorNumOfDomains(virConnectPtr conn) int ret, nbids; static int last_maxids = 2; int maxids = last_maxids; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return -1;
@@ -2860,14 +2829,9 @@ xenHypervisorListDomains(virConnectPtr conn, int *ids, int maxids) { xen_getdomaininfolist dominfos; int ret, nbids, i; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || - (ids == NULL) || (maxids < 0)) + if (priv->handle < 0) return -1;
if (maxids == 0) @@ -3084,11 +3048,8 @@ int xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
- if (conn == NULL) - return -1; - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return -1;
@@ -3108,14 +3069,10 @@ xenHypervisorGetMaxVcpus(virConnectPtr conn, unsigned long xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; xen_getdomaininfo dominfo; int ret;
- if (conn == NULL) - return 0; - - priv = (xenUnifiedPrivatePtr) conn->privateData; if (priv->handle < 0) return 0;
@@ -3148,12 +3105,8 @@ xenHypervisorGetDomMaxMemory(virConnectPtr conn, int id) static unsigned long long ATTRIBUTE_NONNULL(1) xenHypervisorGetMaxMemory(virDomainPtr domain) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) - return 0; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return 0;
@@ -3173,7 +3126,7 @@ xenHypervisorGetMaxMemory(virDomainPtr domain) int xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; xen_getdomaininfo dominfo; int ret; uint32_t domain_flags, domain_state, domain_shutdown_cause; @@ -3184,11 +3137,7 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) kb_per_pages = 4; }
- if (conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) conn->privateData; - if (priv->handle < 0 || info == NULL) + if (priv->handle < 0) return -1;
memset(info, 0, sizeof(virDomainInfo)); @@ -3256,14 +3205,9 @@ xenHypervisorGetDomInfo(virConnectPtr conn, int id, virDomainInfoPtr info) int xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) { - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || info == NULL || - (domain->id < 0)) + if (priv->handle < 0 || domain->id < 0) return -1;
return xenHypervisorGetDomInfo(domain->conn, domain->id, info); @@ -3287,14 +3231,11 @@ xenHypervisorGetDomainState(virDomainPtr domain, int *reason, unsigned int flags) { - xenUnifiedPrivatePtr priv = domain->conn->privateData; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virDomainInfo info;
virCheckFlags(0, -1);
- if (domain->conn == NULL) - return -1; - if (priv->handle < 0 || domain->id < 0) return -1;
@@ -3326,19 +3267,13 @@ xenHypervisorGetDomainState(virDomainPtr domain, * Returns the number of entries filled in freeMems, or -1 in case of error. */ int -xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long *freeMems, +xenHypervisorNodeGetCellsFreeMemory(virConnectPtr conn, + unsigned long long *freeMems, int startCell, int maxCells) { xen_op_v2_sys op_sys; int i, j, ret; - xenUnifiedPrivatePtr priv; - - if (conn == NULL) { - virReportError(VIR_ERR_INVALID_ARG, "%s", _("invalid argument")); - return -1; - } - - priv = conn->privateData; + xenUnifiedPrivatePtr priv = conn->privateData;
if (priv->nbNodeCells < 0) { virReportError(VIR_ERR_XEN_CALL, "%s", @@ -3400,12 +3335,8 @@ int xenHypervisorPauseDomain(virDomainPtr domain) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1;
@@ -3427,12 +3358,8 @@ int xenHypervisorResumeDomain(virDomainPtr domain) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1;
@@ -3459,14 +3386,10 @@ xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags) { int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
virCheckFlags(0, -1);
- if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1;
@@ -3489,12 +3412,8 @@ int xenHypervisorSetMaxMemory(virDomainPtr domain, unsigned long memory) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0) return -1;
@@ -3519,12 +3438,8 @@ int xenHypervisorSetVcpus(virDomainPtr domain, unsigned int nvcpus) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0 || domain->id < 0 || nvcpus < 1) return -1;
@@ -3551,14 +3466,9 @@ xenHypervisorPinVcpu(virDomainPtr domain, unsigned int vcpu, unsigned char *cpumap, int maplen) { int ret; - xenUnifiedPrivatePtr priv; - - if (domain->conn == NULL) - return -1; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || (domain->id < 0) || - (cpumap == NULL) || (maplen < 1)) + if (priv->handle < 0 || domain->id < 0) return -1;
ret = virXen_setvcpumap(priv->handle, domain->id, vcpu, @@ -3593,26 +3503,16 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, { xen_getdomaininfo dominfo; int ret; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; virVcpuInfoPtr ipt; int nbinfo, i;
- if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; - if (priv->handle < 0 || (domain->id < 0) || - (info == NULL) || (maxinfo < 1) || - (sizeof(cpumap_t) & 7)) { + if (priv->handle < 0 || domain->id < 0 || sizeof(cpumap_t) & 7) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("domain shut off or invalid")); return -1; } - if ((cpumaps != NULL) && (maplen < 1)) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("invalid argument")); - return -1; - } + /* first get the number of virtual CPUs in this domain */ XEN_GETDOMAININFO_CLEAR(dominfo); ret = virXen_getdomaininfo(priv->handle, domain->id, @@ -3667,12 +3567,8 @@ xenHypervisorGetVcpuMax(virDomainPtr domain) xen_getdomaininfo dominfo; int ret; int maxcpu; - xenUnifiedPrivatePtr priv; + xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;
- if (domain->conn == NULL) - return -1; - - priv = (xenUnifiedPrivatePtr) domain->conn->privateData; if (priv->handle < 0) return -1;

Clean up some function headers --- src/xen/xen_hypervisor.c | 46 +++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 29 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 2437413..186f0c7 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1,7 +1,7 @@ /* * xen_internal.c: direct access to Xen hypervisor level * - * Copyright (C) 2005-2012 Red Hat, Inc. + * Copyright (C) 2005-2013 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 @@ -1134,8 +1134,8 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids, } static int -virXen_getdomaininfo(int handle, int first_domain, - xen_getdomaininfo *dominfo) { +virXen_getdomaininfo(int handle, int first_domain, xen_getdomaininfo *dominfo) +{ xen_getdomaininfolist dominfos; if (hv_versions.hypervisor < 2) { @@ -1442,8 +1442,7 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain, int -xenHypervisorDomainBlockStats(virDomainPtr dom, - const char *path, +xenHypervisorDomainBlockStats(virDomainPtr dom, const char *path, struct _virDomainBlockStats *stats) { #ifdef __linux__ @@ -1471,8 +1470,7 @@ xenHypervisorDomainBlockStats(virDomainPtr dom, * virNetwork interface, as yet not decided. */ int -xenHypervisorDomainInterfaceStats(virDomainPtr dom, - const char *path, +xenHypervisorDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { #ifdef __linux__ @@ -2195,8 +2193,7 @@ VIR_ONCE_GLOBAL_INIT(xenHypervisor) * Returns 0 or -1 in case of error. */ virDrvOpenStatus -xenHypervisorOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenHypervisorOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { int ret; @@ -2284,12 +2281,11 @@ static int xenDefaultConsoleType(const char *ostype, } static virCapsPtr -xenHypervisorBuildCapabilities(virConnectPtr conn, - virArch hostarch, - int host_pae, - const char *hvm_type, +xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, + int host_pae, const char *hvm_type, struct guest_arch *guest_archs, - int nr_guest_archs) { + int nr_guest_archs) +{ virCapsPtr caps; int i; int hv_major = hv_versions.hv >> 16; @@ -2537,8 +2533,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) * Return the capabilities of this hypervisor. */ virCapsPtr -xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, - virArch hostarch, +xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, virArch hostarch, FILE *cpuinfo, FILE *capabilities) { char line[1024], *str, *token; @@ -2915,8 +2910,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom) } int -xenHypervisorHasDomain(virConnectPtr conn, - int id) +xenHypervisorHasDomain(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv; xen_getdomaininfo dominfo; @@ -2937,8 +2931,7 @@ xenHypervisorHasDomain(virConnectPtr conn, } virDomainPtr -xenHypervisorLookupDomainByID(virConnectPtr conn, - int id) +xenHypervisorLookupDomainByID(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv; xen_getdomaininfo dominfo; @@ -2972,8 +2965,7 @@ xenHypervisorLookupDomainByID(virConnectPtr conn, virDomainPtr -xenHypervisorLookupDomainByUUID(virConnectPtr conn, - const unsigned char *uuid) +xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { xen_getdomaininfolist dominfos; xenUnifiedPrivatePtr priv; @@ -3045,8 +3037,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, * Returns the maximum of CPU defined by Xen. */ int -xenHypervisorGetMaxVcpus(virConnectPtr conn, - const char *type ATTRIBUTE_UNUSED) +xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData; @@ -3226,9 +3217,7 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) * Returns 0 in case of success, -1 in case of error. */ int -xenHypervisorGetDomainState(virDomainPtr domain, - int *state, - int *reason, +xenHypervisorGetDomainState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; @@ -3382,8 +3371,7 @@ xenHypervisorResumeDomain(virDomainPtr domain) * Returns 0 in case of success, -1 in case of error. */ int -xenHypervisorDestroyDomainFlags(virDomainPtr domain, - unsigned int flags) +xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags) { int ret; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; -- 1.7.11.7

John Ferlan wrote:
Clean up some function headers --- src/xen/xen_hypervisor.c | 46 +++++++++++++++++----------------------------- 1 file changed, 17 insertions(+), 29 deletions(-)
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 2437413..186f0c7 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1,7 +1,7 @@ /* * xen_internal.c: direct access to Xen hypervisor level * - * Copyright (C) 2005-2012 Red Hat, Inc. + * Copyright (C) 2005-2013 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 @@ -1134,8 +1134,8 @@ virXen_getdomaininfolist(int handle, int first_domain, int maxids, }
static int -virXen_getdomaininfo(int handle, int first_domain, - xen_getdomaininfo *dominfo) { +virXen_getdomaininfo(int handle, int first_domain, xen_getdomaininfo *dominfo) +{
More of the same. I personally prefer one line when it fits. Either way, ACK to moving opening braces to a new line. Regards, Jim
xen_getdomaininfolist dominfos;
if (hv_versions.hypervisor < 2) { @@ -1442,8 +1442,7 @@ xenHypervisorSetSchedulerParameters(virDomainPtr domain,
int -xenHypervisorDomainBlockStats(virDomainPtr dom, - const char *path, +xenHypervisorDomainBlockStats(virDomainPtr dom, const char *path, struct _virDomainBlockStats *stats) { #ifdef __linux__ @@ -1471,8 +1470,7 @@ xenHypervisorDomainBlockStats(virDomainPtr dom, * virNetwork interface, as yet not decided. */ int -xenHypervisorDomainInterfaceStats(virDomainPtr dom, - const char *path, +xenHypervisorDomainInterfaceStats(virDomainPtr dom, const char *path, struct _virDomainInterfaceStats *stats) { #ifdef __linux__ @@ -2195,8 +2193,7 @@ VIR_ONCE_GLOBAL_INIT(xenHypervisor) * Returns 0 or -1 in case of error. */ virDrvOpenStatus -xenHypervisorOpen(virConnectPtr conn, - virConnectAuthPtr auth ATTRIBUTE_UNUSED, +xenHypervisorOpen(virConnectPtr conn, virConnectAuthPtr auth ATTRIBUTE_UNUSED, unsigned int flags) { int ret; @@ -2284,12 +2281,11 @@ static int xenDefaultConsoleType(const char *ostype, }
static virCapsPtr -xenHypervisorBuildCapabilities(virConnectPtr conn, - virArch hostarch, - int host_pae, - const char *hvm_type, +xenHypervisorBuildCapabilities(virConnectPtr conn, virArch hostarch, + int host_pae, const char *hvm_type, struct guest_arch *guest_archs, - int nr_guest_archs) { + int nr_guest_archs) +{ virCapsPtr caps; int i; int hv_major = hv_versions.hv >> 16; @@ -2537,8 +2533,7 @@ xenHypervisorMakeCapabilitiesSunOS(virConnectPtr conn) * Return the capabilities of this hypervisor. */ virCapsPtr -xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, - virArch hostarch, +xenHypervisorMakeCapabilitiesInternal(virConnectPtr conn, virArch hostarch, FILE *cpuinfo, FILE *capabilities) { char line[1024], *str, *token; @@ -2915,8 +2910,7 @@ xenHypervisorDomainGetOSType(virDomainPtr dom) }
int -xenHypervisorHasDomain(virConnectPtr conn, - int id) +xenHypervisorHasDomain(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv; xen_getdomaininfo dominfo; @@ -2937,8 +2931,7 @@ xenHypervisorHasDomain(virConnectPtr conn, }
virDomainPtr -xenHypervisorLookupDomainByID(virConnectPtr conn, - int id) +xenHypervisorLookupDomainByID(virConnectPtr conn, int id) { xenUnifiedPrivatePtr priv; xen_getdomaininfo dominfo; @@ -2972,8 +2965,7 @@ xenHypervisorLookupDomainByID(virConnectPtr conn,
virDomainPtr -xenHypervisorLookupDomainByUUID(virConnectPtr conn, - const unsigned char *uuid) +xenHypervisorLookupDomainByUUID(virConnectPtr conn, const unsigned char *uuid) { xen_getdomaininfolist dominfos; xenUnifiedPrivatePtr priv; @@ -3045,8 +3037,7 @@ xenHypervisorLookupDomainByUUID(virConnectPtr conn, * Returns the maximum of CPU defined by Xen. */ int -xenHypervisorGetMaxVcpus(virConnectPtr conn, - const char *type ATTRIBUTE_UNUSED) +xenHypervisorGetMaxVcpus(virConnectPtr conn, const char *type ATTRIBUTE_UNUSED) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr) conn->privateData;
@@ -3226,9 +3217,7 @@ xenHypervisorGetDomainInfo(virDomainPtr domain, virDomainInfoPtr info) * Returns 0 in case of success, -1 in case of error. */ int -xenHypervisorGetDomainState(virDomainPtr domain, - int *state, - int *reason, +xenHypervisorGetDomainState(virDomainPtr domain, int *state, int *reason, unsigned int flags) { xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData; @@ -3382,8 +3371,7 @@ xenHypervisorResumeDomain(virDomainPtr domain) * Returns 0 in case of success, -1 in case of error. */ int -xenHypervisorDestroyDomainFlags(virDomainPtr domain, - unsigned int flags) +xenHypervisorDestroyDomainFlags(virDomainPtr domain, unsigned int flags) { int ret; xenUnifiedPrivatePtr priv = (xenUnifiedPrivatePtr)domain->conn->privateData;

Turns out the issue regarding ptr_arith and sign_exension weren't false positives. When shifting an 'unsigned char' as a target, it gets promoted to an 'int'; however, that 'int' cannot be shifted 32 bits which was how the algorithm was written. For the ptr_arith rather than index into the cpumap, change the to address as necessary and assign directly. --- src/xen/xen_hypervisor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 186f0c7..3ea70a2 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1766,17 +1766,17 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu, ret = -1; } else { cpumap_t xen_cpumap; /* limited to 64 CPUs in old hypervisors */ - uint64_t *pm = &xen_cpumap; + uint64_t *pm; int j; if ((maplen > (int)sizeof(cpumap_t)) || (sizeof(cpumap_t) & 7)) return -1; - memset(pm, 0, sizeof(cpumap_t)); + memset(&xen_cpumap, 0, sizeof(cpumap_t)); for (j = 0; j < maplen; j++) { - /* coverity[ptr_arith] */ - /* coverity[sign_extension] */ - *(pm + (j / 8)) |= cpumap[j] << (8 * (j & 7)); + if ((j & 7) == 0) + pm = (uint64_t *)((uint64_t)&xen_cpumap + (j & ~0x7UL)); + *pm |= (uint64_t)cpumap[j] << (8 * (j & 7)); } if (hv_versions.hypervisor == 1) { -- 1.7.11.7

John Ferlan wrote:
Turns out the issue regarding ptr_arith and sign_exension weren't false positives. When shifting an 'unsigned char' as a target, it gets promoted to an 'int'; however, that 'int' cannot be shifted 32 bits which was how the algorithm was written. For the ptr_arith rather than index into the cpumap, change the to address as necessary and assign directly. --- src/xen/xen_hypervisor.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
IIRC, you and Eric have been discussing this change. I hope Eric can take a look since he has superior knowledge here :). Regards, Jim
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 186f0c7..3ea70a2 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1766,17 +1766,17 @@ virXen_setvcpumap(int handle, int id, unsigned int vcpu, ret = -1; } else { cpumap_t xen_cpumap; /* limited to 64 CPUs in old hypervisors */ - uint64_t *pm = &xen_cpumap; + uint64_t *pm; int j;
if ((maplen > (int)sizeof(cpumap_t)) || (sizeof(cpumap_t) & 7)) return -1;
- memset(pm, 0, sizeof(cpumap_t)); + memset(&xen_cpumap, 0, sizeof(cpumap_t)); for (j = 0; j < maplen; j++) { - /* coverity[ptr_arith] */ - /* coverity[sign_extension] */ - *(pm + (j / 8)) |= cpumap[j] << (8 * (j & 7)); + if ((j & 7) == 0) + pm = (uint64_t *)((uint64_t)&xen_cpumap + (j & ~0x7UL)); + *pm |= (uint64_t)cpumap[j] << (8 * (j & 7)); }
if (hv_versions.hypervisor == 1) {

On 01/30/2013 11:51 AM, John Ferlan wrote:
Turns out the issue regarding ptr_arith and sign_exension weren't false positives. When shifting an 'unsigned char' as a target, it gets promoted to an 'int'; however, that 'int' cannot be shifted 32 bits which was how the algorithm was written. For the ptr_arith rather than index into the cpumap, change the to address as necessary and assign directly. --- src/xen/xen_hypervisor.c | 10 +++++-----
for (j = 0; j < maplen; j++) { - /* coverity[ptr_arith] */ - /* coverity[sign_extension] */ - *(pm + (j / 8)) |= cpumap[j] << (8 * (j & 7)); + if ((j & 7) == 0) + pm = (uint64_t *)((uint64_t)&xen_cpumap + (j & ~0x7UL));
I'm not happy with how this turned out. We are turning an address into a pointer but not via intptr_t (probably warnings on mingw); then doing math on that pointer, then turning the result back into a uint64_t pointer. It looks like we are risking alignment errors.
+ *pm |= (uint64_t)cpumap[j] << (8 * (j & 7));
This part looks better, although I can see why you had to push a followup to silence a false negative compiler warning about pm potentially being used uninitialized. I still think we can do a better job; the whole goal of this code is to write an endian-specific ordering of a multiple of 8 bytes; I can't help but wonder if my concurrent work on a new virendian.h header would be a nicer-looking solution here. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 02/11/2013 10:24 AM, Eric Blake wrote:
On 01/30/2013 11:51 AM, John Ferlan wrote:
Turns out the issue regarding ptr_arith and sign_exension weren't false positives. When shifting an 'unsigned char' as a target, it gets promoted to an 'int'; however, that 'int' cannot be shifted 32 bits which was how the algorithm was written. For the ptr_arith rather than index into the cpumap, change the to address as necessary and assign directly. --- src/xen/xen_hypervisor.c | 10 +++++-----
for (j = 0; j < maplen; j++) { - /* coverity[ptr_arith] */ - /* coverity[sign_extension] */ - *(pm + (j / 8)) |= cpumap[j] << (8 * (j & 7)); + if ((j & 7) == 0) + pm = (uint64_t *)((uint64_t)&xen_cpumap + (j & ~0x7UL));
I'm not happy with how this turned out. We are turning an address into a pointer but not via intptr_t (probably warnings on mingw); then doing math on that pointer, then turning the result back into a uint64_t pointer. It looks like we are risking alignment errors.
+ *pm |= (uint64_t)cpumap[j] << (8 * (j & 7));
More research - I think this is highly over-engineered, which explains why we got it so wrong. I checked a full range of xen-devel, from RHEL 5 (3.0.3, about as old as we support out-of-the-box with libvirt.git) to Fedora 18 (4.2.1); ALL of those versions had: /usr/include/xen/dom0_ops.h: typedef uint64_t cpumap_t; which means this type has (more or less) _always_ been exactly 64 bits; the code that tries to allow for iterating through 16 bytes instead of 8 is over-engineered. Really, _ALL_ we are doing is reading a little-endian 64-bit number in from the user (possibly in unaligned memory), and turning it into a host-endian cpumap_t to hand to xen. I should have a patch soon. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 01/30/2013 01:51 PM, John Ferlan wrote:
src/xen/xen_hypervisor.c | 220 ++++++++-------------------------- src/xen/xen_inotify.c | 48 ++++---- src/xen/xend_internal.c | 303 ++++++++++++++--------------------------------- src/xen/xm_internal.c | 195 ++++++++++++++---------------- src/xen/xs_internal.c | 239 +++++++++++-------------------------- 5 files changed, 325 insertions(+), 680 deletions(-)
Thanks for the reviews Osier and Jim. Rather than respond to each, I'll summarize the consistent points. If I should send a v2, let me know. w/r/t PrivatePtr on one line: I was trying to go with the 80 column rule. For 99% they could fit on one line if I removed one extra space. For a couple, spanning 2 lines kept the 80 columns in effect. In looking at other drivers - I don't think any of them typecast the *PrivatePtr)<var>->privateData, so I could remove that too. I left them in only because they were there. w/r/t function headers: I was trying to be consistent amongst all the files and then more in general with other drivers. I kept to the concept that if a header fits within 80 columns then don't break up the line. I changed the files where a header wouldn't fit into 80 columns to be: function(type arg, type2 arg2, type3 arg3, type4 arg4) { } w/r/t: domain->name: I wasn't sure with this one, so I had left it in. I removed them. There are also checks for "domain->id != -1" - any thoughts on those? Those seem to be running or not checks, right? Seems as though the virDomainObjIsActive() or hvirCheckFlags() is used elsewhere. What about priv->handle in xen_hypervisor.c? Seems as though open will set it and everyone else checks it. Given how these drivers call between each other, I'm inclined to leave as it, but I'm not against removing them either. John

John Ferlan wrote:
On 01/30/2013 01:51 PM, John Ferlan wrote:
src/xen/xen_hypervisor.c | 220 ++++++++-------------------------- src/xen/xen_inotify.c | 48 ++++---- src/xen/xend_internal.c | 303 ++++++++++++++--------------------------------- src/xen/xm_internal.c | 195 ++++++++++++++---------------- src/xen/xs_internal.c | 239 +++++++++++-------------------------- 5 files changed, 325 insertions(+), 680 deletions(-)
Thanks for the reviews Osier and Jim. Rather than respond to each, I'll summarize the consistent points. If I should send a v2, let me know.
w/r/t PrivatePtr on one line:
I was trying to go with the 80 column rule. For 99% they could fit on one line if I removed one extra space. For a couple, spanning 2 lines kept the 80 columns in effect. In looking at other drivers - I don't think any of them typecast the *PrivatePtr)<var>->privateData, so I could remove that too. I left them in only because they were there.
Yeah, good point. And dropping the cast will make all of them fit within 80 columns :).
w/r/t function headers:
I was trying to be consistent amongst all the files and then more in general with other drivers. I kept to the concept that if a header fits within 80 columns then don't break up the line. I changed the files where a header wouldn't fit into 80 columns to be:
function(type arg, type2 arg2, type3 arg3, type4 arg4) { }
I like the approach and will strive to use that style going forward.
w/r/t: domain->name:
I wasn't sure with this one, so I had left it in. I removed them.
Sounds good.
There are also checks for "domain->id != -1" - any thoughts on those? Those seem to be running or not checks, right? Seems as though the virDomainObjIsActive() or hvirCheckFlags() is used elsewhere.
Looks like those checks are all done on a virDomainPtr. Probably best to leave them as is instead of getting a virDomainObjPtr solely for use with virDomainObjIsActive.
What about priv->handle in xen_hypervisor.c? Seems as though open will set it and everyone else checks it. Given how these drivers call between each other, I'm inclined to leave as it, but I'm not against removing them either.
I tend to agree with leaving as is. In practice, the hypervisor subdriver is always opened, and if it fails loading the whole xen driver fails. But in theory opening it could be skipped since it is conditional on xenHavePrivilege(). Regards, Jim

On 02/08/2013 04:15 PM, Jim Fehlig wrote:
I was trying to go with the 80 column rule. For 99% they could fit on one line if I removed one extra space. For a couple, spanning 2 lines kept the 80 columns in effect. In looking at other drivers - I don't think any of them typecast the *PrivatePtr)<var>->privateData, so I could remove that too. I left them in only because they were there.
Yeah, good point. And dropping the cast will make all of them fit within 80 columns :).
The cast is needed in C++; but libvirt is C, and void* automatically casts to whatever type you need without having to be explicit. Go ahead and drop the cast. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (4)
-
Eric Blake
-
Jim Fehlig
-
John Ferlan
-
Osier Yang