[PATCH] Re: [Libvir] Segfault with invalid virConnectPtr
Daniel Veillard
veillard at redhat.com
Thu Sep 13 06:59:45 UTC 2007
On Wed, Sep 12, 2007 at 12:04:45PM +0100, Richard W.M. Jones wrote:
> Richard W.M. Jones wrote:
> >Program terminated with signal 11, Segmentation fault.
> >#0 0x0000003d8b472a1b in free () from /lib64/libc.so.6
> >(gdb) bt
> >#0 0x0000003d8b472a1b in free () from /lib64/libc.so.6
> >#1 0x00002aaaaaae8dd7 in virResetError (err=0x33535c8) at virterror.c:111
> >#2 0x00002aaaaaae8fce in __virRaiseError (conn=0x33535a0, dom=0x0,
> >net=0x0,
> > domain=0, code=6, level=VIR_ERR_ERROR,
> > str1=0x2aaaaab0c678 "invalid connection pointer in %s",
> > str2=0x2aaaaab08560 "virConnectNumOfDomains", str3=0x0, int1=0, int2=0,
> > msg=0x2aaaaab0c678 "invalid connection pointer in %s") at
> >virterror.c:358
> >#3 0x00002aaaaaacfa8e in virLibConnError (conn=0x33535a0,
> > error=VIR_ERR_INVALID_CONN, info=0x2aaaaab08560
> >"virConnectNumOfDomains")
> > at libvirt.c:127
> >#4 0x00002aaaaaad1052 in virConnectNumOfDomains (conn=0x736e6961)
> > at libvirt.c:758
> >#5 0x000000000043fa4e in ?? ()
> >
> >
> >A preliminary look at the code seems to indicate a fault in this logic:
> >
> >int
> >virConnectNumOfDomains(virConnectPtr conn)
> >{
> > DEBUG("conn=%p", conn);
> >
> > if (!VIR_IS_CONNECT(conn)) {
> > virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> >
> >The VIR_IS_CONNECT macro is defined as:
> >
> >#define VIR_CONNECT_MAGIC 0x4F23DEAD
> >#define VIR_IS_CONNECT(obj) ((obj) && (obj)->magic==VIR_CONNECT_MAGIC)
> >
> >Obviously if VIR_IS_CONNECT fails then "conn" should not be used
> >further, so calling virLibConnError (conn, ...) is wrong. Personally I
> >think when we detect memory corruption in a C program we should just
> >call abort().
> >
> >I'll see if I can come up with a patch to fix this later ... at the
> >moment I'm more interested in why my program is passing an invalid
> >connection pointer in the first place :-(
>
> So a couple of things to say about this.
>
> Firstly attached is a patch which fixes the libvirt problem, by passing
> NULL when we find that a connection is invalid, rather than passing the
> known invalid connection to virLibConnError. (It also fixes the same
> problem with virDomainPtr and virNetworkPtr objects).
>
> Secondly I worked out why my program was passing an invalid connection
> pointer, and it may act as a salutary lesson for anyone writing libvirt
> bindings for a language which has real garbage collection (hello, Java).
>
> In the normal case I wrap objects like virConnectPtr, virDomainPtr in an
> OCaml object which has an attached finaliser. Thus the OCaml user
> doesn't need to worry about manually freeing these objects - the garbage
> collector will collect them when they become unreachable and the
> finaliser will call virConnectClose, virDomainFree, etc. as appropriate.
>
> The problem I was hitting was when a libvirt function raised an error
> (ie. virterror), which contains a virConnectPtr, virDomainPtr and
> virNetworkPtr. I was using my normal wrapping functions to attach
> finalisers, but that was the wrong thing to do in this case because
> virterror does not increment the reference counts of these objects.
> When my OCaml virterror exception was garbage collected, that would
> cause virConnectClose in this case to be called, but that closed the
> connection that I was still using, resulting in the main program
> continuing to use a closed virConnectPtr.
>
> Note that the same double-free or invalid pointer use could also happen
> with the domain pointer or the network pointer in the virterror. It was
> just my good luck that it happened to be the connection pointer that got
> closed first which made the error rather more obvious and easier to find.
>
> The partial solution on the OCaml side is to use a special wrapper
> function for the virterror objects which doesn't attach a finaliser.
>
> This is not a complete solution because an OCaml program might save the
> exception and use it later, after the virConnectPtr, virDomainPtr or
> virNetworkPtr that it contains have been freed elsewhere. So the OCaml
> program becomes unsafe, and we have to tell users not to save these
> exceptions. Unfortunately there is no good solution on the C side
> either: if we change virterror so it increments the reference counts
> then any C program which uses virterror will have to be changed.
>
> (The above is fixed in ocaml-libvirt bindings >= 0.3.2.6).
>
> Rich.
>
> --
> Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
> Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
> Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
> England and Wales under Company Registration No. 03798903
> Index: src/libvirt.c
> ===================================================================
> RCS file: /data/cvs/libvirt/src/libvirt.c,v
> retrieving revision 1.98
> diff -u -p -r1.98 libvirt.c
> --- src/libvirt.c 10 Sep 2007 09:37:10 -0000 1.98
> +++ src/libvirt.c 12 Sep 2007 11:04:19 -0000
> @@ -573,7 +573,7 @@ virConnectGetType(virConnectPtr conn)
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
>
> @@ -603,7 +603,7 @@ virConnectGetVersion(virConnectPtr conn,
> DEBUG("conn=%p, hvVer=%p", conn, hvVer);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -637,7 +637,7 @@ virConnectGetHostname (virConnectPtr con
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return NULL;
> }
>
> @@ -669,7 +669,7 @@ virConnectGetURI (virConnectPtr conn)
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return NULL;
> }
>
> @@ -698,7 +698,7 @@ virConnectGetMaxVcpus(virConnectPtr conn
> DEBUG("conn=%p, type=%s", conn, type);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -725,7 +725,7 @@ virConnectListDomains(virConnectPtr conn
> DEBUG("conn=%p, ids=%p, maxids=%d", conn, ids, maxids);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -755,7 +755,7 @@ virConnectNumOfDomains(virConnectPtr con
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
yes those changes make sense.
> @@ -786,7 +786,7 @@ virDomainGetConnect (virDomainPtr dom)
> DEBUG("dom=%p", dom);
>
> if (!VIR_IS_DOMAIN (dom)) {
> - virLibDomainError (dom, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError (NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return NULL;
> }
> return dom->conn;
unclear, if there is no way the conn can be extracted, then yes,
maybe that's true.
> @@ -811,7 +811,7 @@ virDomainCreateLinux(virConnectPtr conn,
> DEBUG("conn=%p, xmlDesc=%s, flags=%d", conn, xmlDesc, flags);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (xmlDesc == NULL) {
> @@ -847,7 +847,7 @@ virDomainLookupByID(virConnectPtr conn,
> DEBUG("conn=%p, id=%d", conn, id);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (id < 0) {
> @@ -878,7 +878,7 @@ virDomainLookupByUUID(virConnectPtr conn
> DEBUG("conn=%p, uuid=%s", conn, uuid);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (uuid == NULL) {
> @@ -913,7 +913,7 @@ virDomainLookupByUUIDString(virConnectPt
> DEBUG("conn=%p, uuidstr=%s", conn, uuidstr);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (uuidstr == NULL) {
> @@ -961,7 +961,7 @@ virDomainLookupByName(virConnectPtr conn
> DEBUG("conn=%p, name=%s", conn, name);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (name == NULL) {
> @@ -996,7 +996,7 @@ virDomainDestroy(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
>
> @@ -1028,7 +1028,7 @@ virDomainFree(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (virFreeDomain(domain->conn, domain) < 0)
> @@ -1055,7 +1055,7 @@ virDomainSuspend(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1089,7 +1089,7 @@ virDomainResume(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1126,7 +1126,7 @@ virDomainSave(virDomainPtr domain, const
> DEBUG("domain=%p, to=%s", domain, to);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1182,7 +1182,7 @@ virDomainRestore(virConnectPtr conn, con
> DEBUG("conn=%p, from=%s", conn, from);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
> if (conn->flags & VIR_CONNECT_RO) {
> @@ -1240,7 +1240,7 @@ virDomainCoreDump(virDomainPtr domain, c
> DEBUG("domain=%p, to=%s, flags=%d", domain, to, flags);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1300,7 +1300,7 @@ virDomainShutdown(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1335,7 +1335,7 @@ virDomainReboot(virDomainPtr domain, uns
> DEBUG("domain=%p, flags=%u", domain, flags);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1367,7 +1367,7 @@ virDomainGetName(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (NULL);
> }
> return (domain->name);
> @@ -1388,7 +1388,7 @@ virDomainGetUUID(virDomainPtr domain, un
> DEBUG("domain=%p, uuid=%p", domain, uuid);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (uuid == NULL) {
> @@ -1421,7 +1421,7 @@ virDomainGetUUIDString(virDomainPtr doma
> DEBUG("domain=%p, buf=%p", domain, buf);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (buf == NULL) {
> @@ -1450,7 +1450,7 @@ virDomainGetID(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return ((unsigned int) -1);
> }
> return (domain->id);
> @@ -1472,7 +1472,7 @@ virDomainGetOSType(virDomainPtr domain)
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (NULL);
> }
>
> @@ -1502,7 +1502,7 @@ virDomainGetMaxMemory(virDomainPtr domai
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (0);
> }
>
> @@ -1538,7 +1538,7 @@ virDomainSetMaxMemory(virDomainPtr domai
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1581,7 +1581,7 @@ virDomainSetMemory(virDomainPtr domain,
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -1620,7 +1620,7 @@ virDomainGetInfo(virDomainPtr domain, vi
> DEBUG("domain=%p, info=%p", domain, info);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (info == NULL) {
> @@ -1657,7 +1657,7 @@ virDomainGetXMLDesc(virDomainPtr domain,
> DEBUG("domain=%p, flags=%d", domain, flags);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (NULL);
> }
> if (flags != 0) {
> @@ -1739,7 +1739,7 @@ virDomainMigrate (virDomainPtr domain,
> domain, dconn, flags, dname, uri, bandwidth);
>
> if (!VIR_IS_DOMAIN (domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return NULL;
> }
> conn = domain->conn; /* Source connection. */
> @@ -1821,7 +1821,7 @@ __virDomainMigratePrepare (virConnectPtr
> DEBUG("dconn=%p, cookie=%p, cookielen=%p, uri_in=%s, uri_out=%p, flags=%lu, dname=%s, bandwidth=%lu", dconn, cookie, cookielen, uri_in, uri_out, flags, dname, bandwidth);
>
> if (!VIR_IS_CONNECT (dconn)) {
> - virLibConnError (dconn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return -1;
> }
>
> @@ -1850,7 +1850,7 @@ __virDomainMigratePerform (virDomainPtr
> DEBUG("domain=%p, cookie=%p, cookielen=%d, uri=%s, flags=%lu, dname=%s, bandwidth=%lu", domain, cookie, cookielen, uri, flags, dname, bandwidth);
>
> if (!VIR_IS_DOMAIN (domain)) {
> - virLibDomainError (domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError (NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return -1;
> }
> conn = domain->conn;
> @@ -1878,7 +1878,7 @@ __virDomainMigrateFinish (virConnectPtr
> DEBUG("dconn=%p, dname=%s, cookie=%p, cookielen=%d, uri=%s, flags=%lu", dconn, dname, cookie, cookielen, uri, flags);
>
> if (!VIR_IS_CONNECT (dconn)) {
> - virLibConnError (dconn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return NULL;
> }
>
> @@ -1907,7 +1907,7 @@ virNodeGetInfo(virConnectPtr conn, virNo
> DEBUG("conn=%p, info=%p", conn, info);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
> if (info == NULL) {
> @@ -1938,7 +1938,7 @@ virConnectGetCapabilities (virConnectPtr
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT (conn)) {
> - virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError (NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return NULL;
> }
>
> @@ -1966,16 +1966,11 @@ virDomainGetSchedulerType(virDomainPtr d
> DEBUG("domain=%p, nparams=%p", domain, nparams);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return NULL;
> }
> conn = domain->conn;
>
> - if (!VIR_IS_CONNECT (conn)) {
> - virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> - return NULL;
> - }
> -
> if (conn->driver->domainGetSchedulerType){
> schedtype = conn->driver->domainGetSchedulerType (domain, nparams);
> return schedtype;
> @@ -2008,16 +2003,11 @@ virDomainGetSchedulerParameters(virDomai
> DEBUG("domain=%p, params=%p, nparams=%p", domain, params, nparams);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return -1;
> }
> conn = domain->conn;
>
> - if (!VIR_IS_CONNECT (conn)) {
> - virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> - return -1;
> - }
> -
> if (conn->driver->domainGetSchedulerParameters)
> return conn->driver->domainGetSchedulerParameters (domain, params, nparams);
>
> @@ -2045,16 +2035,11 @@ virDomainSetSchedulerParameters(virDomai
> DEBUG("domain=%p, params=%p, nparams=%d", domain, params, nparams);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return -1;
> }
> conn = domain->conn;
>
> - if (!VIR_IS_CONNECT (conn)) {
> - virLibConnError (conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> - return -1;
> - }
> -
> if (conn->driver->domainSetSchedulerParameters)
> return conn->driver->domainSetSchedulerParameters (domain, params, nparams);
>
> @@ -2099,7 +2084,7 @@ virDomainBlockStats (virDomainPtr dom, c
> return -1;
> }
> if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
> - virLibDomainError (dom, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError (NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return -1;
> }
> conn = dom->conn;
> @@ -2151,7 +2136,7 @@ virDomainInterfaceStats (virDomainPtr do
> return -1;
> }
> if (!VIR_IS_CONNECTED_DOMAIN (dom)) {
> - virLibDomainError (dom, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError (NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return -1;
> }
> conn = dom->conn;
> @@ -2191,7 +2176,7 @@ virDomainDefineXML(virConnectPtr conn, c
> DEBUG("conn=%p, xml=%s", conn, xml);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (conn->flags & VIR_CONNECT_RO) {
> @@ -2224,7 +2209,7 @@ virDomainUndefine(virDomainPtr domain) {
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> conn = domain->conn;
> @@ -2254,7 +2239,7 @@ virConnectNumOfDefinedDomains(virConnect
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2281,7 +2266,7 @@ virConnectListDefinedDomains(virConnectP
> DEBUG("conn=%p, names=%p, maxnames=%d", conn, names, maxnames);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2316,7 +2301,7 @@ virDomainCreate(virDomainPtr domain) {
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> conn = domain->conn;
> @@ -2351,7 +2336,7 @@ virDomainGetAutostart(virDomainPtr domai
> DEBUG("domain=%p, autostart=%p", domain, autostart);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (!autostart) {
> @@ -2386,7 +2371,7 @@ virDomainSetAutostart(virDomainPtr domai
> DEBUG("domain=%p, autostart=%d", domain, autostart);
>
> if (!VIR_IS_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2423,7 +2408,7 @@ virDomainSetVcpus(virDomainPtr domain, u
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -2474,7 +2459,7 @@ virDomainPinVcpu(virDomainPtr domain, un
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -2529,7 +2514,7 @@ virDomainGetVcpus(virDomainPtr domain, v
> return (-1);
> }
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if ((info == NULL) || (maxinfo < 1)) {
> @@ -2570,7 +2555,7 @@ virDomainGetMaxVcpus(virDomainPtr domain
> DEBUG("domain=%p", domain);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2600,7 +2585,7 @@ virDomainAttachDevice(virDomainPtr domai
> DEBUG("domain=%p, xml=%s", domain, xml);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -2632,7 +2617,7 @@ virDomainDetachDevice(virDomainPtr domai
> DEBUG("domain=%p, xml=%s", domain, xml);
>
> if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
> - virLibDomainError(domain, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> + virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
> return (-1);
> }
> if (domain->conn->flags & VIR_CONNECT_RO) {
> @@ -2668,7 +2653,7 @@ virNetworkGetConnect (virNetworkPtr net)
> DEBUG("net=%p", net);
>
> if (!VIR_IS_NETWORK (net)) {
> - virLibNetworkError (net, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError (NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return NULL;
> }
> return net->conn;
> @@ -2688,7 +2673,7 @@ virConnectNumOfNetworks(virConnectPtr co
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2715,7 +2700,7 @@ virConnectListNetworks(virConnectPtr con
> DEBUG("conn=%p, names=%p, maxnames=%d", conn, names, maxnames);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2745,7 +2730,7 @@ virConnectNumOfDefinedNetworks(virConnec
> DEBUG("conn=%p", conn);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2773,7 +2758,7 @@ virConnectListDefinedNetworks(virConnect
> DEBUG("conn=%p, names=%p, maxnames=%d", conn, names, maxnames);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (-1);
> }
>
> @@ -2806,7 +2791,7 @@ virNetworkLookupByName(virConnectPtr con
> DEBUG("conn=%p, name=%s", conn, name);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (name == NULL) {
> @@ -2837,7 +2822,7 @@ virNetworkLookupByUUID(virConnectPtr con
> DEBUG("conn=%p, uuid=%s", conn, uuid);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (uuid == NULL) {
> @@ -2871,7 +2856,7 @@ virNetworkLookupByUUIDString(virConnectP
> DEBUG("conn=%p, uuidstr=%s", conn, uuidstr);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (uuidstr == NULL) {
> @@ -2919,7 +2904,7 @@ virNetworkCreateXML(virConnectPtr conn,
> DEBUG("conn=%p, xmlDesc=%s", conn, xmlDesc);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (xmlDesc == NULL) {
> @@ -2953,7 +2938,7 @@ virNetworkDefineXML(virConnectPtr conn,
> DEBUG("conn=%p, xml=%s", conn, xml);
>
> if (!VIR_IS_CONNECT(conn)) {
> - virLibConnError(conn, VIR_ERR_INVALID_CONN, __FUNCTION__);
> + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
> return (NULL);
> }
> if (conn->flags & VIR_CONNECT_RO) {
> @@ -2986,7 +2971,7 @@ virNetworkUndefine(virNetworkPtr network
> DEBUG("network=%p", network);
>
> if (!VIR_IS_CONNECTED_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> conn = network->conn;
> @@ -3022,7 +3007,7 @@ virNetworkCreate(virNetworkPtr network)
> return (-1);
> }
> if (!VIR_IS_CONNECTED_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> conn = network->conn;
> @@ -3057,7 +3042,7 @@ virNetworkDestroy(virNetworkPtr network)
> DEBUG("network=%p", network);
>
> if (!VIR_IS_CONNECTED_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
>
> @@ -3089,7 +3074,7 @@ virNetworkFree(virNetworkPtr network)
> DEBUG("network=%p", network);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> if (virFreeNetwork(network->conn, network) < 0)
> @@ -3112,7 +3097,7 @@ virNetworkGetName(virNetworkPtr network)
> DEBUG("network=%p", network);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (NULL);
> }
> return (network->name);
> @@ -3133,7 +3118,7 @@ virNetworkGetUUID(virNetworkPtr network,
> DEBUG("network=%p, uuid=%p", network, uuid);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> if (uuid == NULL) {
> @@ -3163,7 +3148,7 @@ virNetworkGetUUIDString(virNetworkPtr ne
> DEBUG("network=%p, buf=%p", network, buf);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> if (buf == NULL) {
> @@ -3196,7 +3181,7 @@ virNetworkGetXMLDesc(virNetworkPtr netwo
> DEBUG("network=%p, flags=%d", network, flags);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (NULL);
> }
> if (flags != 0) {
> @@ -3230,7 +3215,7 @@ virNetworkGetBridgeName(virNetworkPtr ne
> DEBUG("network=%p", network);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (NULL);
> }
>
> @@ -3262,7 +3247,7 @@ virNetworkGetAutostart(virNetworkPtr net
> DEBUG("network=%p, autostart=%p", network, autostart);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
> if (!autostart) {
> @@ -3297,7 +3282,7 @@ virNetworkSetAutostart(virNetworkPtr net
> DEBUG("network=%p, autostart=%d", network, autostart);
>
> if (!VIR_IS_NETWORK(network)) {
> - virLibNetworkError(network, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> + virLibNetworkError(NULL, VIR_ERR_INVALID_NETWORK, __FUNCTION__);
> return (-1);
> }
>
In general +1, yes, just need to make sure that if the domain checking
macro fails then we really can't get the connection, I guess it's true.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard at redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
More information about the libvir-list
mailing list