9:01 a.m.
Upgraded to Coverity 6.5.3 which uncovered a few code paths with issues.
John Ferlan (3):
lxc_process: Resolve Coverity NULL_RETURNS error
xencapstest: Resolve Coverity CHECKED_RETURN error
shunloadtest: Resolve Coverity CHECKED_RETURN error
src/lxc/lxc_process.c | 4 ++--
tests/shunloadhelper.c | 12 ++++++++----
tests/shunloadtest.c | 26 +++++++++++++++++---------
tests/xencapstest.c | 3 ++-
4 files changed, 29 insertions(+), 16 deletions(-)
--
1.8.1.4
9:01 a.m.
New subject: [libvirt] [PATCH 1/3] lxc_process: Resolve Coverity NULL_RETURNS error
In virLXCProcessReboot, if the 'conn' value is NULL, the eventual
call to virLXCNetworkLookupByName() via virLXCProcessStart() and
virLXCProcessSetupInterfaces() will cause a core since it references
conn->networkDriver directly.
---
src/lxc/lxc_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index b06d748..4410db4 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -181,7 +181,6 @@ virLXCProcessReboot(virLXCDriverPtr driver,
autodestroy = true;
} else {
conn = virConnectOpen("lxc:///");
- /* Ignoring NULL conn which is mostly harmless here */
}
/* In a reboot scenario, we need to make sure we continue
@@ -192,7 +191,8 @@ virLXCProcessReboot(virLXCDriverPtr driver,
vm->newDef = NULL;
virLXCProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_SHUTDOWN);
vm->newDef = savedDef;
- if (virLXCProcessStart(conn, driver, vm, autodestroy, reason) < 0) {
+ if (!conn ||
+ virLXCProcessStart(conn, driver, vm, autodestroy, reason) < 0) {
VIR_WARN("Unable to handle reboot of vm %s",
vm->def->name);
goto cleanup;
--
1.8.1.4
9:18 a.m.
New subject: [libvirt] [PATCH 1/3] lxc_process: Resolve Coverity NULL_RETURNS error
On Thu, May 16, 2013 at 10:01:51AM -0400, John Ferlan wrote:
In virLXCProcessReboot, if the 'conn' value is NULL, the
eventual
call to virLXCNetworkLookupByName() via virLXCProcessStart() and
virLXCProcessSetupInterfaces() will cause a core since it references
conn->networkDriver directly.
---
src/lxc/lxc_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index b06d748..4410db4 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -181,7 +181,6 @@ virLXCProcessReboot(virLXCDriverPtr driver,
autodestroy = true;
} else {
conn = virConnectOpen("lxc:///");
- /* Ignoring NULL conn which is mostly harmless here */
I'm actually inclined to say that we should stop ignoring failures
of virConnectOpen(). I don't think there's any valid reason for us
to be doing that. Then we wouldnt have to worry about conn being
NULL later
This ignoring of errors is historical legacy of my own creation
when first writing the QEMU driver. I can't think what possessed
me to ignore the errors at the time.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
9:19 a.m.
New subject: [libvirt] [PATCH 1/3] lxc_process: Resolve Coverity NULL_RETURNS error
On 05/16/2013 08:01 AM, John Ferlan wrote:
In virLXCProcessReboot, if the 'conn' value is NULL, the
eventual
call to virLXCNetworkLookupByName() via virLXCProcessStart() and
virLXCProcessSetupInterfaces() will cause a core since it references
conn->networkDriver directly.
Did you investigate which commit introduced this problem?
---
src/lxc/lxc_process.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:51 a.m.
New subject: [libvirt] [PATCH 1/3] lxc_process: Resolve Coverity NULL_RETURNS error
On 05/16/2013 10:19 AM, Eric Blake wrote:
On 05/16/2013 08:01 AM, John Ferlan wrote:
> In virLXCProcessReboot, if the 'conn' value is NULL, the eventual
> call to virLXCNetworkLookupByName() via virLXCProcessStart() and
> virLXCProcessSetupInterfaces() will cause a core since it references
> conn->networkDriver directly.
Did you investigate which commit introduced this problem?
yes, cb612ee4, when the function was created.
I will add it to the commit message
Given Daniel's response - is there a preference to perhaps add some sort
of message that conn was NULL from the virConnectOpen(). I would think
that the virLXCProcessStop() would still need to occur though, but also
wouldn't want to assume anything :-)
John
> ---
> src/lxc/lxc_process.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
ACK.
9:01 a.m.
New subject: [libvirt] [PATCH 2/3] xencapstest: Resolve Coverity CHECKED_RETURN error
The return from virInitialize() needs to be checked.
---
tests/xencapstest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tests/xencapstest.c b/tests/xencapstest.c
index e220234..8bdd6cd 100644
--- a/tests/xencapstest.c
+++ b/tests/xencapstest.c
@@ -161,7 +161,8 @@ mymain(void)
int ret = 0;
xenHypervisorInit(&hv_versions);
- virInitialize();
+ if (virInitialize() < 0)
+ return EXIT_FAILURE;
if (virtTestRun("Capabilities for i686, no PAE, no HVM",
1, testXeni686, NULL) != 0)
--
1.8.1.4
9:20 a.m.
New subject: [libvirt] [PATCH 2/3] xencapstest: Resolve Coverity CHECKED_RETURN error
On 05/16/2013 08:01 AM, John Ferlan wrote:
The return from virInitialize() needs to be checked.
---
tests/xencapstest.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
ACK.
diff --git a/tests/xencapstest.c b/tests/xencapstest.c
index e220234..8bdd6cd 100644
--- a/tests/xencapstest.c
+++ b/tests/xencapstest.c
@@ -161,7 +161,8 @@ mymain(void)
int ret = 0;
xenHypervisorInit(&hv_versions);
- virInitialize();
+ if (virInitialize() < 0)
+ return EXIT_FAILURE;
if (virtTestRun("Capabilities for i686, no PAE, no HVM",
1, testXeni686, NULL) != 0)
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
9:01 a.m.
New subject: [libvirt] [PATCH 3/3] shunloadtest: Resolve Coverity CHECKED_RETURN error
The shunloadStart function didn't check the status of virInitialize which
was flagged by Coverity. Adjust the function and shunloadtest in order
to handle the situation.
---
tests/shunloadhelper.c | 12 ++++++++----
tests/shunloadtest.c | 26 +++++++++++++++++---------
2 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/tests/shunloadhelper.c b/tests/shunloadhelper.c
index a8f5aef..f2afbe8 100644
--- a/tests/shunloadhelper.c
+++ b/tests/shunloadhelper.c
@@ -36,16 +36,20 @@ static void shunloadError(void *userData ATTRIBUTE_UNUSED,
{
}
-void shunloadStart(void);
+int shunloadStart(void);
-void shunloadStart(void) {
+int shunloadStart(void) {
virConnectPtr conn;
virSetErrorFunc(NULL, shunloadError);
- virInitialize();
+ if (virInitialize() < 0)
+ return -1;
conn = virConnectOpen("test:///default");
virDomainDestroy(NULL);
- if (conn)
+ if (conn) {
virConnectClose(conn);
+ return 0;
+ }
+ return -1;
}
diff --git a/tests/shunloadtest.c b/tests/shunloadtest.c
index 8271b93..8190e97 100644
--- a/tests/shunloadtest.c
+++ b/tests/shunloadtest.c
@@ -56,17 +56,22 @@
pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
pthread_mutex_t lock = PTHREAD_MUTEX_INITIALIZER;
bool running = false;
+bool failstart = false;
bool quit = false;
static void *threadMain(void *arg)
{
- void (*startup)(void) = arg;
-
- startup();
-
- pthread_mutex_lock(&lock);
- running = true;
- pthread_cond_signal(&cond);
+ int (*startup)(void) = arg;
+
+ if (startup() < 0) {
+ pthread_mutex_lock(&lock);
+ failstart = true;
+ pthread_cond_signal(&cond);
+ } else {
+ pthread_mutex_lock(&lock);
+ running = true;
+ pthread_cond_signal(&cond);
+ }
while (!quit) {
pthread_cond_wait(&cond, &lock);
@@ -119,7 +124,7 @@ int main(int argc ATTRIBUTE_UNUSED, char **argv)
/* Wait for the thread to start and call libvirt */
pthread_mutex_lock(&lock);
- while (!running) {
+ while (!running && !failstart) {
pthread_cond_wait(&cond, &lock);
}
@@ -138,7 +143,10 @@ int main(int argc ATTRIBUTE_UNUSED, char **argv)
* causing a SEGV !
*/
- fprintf(stderr, "OK\n");
+ if (failstart)
+ fprintf(stderr, "FAIL to initialize libvirt\n");
+ else
+ fprintf(stderr, "OK\n");
return 0;
}
--
1.8.1.4
9:22 a.m.
New subject: [libvirt] [PATCH 3/3] shunloadtest: Resolve Coverity CHECKED_RETURN error
On 05/16/2013 08:01 AM, John Ferlan wrote:
The shunloadStart function didn't check the status of
virInitialize which
was flagged by Coverity. Adjust the function and shunloadtest in order
to handle the situation.
---
tests/shunloadhelper.c | 12 ++++++++----
tests/shunloadtest.c | 26 +++++++++++++++++---------
2 files changed, 25 insertions(+), 13 deletions(-)
ACK.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:30 a.m.
On 05/16/2013 10:01 AM, John Ferlan wrote:
Upgraded to Coverity 6.5.3 which uncovered a few code paths with
issues.
John Ferlan (3):
lxc_process: Resolve Coverity NULL_RETURNS error
xencapstest: Resolve Coverity CHECKED_RETURN error
shunloadtest: Resolve Coverity CHECKED_RETURN error
src/lxc/lxc_process.c | 4 ++--
tests/shunloadhelper.c | 12 ++++++++----
tests/shunloadtest.c | 26 +++++++++++++++++---------
tests/xencapstest.c | 3 ++-
4 files changed, 29 insertions(+), 16 deletions(-)
patches 2/3 and 3/3 were pushed, I'm reworking 1/3 to store the
virConnectPtr
John
4203
days inactive
4208
days old
9 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
John Ferlan