[libvirt] [PATCH 00/11] Resolution to various Coverity warnings
I upgraded to a new version of Coverity last night (6.5.1) since I was informed it resolved the BAD_SIZEOF errors from the TRACE macros. A majority were resolved, except for 3 which just required a couple of changes to avoid. Since I was down to only a few warnings I tried ratcheting up the analysis "aggressiveness-level" from the default of low to medium and high. There many false positives out of those, but I did manage to find a few more leaks and a missing error check in openvz_driver.c. After this series only a couple of warnings exist in the tests area; however, I've seen IRC chatter on vircommand.c and commandtest.c, so I'll patiently wait on that. There are some warnings in a couple of gnulib modules as well as /usr/include/bits/stdio2.h. John Ferlan (11): tlscontext: Make sure to get proper pointer to name keepalive: Resolve Coverity complaint storage: Resolve resource leaks with cmd processing network: Remove conditional settings to resolve resource leak parallels: Need to free memory on error path openvz: Need to error check openvzDomainSetVcpusFlagsInternal() uml: If need to requery, then VIR_FREE(res) rpc: Need to virCommandFree on error path lxc: Need to call usbFreeDevice() qemu_cgroup: Need to call usbFreeDevice() qemu_hotplug: Need to call usbFreeDevice() src/lxc/lxc_cgroup.c | 8 +++++--- src/network/bridge_driver.c | 9 +++------ src/openvz/openvz_driver.c | 9 +++++++-- src/parallels/parallels_utils.c | 4 +++- src/qemu/qemu_cgroup.c | 6 ++++-- src/qemu/qemu_hotplug.c | 7 +++++-- src/rpc/virkeepalive.c | 14 +++++++------- src/rpc/virnetsocket.c | 1 + src/rpc/virnettlscontext.c | 8 +++++--- src/storage/storage_backend_disk.c | 28 +++++++++++++++------------- src/uml/uml_driver.c | 3 ++- 11 files changed, 57 insertions(+), 40 deletions(-) -- 1.7.11.7
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and makes sure dname was properly filled before attempting the TRACE message. --- src/rpc/virnettlscontext.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 0f0ddff..29d1508 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1,7 +1,7 @@ /* * virnettlscontext.c: TLS encryption/x509 handling * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-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 @@ -950,6 +950,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, unsigned int nCerts, i; char dname[256]; size_t dnamesize = sizeof(dname); + char *dnameptr = NULL; memset(dname, 0, dnamesize); @@ -1025,6 +1026,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, "[session]", gnutls_strerror(ret)); goto authfail; } + dnameptr = dname; VIR_DEBUG("Peer DN is %s", dname); if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname, dname, @@ -1062,14 +1064,14 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr ? dnameptr : "(unknown)"); return 0; authdeny: PROBE(RPC_TLS_CONTEXT_SESSION_DENY, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr ? dnameptr : "(unknown)"); return -1; -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and makes sure dname was properly filled before attempting the TRACE message. --- src/rpc/virnettlscontext.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 0f0ddff..29d1508 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1,7 +1,7 @@ /* * virnettlscontext.c: TLS encryption/x509 handling * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-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 @@ -950,6 +950,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, unsigned int nCerts, i; char dname[256]; size_t dnamesize = sizeof(dname); + char *dnameptr = NULL;
memset(dname, 0, dnamesize);
@@ -1025,6 +1026,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, "[session]", gnutls_strerror(ret)); goto authfail; } + dnameptr = dname; VIR_DEBUG("Peer DN is %s", dname);
if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname, dname, @@ -1062,14 +1064,14 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt,
PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr ? dnameptr : "(unknown)");
return 0;
authdeny: PROBE(RPC_TLS_CONTEXT_SESSION_DENY, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr ? dnameptr : "(unknown)");
return -1;
I guess dname[0] is guaranteed to be not nul as long as gnutls_x509_crt_get_dn succeeded. If so, the patch can be simplified as: dname[0] ? dname : "(unknown)" Osier
On 01/31/2013 03:44 AM, Osier Yang wrote:
On 2013年01月31日 03:36, John Ferlan wrote:
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and
s/compain/complain/
makes sure dname was properly filled before attempting the TRACE message. --- src/rpc/virnettlscontext.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
@@ -950,6 +950,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, unsigned int nCerts, i; char dname[256]; size_t dnamesize = sizeof(dname); + char *dnameptr = NULL;
Would it be any simpler to just 0-initialize dname, as in: char dname[256] = "";
PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname);
At which point, the PROBE(..., dname) would be guaranteed to have a NUL terminator within range? If I understand it, Coverity is complaining that if dname is uninitialized, then the PROBE() may read beyond 256 bytes while looking for the end of a string.
I guess dname[0] is guaranteed to be not nul as long as gnutls_x509_crt_get_dn succeeded.
Not unless we pre-initialize dname[0].
If so, the patch can be simplified as:
dname[0] ? dname : "(unknown)"
Using a conditional would make the difference between a probe stating 'dname=' vs. 'dname=(unknown)'; I don't think it adds that much to need a ternary ?: in the PROBE. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 2013年02月01日 00:41, Eric Blake wrote:
On 01/31/2013 03:44 AM, Osier Yang wrote:
On 2013年01月31日 03:36, John Ferlan wrote:
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and
s/compain/complain/
makes sure dname was properly filled before attempting the TRACE message. --- src/rpc/virnettlscontext.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
@@ -950,6 +950,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, unsigned int nCerts, i; char dname[256]; size_t dnamesize = sizeof(dname); + char *dnameptr = NULL;
Would it be any simpler to just 0-initialize dname, as in:
char dname[256] = "";
PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname);
At which point, the PROBE(..., dname) would be guaranteed to have a NUL terminator within range? If I understand it, Coverity is complaining that if dname is uninitialized, then the PROBE() may read beyond 256 bytes while looking for the end of a string.
I guess dname[0] is guaranteed to be not nul as long as gnutls_x509_crt_get_dn succeeded.
Not unless we pre-initialize dname[0].
There is memset in the code actually, if you check the function.
If so, the patch can be simplified as:
dname[0] ? dname : "(unknown)"
Using a conditional would make the difference between a probe stating 'dname=' vs. 'dname=(unknown)'; I don't think it adds that much to need a ternary ?: in the PROBE.
On 01/31/2013 11:41 AM, Eric Blake wrote:
On 01/31/2013 03:44 AM, Osier Yang wrote:
On 2013年01月31日 03:36, John Ferlan wrote:
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and
s/compain/complain/
fixed this... fingers don't always type what the brain tells them to :-)
makes sure dname was properly filled before attempting the TRACE message. --- src/rpc/virnettlscontext.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
@@ -950,6 +950,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, unsigned int nCerts, i; char dname[256]; size_t dnamesize = sizeof(dname); + char *dnameptr = NULL;
Would it be any simpler to just 0-initialize dname, as in:
char dname[256] = "";
As Osier points out there is a memset(dname, 0, dnamesize) in the code Changing the code to use the above still results in Coverity complaint for each PROBE: 1062 (1) Event bad_sizeof: Taking the size of "dname", which is the address of an object, is suspicious. Did you intend the size of the object itself? 1063 PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, 1064 "ctxt=%p sess=%p dname=%s", 1065 ctxt, sess, dname); This is the only PROBE I found using a stack buffer of a specific size. If I changed the code to VIR_ALLOC_N(dname, dnamesize);, then the message goes away too. I believe the 256 was chosen since that the max length of a distinguished name field from what I've read
PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname);
At which point, the PROBE(..., dname) would be guaranteed to have a NUL terminator within range? If I understand it, Coverity is complaining that if dname is uninitialized, then the PROBE() may read beyond 256 bytes while looking for the end of a string.
I guess dname[0] is guaranteed to be not nul as long as gnutls_x509_crt_get_dn succeeded.
Not unless we pre-initialize dname[0].
If so, the patch can be simplified as:
dname[0] ? dname : "(unknown)"
Using a conditional would make the difference between a probe stating 'dname=' vs. 'dname=(unknown)'; I don't think it adds that much to need a ternary ?: in the PROBE.
On 01/31/2013 12:28 PM, John Ferlan wrote:
On 01/31/2013 11:41 AM, Eric Blake wrote:
On 01/31/2013 03:44 AM, Osier Yang wrote:
On 2013年01月31日 03:36, John Ferlan wrote:
The 'dname' string was only filled in within the loop when available; however, the TRACE macros used it unconditionally and caused Coverity to compain about BAD_SIZEOF. Using a dnameptr keeps Coverity at bay and
s/compain/complain/
+ char *dnameptr = NULL;
Would it be any simpler to just 0-initialize dname, as in:
char dname[256] = "";
As Osier points out there is a memset(dname, 0, dnamesize) in the code
Okay, the memset() does the same thing as initializing would have done.
Changing the code to use the above still results in Coverity complaint for each PROBE:
1062
(1) Event bad_sizeof: Taking the size of "dname", which is the address of an object, is suspicious. Did you intend the size of the object itself?
1063 PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, 1064 "ctxt=%p sess=%p dname=%s", 1065 ctxt, sess, dname);
Lookin at the preprocessed source, it looks like Coverity is complaining about this snippet of the expansion of PROBE(): __builtin_classify_type (((void * )(intptr_t)(dname))) == 5) ? sizeof (void *) : sizeof (((void *)(intptr_t)(dname)))) and yes, we really DO want to take the sizeof the address, not what it points to, because the point of the PROBE() is to write the address at which data starts. So since it sounds like you were able to shut things up by having a pointer instead of an array to begin with, the simplest solution is thus: char dname[256]; char dnameptr = dname; then PROBE(dnameptr) No need to do PROBE(dnameptr ? dnameptr : "(unknown)"). All we are doing is handing Coverity a pointer instead of an array, although both point to the same data, in order to shut up the false positive. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Keep Coverity happy by passing a pointer to 'dname' rather than the array itself. The PROBE expansion would cause a BAD_SIZEOF. --- After all that we're back to this version without the (unknown). I guess that was my former world sneaking in - have to put something there. src/rpc/virnettlscontext.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 0f0ddff..3e7e5e0 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -1,7 +1,7 @@ /* * virnettlscontext.c: TLS encryption/x509 handling * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-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 @@ -949,6 +949,7 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, const gnutls_datum_t *certs; unsigned int nCerts, i; char dname[256]; + char *dnameptr = dname; size_t dnamesize = sizeof(dname); memset(dname, 0, dnamesize); @@ -1062,14 +1063,14 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, PROBE(RPC_TLS_CONTEXT_SESSION_ALLOW, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr); return 0; authdeny: PROBE(RPC_TLS_CONTEXT_SESSION_DENY, "ctxt=%p sess=%p dname=%s", - ctxt, sess, dname); + ctxt, sess, dnameptr); return -1; -- 1.7.11.7
On 01/31/2013 02:50 PM, John Ferlan wrote:
Keep Coverity happy by passing a pointer to 'dname' rather than the array itself. The PROBE expansion would cause a BAD_SIZEOF. --- After all that we're back to this version without the (unknown). I guess that was my former world sneaking in - have to put something there.
src/rpc/virnettlscontext.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
The Coverity analysis emitted a BAD_SIZEOF error when doing the math within the TRACE macro. Doing the math outside the macro keeps Coverity quiet. --- src/rpc/virkeepalive.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index f57fed4..d1fa642 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -1,7 +1,7 @@ /* * virkeepalive.c: keepalive handling * - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011-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 @@ -117,28 +117,28 @@ virKeepAliveTimerInternal(virKeepAlivePtr ka, virNetMessagePtr *msg) { time_t now = time(NULL); + int timeval; if (ka->interval <= 0 || ka->intervalStart == 0) return false; if (now - ka->intervalStart < ka->interval) { - int timeout = ka->interval - (now - ka->intervalStart); - virEventUpdateTimeout(ka->timer, timeout * 1000); + timeval = ka->interval - (now - ka->intervalStart); + virEventUpdateTimeout(ka->timer, timeval * 1000); return false; } + timeval = now - ka->lastPacketReceived; PROBE(RPC_KEEPALIVE_TIMEOUT, "ka=%p client=%p countToDeath=%d idle=%d", - ka, ka->client, ka->countToDeath, - (int) (now - ka->lastPacketReceived)); - + ka, ka->client, ka->countToDeath, timeval); if (ka->countToDeath == 0) { VIR_WARN("No response from client %p after %d keepalive messages in" " %d seconds", ka->client, ka->count, - (int) (now - ka->lastPacketReceived)); + timeval); return true; } else { ka->countToDeath--; -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
The Coverity analysis emitted a BAD_SIZEOF error when doing the math within the TRACE macro. Doing the math outside the macro keeps Coverity quiet. --- src/rpc/virkeepalive.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/rpc/virkeepalive.c b/src/rpc/virkeepalive.c index f57fed4..d1fa642 100644 --- a/src/rpc/virkeepalive.c +++ b/src/rpc/virkeepalive.c @@ -1,7 +1,7 @@ /* * virkeepalive.c: keepalive handling * - * Copyright (C) 2011 Red Hat, Inc. + * Copyright (C) 2011-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 @@ -117,28 +117,28 @@ virKeepAliveTimerInternal(virKeepAlivePtr ka, virNetMessagePtr *msg) { time_t now = time(NULL); + int timeval;
if (ka->interval<= 0 || ka->intervalStart == 0) return false;
if (now - ka->intervalStart< ka->interval) { - int timeout = ka->interval - (now - ka->intervalStart); - virEventUpdateTimeout(ka->timer, timeout * 1000); + timeval = ka->interval - (now - ka->intervalStart); + virEventUpdateTimeout(ka->timer, timeval * 1000); return false; }
+ timeval = now - ka->lastPacketReceived; PROBE(RPC_KEEPALIVE_TIMEOUT, "ka=%p client=%p countToDeath=%d idle=%d", - ka, ka->client, ka->countToDeath, - (int) (now - ka->lastPacketReceived)); - + ka, ka->client, ka->countToDeath, timeval);
if (ka->countToDeath == 0) { VIR_WARN("No response from client %p after %d keepalive messages in" " %d seconds", ka->client, ka->count, - (int) (now - ka->lastPacketReceived)); + timeval); return true; } else { ka->countToDeath--;
ACK
On 01/30/2013 12:36 PM, John Ferlan wrote:
The Coverity analysis emitted a BAD_SIZEOF error when doing the math within the TRACE macro. Doing the math outside the macro keeps Coverity quiet. --- src/rpc/virkeepalive.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
time_t now = time(NULL); + int timeval;
if (ka->interval <= 0 || ka->intervalStart == 0) return false;
if (now - ka->intervalStart < ka->interval) { - int timeout = ka->interval - (now - ka->intervalStart); - virEventUpdateTimeout(ka->timer, timeout * 1000); + timeval = ka->interval - (now - ka->intervalStart); + virEventUpdateTimeout(ka->timer, timeval * 1000); return false;
Eww - pre-existing bug, but we have the potential for multiplication overflow, and for truncation if time_t is wider than int. We probably ought to do a followup code to make this math more robust (by rejecting any timeval input from the user that exceeds MAX_INT/1000). But that doesn't affect Osier's ACK for this patch. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
--- src/storage/storage_backend_disk.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4214e97..40da306 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -1,7 +1,7 @@ /* * storage_backend_disk.c: storage backend for disk handling * - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -385,14 +385,7 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, { bool ok_to_mklabel = false; int ret = -1; - /* eg parted /dev/sda mklabel msdos */ - virCommandPtr cmd = virCommandNewArgList(PARTED, - pool->def->source.devices[0].path, - "mklabel", - "--script", - ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : - virStoragePoolFormatDiskTypeToString(pool->def->source.format)), - NULL); + virCommandPtr cmd = NULL; virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -423,8 +416,17 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } } - if (ok_to_mklabel) + if (ok_to_mklabel) { + /* eg parted /dev/sda mklabel msdos */ + cmd = virCommandNewArgList(PARTED, + pool->def->source.devices[0].path, + "mklabel", + "--script", + ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : + virStoragePoolFormatDiskTypeToString(pool->def->source.format)), + NULL); ret = virCommandRun(cmd, NULL); + } error: virCommandFree(cmd); @@ -634,7 +636,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { int res = -1; - char *partFormat; + char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; virCommandPtr cmd = virCommandNewArgList(PARTED, pool->def->source.devices[0].path, @@ -646,11 +648,11 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool does not support encrypted " "volumes")); - return -1; + goto cleanup; } if (virStorageBackendDiskPartFormat(pool, vol, &partFormat) != 0) { - return -1; + goto cleanup; } virCommandAddArg(cmd, partFormat); -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/storage/storage_backend_disk.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/storage/storage_backend_disk.c b/src/storage/storage_backend_disk.c index 4214e97..40da306 100644 --- a/src/storage/storage_backend_disk.c +++ b/src/storage/storage_backend_disk.c @@ -1,7 +1,7 @@ /* * storage_backend_disk.c: storage backend for disk handling * - * Copyright (C) 2007-2008, 2010-2012 Red Hat, Inc. + * Copyright (C) 2007-2008, 2010-2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -385,14 +385,7 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, { bool ok_to_mklabel = false; int ret = -1; - /* eg parted /dev/sda mklabel msdos */ - virCommandPtr cmd = virCommandNewArgList(PARTED, - pool->def->source.devices[0].path, - "mklabel", - "--script", - ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : - virStoragePoolFormatDiskTypeToString(pool->def->source.format)), - NULL); + virCommandPtr cmd = NULL;
virCheckFlags(VIR_STORAGE_POOL_BUILD_OVERWRITE | VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, ret); @@ -423,8 +416,17 @@ virStorageBackendDiskBuildPool(virConnectPtr conn ATTRIBUTE_UNUSED, } }
- if (ok_to_mklabel) + if (ok_to_mklabel) { + /* eg parted /dev/sda mklabel msdos */ + cmd = virCommandNewArgList(PARTED, + pool->def->source.devices[0].path, + "mklabel", + "--script", + ((pool->def->source.format == VIR_STORAGE_POOL_DISK_DOS) ? "msdos" : + virStoragePoolFormatDiskTypeToString(pool->def->source.format)), + NULL); ret = virCommandRun(cmd, NULL); + }
Good catch.
error: virCommandFree(cmd); @@ -634,7 +636,7 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virStorageVolDefPtr vol) { int res = -1; - char *partFormat; + char *partFormat = NULL; unsigned long long startOffset = 0, endOffset = 0; virCommandPtr cmd = virCommandNewArgList(PARTED, pool->def->source.devices[0].path, @@ -646,11 +648,11 @@ virStorageBackendDiskCreateVol(virConnectPtr conn ATTRIBUTE_UNUSED, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("storage pool does not support encrypted " "volumes")); - return -1; + goto cleanup; }
if (virStorageBackendDiskPartFormat(pool, vol,&partFormat) != 0) { - return -1; + goto cleanup; } virCommandAddArg(cmd, partFormat);
ACK
The conditional setting of cmdout in networkBuildDhcpDaemonCommandLine() caused Coverity to complain that 'cmd' could be leaked if !cmdout. Since the function is local and only called with cmdout being passed those checks have been removed. --- src/network/bridge_driver.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 21255f0..e9a36e5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -995,7 +995,8 @@ cleanup: /* build the dnsmasq command line */ static int -networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout, +networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, + virCommandPtr *cmdout, char *pidfile, dnsmasqContext *dctx, dnsmasqCapsPtr caps) { @@ -1027,13 +1028,9 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdou cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); - - if (cmdout) - *cmdout = cmd; + *cmdout = cmd; ret = 0; cleanup: - if (ret < 0) - virCommandFree(cmd); return ret; } -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
The conditional setting of cmdout in networkBuildDhcpDaemonCommandLine() caused Coverity to complain that 'cmd' could be leaked if !cmdout. Since the function is local and only called with cmdout being passed those checks have been removed. --- src/network/bridge_driver.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 21255f0..e9a36e5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -995,7 +995,8 @@ cleanup:
/* build the dnsmasq command line */ static int -networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdout, +networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, + virCommandPtr *cmdout, char *pidfile, dnsmasqContext *dctx, dnsmasqCapsPtr caps) { @@ -1027,13 +1028,9 @@ networkBuildDhcpDaemonCommandLine(virNetworkObjPtr network, virCommandPtr *cmdou
cmd = virCommandNew(dnsmasqCapsGetBinaryPath(caps)); virCommandAddArgFormat(cmd, "--conf-file=%s", configfile); - - if (cmdout) - *cmdout = cmd; + *cmdout = cmd; ret = 0; cleanup: - if (ret< 0) - virCommandFree(cmd); return ret; }
Per the function is static, the changes are fine. But to be safe, using ATTRIBUTE_NONNULL will be better: static int ATTRIBUTE_NONNULL(2) networkBuildDhcpDaemonCommandLine (...) Osier
--- src/parallels/parallels_utils.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/parallels/parallels_utils.c b/src/parallels/parallels_utils.c index 171f5d0..0b589ab 100644 --- a/src/parallels/parallels_utils.c +++ b/src/parallels/parallels_utils.c @@ -135,8 +135,10 @@ parallelsAddFileExt(const char *path, const char *ext) return NULL; } - if (!virStrcpy(new_path, path, len)) + if (!virStrcpy(new_path, path, len)) { + VIR_FREE(new_path); return NULL; + } strcat(new_path, ext); return new_path; -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/parallels/parallels_utils.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_utils.c b/src/parallels/parallels_utils.c index 171f5d0..0b589ab 100644 --- a/src/parallels/parallels_utils.c +++ b/src/parallels/parallels_utils.c @@ -135,8 +135,10 @@ parallelsAddFileExt(const char *path, const char *ext) return NULL; }
- if (!virStrcpy(new_path, path, len)) + if (!virStrcpy(new_path, path, len)) { + VIR_FREE(new_path); return NULL; + } strcat(new_path, ext);
return new_path;
ACK.
--- src/openvz/openvz_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e12a84b..61d8390 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1,7 +1,7 @@ /* * openvz_driver.c: core driver methods for managing OpenVZ VEs * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -1367,7 +1367,12 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto cleanup; } - openvzDomainSetVcpusInternal(vm, nvcpus); + if (openvzDomainSetVcpusInternal(vm, nvcpus) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not set number of virtual cpu")); + goto cleanup; + } + ret = 0; cleanup: -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/openvz/openvz_driver.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e12a84b..61d8390 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1,7 +1,7 @@ /* * openvz_driver.c: core driver methods for managing OpenVZ VEs * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -1367,7 +1367,12 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, goto cleanup; }
- openvzDomainSetVcpusInternal(vm, nvcpus); + if (openvzDomainSetVcpusInternal(vm, nvcpus)< 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not set number of virtual cpu"));
Generally, we use the term "vCPU".
+ goto cleanup; + } + ret = 0;
cleanup:
ACK with small change.
--- Since I'm not only changing one 'virtual cpu' reference to 'vCPU', I figured I would just post an update. As you see the message I added was a cut-n-paste of other messages where similar calls were made. To be consistent, I changed other references to vCPUs and I changed one other message as well. src/openvz/openvz_driver.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index e12a84b..90c753b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1,7 +1,7 @@ /* * openvz_driver.c: core driver methods for managing OpenVZ VEs * - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain * Copyright (C) 2007 Anoop Joe Cyriac @@ -1004,7 +1004,7 @@ openvzDomainDefineXML(virConnectPtr conn, const char *xml) if (vm->def->maxvcpus > 0) { if (openvzDomainSetVcpusInternal(vm, vm->def->maxvcpus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not set number of virtual cpu")); + _("Could not set number of vCPUs")); goto cleanup; } } @@ -1097,7 +1097,7 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, if (vm->def->maxvcpus > 0) { if (openvzDomainSetVcpusInternal(vm, vm->def->maxvcpus) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not set number of virtual cpu")); + _("Could not set number of vCPUs")); goto cleanup; } } @@ -1363,11 +1363,16 @@ static int openvzDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus, if (nvcpus <= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("VCPUs should be >= 1")); + _("Number of vCPUs should be >= 1")); + goto cleanup; + } + + if (openvzDomainSetVcpusInternal(vm, nvcpus) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not set number of vCPUs")); goto cleanup; } - openvzDomainSetVcpusInternal(vm, nvcpus); ret = 0; cleanup: -- 1.7.11.7
Coverity noted that in the retry logic loop if res had been set, then it could be leaked --- src/uml/uml_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 705495e..d97cc96 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1,7 +1,7 @@ /* * uml_driver.c: core driver methods for managing UML guests * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -263,6 +263,7 @@ requery: way somehow ...perhaps register a timer */ if (retries++ < 50) { usleep(1000*10); + VIR_FREE(res); goto requery; } } -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
Coverity noted that in the retry logic loop if res had been set, then it could be leaked --- src/uml/uml_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 705495e..d97cc96 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1,7 +1,7 @@ /* * uml_driver.c: core driver methods for managing UML guests * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -263,6 +263,7 @@ requery: way somehow ...perhaps register a timer */ if (retries++< 50) { usleep(1000*10); + VIR_FREE(res);
Personally I'd like the VIR_FREE above the "if" branch, but since VIR_FREE(res) at the end of the function will take over the work if "retries" happened to be 50. It's fine. However, putting VIR_FREE before the sleeping is never a bad idea. ACK with the added VIR_FREE moved 1 line above.
--- src/rpc/virnetsocket.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 0158970..93980d6 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -704,6 +704,7 @@ int virNetSocketNewConnectSSH(const char *nodename, virBufferEscapeShell(&buf, netcat); if (virBufferError(&buf)) { + virCommandFree(cmd); virBufferFreeAndReset(&buf); virReportOOMError(); return -1; -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/rpc/virnetsocket.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 0158970..93980d6 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -704,6 +704,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
virBufferEscapeShell(&buf, netcat); if (virBufferError(&buf)) { + virCommandFree(cmd); virBufferFreeAndReset(&buf); virReportOOMError(); return -1;
ACK
--- src/lxc/lxc_cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 1984c5f..ece9e03 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_cgroup.c: LXC cgroup helpers @@ -426,8 +426,10 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, NULL)) == NULL) goto cleanup; - if (usbDeviceFileIterate(usb, virLXCSetupHostUsbDeviceCgroup, - cgroup) < 0) + rc = usbDeviceFileIterate(usb, virLXCSetupHostUsbDeviceCgroup, + cgroup); + usbFreeDevice(usb); + if (rc < 0) goto cleanup; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/lxc/lxc_cgroup.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 1984c5f..ece9e03 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2010-2012 Red Hat, Inc. + * Copyright (C) 2010-2013 Red Hat, Inc. * Copyright IBM Corp. 2008 * * lxc_cgroup.c: LXC cgroup helpers @@ -426,8 +426,10 @@ static int virLXCCgroupSetupDeviceACL(virDomainDefPtr def, NULL)) == NULL) goto cleanup;
- if (usbDeviceFileIterate(usb, virLXCSetupHostUsbDeviceCgroup, - cgroup)< 0) + rc = usbDeviceFileIterate(usb, virLXCSetupHostUsbDeviceCgroup, + cgroup); + usbFreeDevice(usb); + if (rc< 0) goto cleanup; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
ACK
--- src/qemu/qemu_cgroup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 6527146..9f0eaf9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -294,8 +294,10 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, NULL)) == NULL) goto cleanup; - if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, - &data) < 0) + rc = usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, + &data); + usbFreeDevice(usb); + if (rc < 0) goto cleanup; } } -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/qemu/qemu_cgroup.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 6527146..9f0eaf9 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -294,8 +294,10 @@ int qemuSetupCgroup(virQEMUDriverPtr driver, NULL)) == NULL) goto cleanup;
- if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, -&data)< 0) + rc = usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, +&data); + usbFreeDevice(usb); + if (rc< 0) goto cleanup; } }
ACK.
--- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 18c4109..2a52650 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1,7 +1,7 @@ /* * qemu_hotplug.h: QEMU device hotplug management * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1120,6 +1120,7 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, virCgroupPtr cgroup = NULL; usbDevice *usb; qemuCgroupData data; + int rc; if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1135,7 +1136,9 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, data.vm = vm; data.cgroup = cgroup; - if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, &data) < 0) + rc = usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, &data); + usbFreeDevice(usb); + if (rc < 0) goto error; } -- 1.7.11.7
On 2013年01月31日 03:36, John Ferlan wrote:
--- src/qemu/qemu_hotplug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 18c4109..2a52650 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1,7 +1,7 @@ /* * qemu_hotplug.h: QEMU device hotplug management * - * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006-2013 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -1120,6 +1120,7 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver, virCgroupPtr cgroup = NULL; usbDevice *usb; qemuCgroupData data; + int rc;
if (virCgroupForDomain(driver->cgroup, vm->def->name,&cgroup, 0) != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1135,7 +1136,9 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
data.vm = vm; data.cgroup = cgroup; - if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,&data)< 0) + rc = usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,&data); + usbFreeDevice(usb); + if (rc< 0) goto error;
Hum, since there are 3 patches do the similar changes. I have to say we should avoid what Peter always tries to cleanup, and change these patches like: if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,&data)< 0) { usbFreeDevice(usb); goto error; } usbFreeDevice(usb); This applies to 9/11 and 10/11 too. ACK with the changes.
On 01/30/2013 02:36 PM, John Ferlan wrote:
I upgraded to a new version of Coverity last night (6.5.1) since I was informed it resolved the BAD_SIZEOF errors from the TRACE macros. A majority were resolved, except for 3 which just required a couple of changes to avoid.
Since I was down to only a few warnings I tried ratcheting up the analysis "aggressiveness-level" from the default of low to medium and high. There many false positives out of those, but I did manage to find a few more leaks and a missing error check in openvz_driver.c.
After this series only a couple of warnings exist in the tests area; however, I've seen IRC chatter on vircommand.c and commandtest.c, so I'll patiently wait on that. There are some warnings in a couple of gnulib modules as well as /usr/include/bits/stdio2.h.
John Ferlan (11): tlscontext: Make sure to get proper pointer to name keepalive: Resolve Coverity complaint storage: Resolve resource leaks with cmd processing network: Remove conditional settings to resolve resource leak parallels: Need to free memory on error path openvz: Need to error check openvzDomainSetVcpusFlagsInternal() uml: If need to requery, then VIR_FREE(res) rpc: Need to virCommandFree on error path lxc: Need to call usbFreeDevice() qemu_cgroup: Need to call usbFreeDevice() qemu_hotplug: Need to call usbFreeDevice()
src/lxc/lxc_cgroup.c | 8 +++++--- src/network/bridge_driver.c | 9 +++------ src/openvz/openvz_driver.c | 9 +++++++-- src/parallels/parallels_utils.c | 4 +++- src/qemu/qemu_cgroup.c | 6 ++++-- src/qemu/qemu_hotplug.c | 7 +++++-- src/rpc/virkeepalive.c | 14 +++++++------- src/rpc/virnetsocket.c | 1 + src/rpc/virnettlscontext.c | 8 +++++--- src/storage/storage_backend_disk.c | 28 +++++++++++++++------------- src/uml/uml_driver.c | 3 ++- 11 files changed, 57 insertions(+), 40 deletions(-)
With Dan Berrange's changes: https://www.redhat.com/archives/libvir-list/2013-February/msg00021.html My 9/11 and 10/11 were no longer necessary. The 11/11 changes (qemu_hotplug.c) still apply, but were changed to use the new virUSBDeviceFree() rather than the previous usbFreeDevice(). Other changes 1/11, 4/11, 6/11, & 7/11 were made as requested. I rebuilt, retested, and pushed. John
participants (3)
-
Eric Blake -
John Ferlan -
Osier Yang