1:17 p.m.
locking:
Fixes Coverity OVERRUN warning due to the size of the source string being
19 bytes and the memcpy size of 48 bytes
security:
Fixes Coverity warning STRING_OVERFLOW about due to the copy being from a
variable sized string to a fixed length string. Even though on read other
checks ensure the source buffer is no longer than the number of bytes in
the destination, Coverity doesn't know that.
rpc:
Fixes Coverity warning DEADCODE due to removal of code as specified in
the commit message
parallels:
Fixes Coverity warning NULL_RETURNS from not checking result of
virJSONValueObjectGetString() call as part of commit id: 8ce9e2ab
util:
Fixes Coverity warning NULL_RETURNS due to not checking the return
value 'child' from a virJSONValueNewObject() call.
qemu:
Fixes Coverity warning NULL_RETURNS from not checking the return
value 'activeDev' from a pciDeviceListFind() call.
selinux:
Fixes Coverity warning RESOURCE_LEAK as a result of code specified in
the commit message
esx:
Fixes Coverity warning REVERSE_INULL as a result of needlessly checking
if objectSpec != NULL. At the point in the code, objectSpec must be valid.
virobject:
Fixes Coverity warning CONSTANT_EXPRESSION_RESULT and DEADCODE as a
result of commit id b545f65 using "if (!virObjectInitialize() < 0)"
network:
Fixes Coverity warning UNUSED_VALUE as specific in the commit message
John Ferlan (10):
locking: use virStrcpy instead of memcpy
security: Use virStrcpy to move the label
rpc: Remove call to virKeepAliveStop at init
parallels: Need to handle virJSONValueObjectGetString error
util: Need to check child JSON allocation before use
qemu: Check valid activeDev before calling pciDeviceSetUsedBy
selinux: Resolve resource leak using the default disk label
esx: No need to check for objectSpec
virobject: Remove the bogus ! from call to virObjectInitialize()
network: Remove dead code getting, but not using ipdef
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
src/conf/domain_conf.h | 3 +++
src/esx/esx_vi.c | 7 +++----
src/locking/lock_driver_sanlock.c | 9 ++++++++-
src/network/bridge_driver.c | 3 ---
src/parallels/parallels_driver.c | 5 ++++-
src/qemu/qemu_hostdev.c | 3 ++-
src/rpc/virnetserverclient.c | 2 --
src/security/security_dac.c | 8 +++++++-
src/security/security_selinux.c | 6 +++---
src/util/virlockspace.c | 3 +++
src/util/virobject.c | 4 ++--
12 files changed, 64 insertions(+), 18 deletions(-)
--
1.7.11.7
1:17 p.m.
New subject: [libvirt] [PATCH 01/10] locking: use virStrcpy instead of memcpy
---
src/locking/lock_driver_sanlock.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 0b7c6d5..a054806 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -206,7 +206,14 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error;
}
- memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN);
+ if (!virStrcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
+ SANLK_NAME_LEN)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Lockspace path '%s' exceeded %d
characters"),
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
+ SANLK_PATH_LEN);
+ goto error;
+ }
ls.host_id = 0; /* Doesn't matter for initialization */
ls.flags = 0;
if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) {
--
1.7.11.7
2:38 p.m.
New subject: [libvirt] [PATCH 01/10] locking: use virStrcpy instead of memcpy
On 01/17/13 20:17, John Ferlan wrote:
---
src/locking/lock_driver_sanlock.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 0b7c6d5..a054806 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -206,7 +206,14 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error;
}
- memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN);
+ if (!virStrcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
ls.name is static in this function, you can use virStrcpyStatic here to
save the length parameter.
+ SANLK_NAME_LEN)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Lockspace path '%s' exceeded %d
characters"),
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
+ SANLK_PATH_LEN);
+ goto error;
+ }
ls.host_id = 0; /* Doesn't matter for initialization */
ls.flags = 0;
if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) {
7:50 a.m.
New subject: [libvirt] [PATCH v2 01/10] locking: use virStrcpyStatic instead of memcpy
---
src/locking/lock_driver_sanlock.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 0b7c6d5..95f7d61 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -206,7 +206,14 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error;
}
- memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN);
+ if (!virStrcpyStatic(ls.name,
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Lockspace path '%s' exceeded %d
characters"),
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
+ SANLK_PATH_LEN);
+ goto error;
+ }
ls.host_id = 0; /* Doesn't matter for initialization */
ls.flags = 0;
if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) {
--
1.7.11.7
6:42 a.m.
New subject: [libvirt] [PATCH v2 01/10] locking: use virStrcpyStatic instead of memcpy
On 01/18/13 14:50, John Ferlan wrote:
---
src/locking/lock_driver_sanlock.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
<off-topic>
Next time please start a new thread with V2 patches. It's easy to
overlook them in an existing thread.
</off-topic>
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index 0b7c6d5..95f7d61 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -206,7 +206,14 @@ static int virLockManagerSanlockSetupLockspace(void)
goto error;
}
- memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN);
+ if (!virStrcpyStatic(ls.name,
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Lockspace path '%s' exceeded %d
characters"),
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE,
+ SANLK_PATH_LEN);
+ goto error;
+ }
Hm the error message will trigger only if we ever change the length of
VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE. I think that it won't ever
happen but this might be a good sanity check.
ls.host_id = 0; /* Doesn't matter for initialization */
ls.flags = 0;
if (!virStrcpy(ls.host_id_disk.path, path, SANLK_PATH_LEN)) {
If nobody else responds otherwise till tomorrow, I will push the patch
as-is.
Peter
3:52 p.m.
New subject: [libvirt] [PATCH v2 01/10] locking: use virStrcpyStatic instead of memcpy
On 01/22/13 13:42, Peter Krempa wrote:
On 01/18/13 14:50, John Ferlan wrote:
> ---
> src/locking/lock_driver_sanlock.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
If nobody else responds otherwise till tomorrow, I will push the patch
as-is.
Pushed.
Peter
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
1:17 p.m.
New subject: [libvirt] [PATCH 02/10] security: Use virStrcpy to move the label
---
src/security/security_dac.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index deff024..7ef7eb9 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -995,7 +995,13 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return -1;
if (secdef->label)
- strcpy(seclabel->label, secdef->label);
+ if (!virStrcpy(seclabel->label, secdef->label,
+ VIR_SECURITY_LABEL_BUFLEN)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label exceeds max %d bytes"),
+ VIR_SECURITY_LABEL_BUFLEN-1);
+ return -1;
+ }
return 0;
}
--
1.7.11.7
3:44 p.m.
New subject: [libvirt] [PATCH 02/10] security: Use virStrcpy to move the label
On 01/17/13 20:17, John Ferlan wrote:
---
src/security/security_dac.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index deff024..7ef7eb9 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -995,7 +995,13 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return -1;
if (secdef->label)
- strcpy(seclabel->label, secdef->label);
+ if (!virStrcpy(seclabel->label, secdef->label,
+ VIR_SECURITY_LABEL_BUFLEN)) {
I'd rather go for a ignore_value here. AFAIK it's impossible to overflow
the allocated buffer here.
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("security label exceeds max %d bytes"),
+ VIR_SECURITY_LABEL_BUFLEN-1);
+ return -1;
+ }
return 0;
}
Peter
8:27 a.m.
New subject: [libvirt] [PATCH v2 2/10] security: Use virStrcpy to move the label
---
src/security/security_dac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index deff024..93a0301 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -995,7 +995,8 @@ virSecurityDACGetProcessLabel(virSecurityManagerPtr mgr
ATTRIBUTE_UNUSED,
return -1;
if (secdef->label)
- strcpy(seclabel->label, secdef->label);
+ ignore_value(virStrcpy(seclabel->label, secdef->label,
+ VIR_SECURITY_LABEL_BUFLEN));
return 0;
}
--
1.7.11.7
1:17 p.m.
New subject: [libvirt] [PATCH 03/10] rpc: Remove call to virKeepAliveStop at init
This code is not reachable as of commit id: bb85f229.
---
src/rpc/virnetserverclient.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 85239dd..e254309 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1360,8 +1360,6 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client,
cleanup:
virObjectUnlock(client);
- if (ka)
- virKeepAliveStop(ka);
virObjectUnref(ka);
return ret;
--
1.7.11.7
1:57 p.m.
New subject: [libvirt] [PATCH 03/10] rpc: Remove call to virKeepAliveStop at init
On 01/17/13 20:17, John Ferlan wrote:
This code is not reachable as of commit id: bb85f229.
---
src/rpc/virnetserverclient.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 85239dd..e254309 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1360,8 +1360,6 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client,
cleanup:
virObjectUnlock(client);
- if (ka)
- virKeepAliveStop(ka);
virObjectUnref(ka);
This unref also doesn't make sense. The cleanup path is reached only
when ka is NULL. This allows to optimize the function even more.
return ret;
8:29 a.m.
New subject: [libvirt] [PATCH v2 3/10] rpc: Remove unnecessary calls
The code is not reachable as of commit id: bb85f229. Removed
virKeepAliveStop() and virObjectUnref() because 'ka' cannot be
anything but NULL at the cleanup label.
---
src/rpc/virnetserverclient.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 85239dd..40e8481 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1360,9 +1360,6 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client,
cleanup:
virObjectUnlock(client);
- if (ka)
- virKeepAliveStop(ka);
- virObjectUnref(ka);
return ret;
}
--
1.7.11.7
6:45 a.m.
New subject: [libvirt] [PATCH v2 3/10] rpc: Remove unnecessary calls
On 01/18/13 15:29, John Ferlan wrote:
The code is not reachable as of commit id: bb85f229. Removed
virKeepAliveStop() and virObjectUnref() because 'ka' cannot be
anything but NULL at the cleanup label.
---
src/rpc/virnetserverclient.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 85239dd..40e8481 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1360,9 +1360,6 @@ virNetServerClientInitKeepAlive(virNetServerClientPtr client,
cleanup:
virObjectUnlock(client);
- if (ka)
- virKeepAliveStop(ka);
- virObjectUnref(ka);
return ret;
}
ACK with one nit. the asignment ka = NULL just before the cleanup label
is dead code now too. I will be pushing this with the following change
squashed in:
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 40e8481..af0560e 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1356,7 +1356,6 @@
virNetServerClientInitKeepAlive(virNetServerClientPtr client,
virObjectRef(client);
client->keepalive = ka;
- ka = NULL;
cleanup:
virObjectUnlock(client);
Peter
1:17 p.m.
New subject: [libvirt] [PATCH 04/10] parallels: Need to handle virJSONValueObjectGetString error
---
src/parallels/parallels_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index ea193af..1cf66ea 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -514,7 +514,10 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
if (virJSONValueObjectHasKey(value, "state")) {
- tmp = virJSONValueObjectGetString(value, "state");
+ if (!(tmp = virJSONValueObjectGetString(value, "state"))) {
+ parallelsParseError();
+ goto error;
+ }
if STREQ(tmp, "disconnected")
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
}
--
1.7.11.7
2:03 p.m.
New subject: [libvirt] [PATCH 04/10] parallels: Need to handle virJSONValueObjectGetString error
On 01/17/13 20:17, John Ferlan wrote:
---
src/parallels/parallels_driver.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index ea193af..1cf66ea 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -514,7 +514,10 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
if (virJSONValueObjectHasKey(value, "state")) {
- tmp = virJSONValueObjectGetString(value, "state");
Hm calling this without checking should be safe as the
virJSONValueObjectHasKey basically guarantees successful return of a
value in virJSONValueObjectGetString.
+ if (!(tmp = virJSONValueObjectGetString(value,
"state"))) {
+ parallelsParseError();
+ goto error;
+ }
if STREQ(tmp, "disconnected")
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
}
I don't mind doing the change, but I think this is a false positive.
I give ACK to this patch, but it shouldn't be a problem to go without it.
Peter
2:42 p.m.
New subject: [libvirt] [PATCH 04/10] parallels: Need to handle virJSONValueObjectGetString error
On 01/17/2013 01:03 PM, Peter Krempa wrote:
On 01/17/13 20:17, John Ferlan wrote:
> ---
> src/parallels/parallels_driver.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/parallels/parallels_driver.c
> b/src/parallels/parallels_driver.c
> index ea193af..1cf66ea 100644
> --- a/src/parallels/parallels_driver.c
> +++ b/src/parallels/parallels_driver.c
> @@ -514,7 +514,10 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
>
> net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
> if (virJSONValueObjectHasKey(value, "state")) {
> - tmp = virJSONValueObjectGetString(value, "state");
Hm calling this without checking should be safe as the
virJSONValueObjectHasKey basically guarantees successful return of a
value in virJSONValueObjectGetString.
> + if (!(tmp = virJSONValueObjectGetString(value, "state"))) {
> + parallelsParseError();
> + goto error;
> + }
> if STREQ(tmp, "disconnected")
Another three possible solutions, both shorter, for silencing Coverity
would be:
1. Add in a null check (slight speed penalty, though):
if (virJSONValueObjectHasKey(value, "state")) {
tmp = virJSONValueObjectGetString(value, "state");
if STREQ_NULLABLE(tmp, "disconnected")
2. use ga_assert() to tell Coverity something that we already know:
if (virJSONValueObjectHasKey(value, "state")) {
tmp = virJSONValueObjectGetString(value, "state");
ga_assert(tmp);
if STREQ(tmp, "disconnected")
3. My favorite - use fewer function calls to begin with:
if ((tmp = virJSONValueObjectGetString(value, "state")) &&
STREQ(tmp, "disconnected"))
I give ACK to this patch, but it shouldn't be a problem to go
without it.
I think a v2 would be appropriate with a shorter solution.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
8:30 a.m.
New subject: [libvirt] [PATCH v2 4/10] parallels: Need to handle virJSONValueObjectGetString error
---
src/parallels/parallels_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index ea193af..1b47246 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -513,10 +513,9 @@ parallelsGetNetInfo(virDomainNetDefPtr net,
}
net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_UP;
- if (virJSONValueObjectHasKey(value, "state")) {
- tmp = virJSONValueObjectGetString(value, "state");
- if STREQ(tmp, "disconnected")
- net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
+ if ((tmp = virJSONValueObjectGetString(value, "state")) &&
+ STREQ(tmp, "disconnected")) {
+ net->linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DOWN;
}
return 0;
--
1.7.11.7
1:17 p.m.
New subject: [libvirt] [PATCH 05/10] util: Need to check child JSON allocation before use
---
src/util/virlockspace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 163404f..fd89598 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -483,6 +483,9 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
virJSONValuePtr owners = NULL;
size_t i;
+ if (!child)
+ goto error;
+
if (virJSONValueArrayAppend(resources, child) < 0) {
virJSONValueFree(child);
goto error;
--
1.7.11.7
2:05 p.m.
New subject: [libvirt] [PATCH 05/10] util: Need to check child JSON allocation before use
On 01/17/13 20:17, John Ferlan wrote:
---
src/util/virlockspace.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 163404f..fd89598 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -483,6 +483,9 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
virJSONValuePtr owners = NULL;
size_t i;
+ if (!child)
+ goto error;
No error is raised if this code path is taken resulting into a strange
error message. a virReportOOMError() should be reported here.
+
if (virJSONValueArrayAppend(resources, child) < 0) {
virJSONValueFree(child);
goto error;
Peter
8:31 a.m.
New subject: [libvirt] [PATCH v2 5/10] util: Need to check child JSON allocation before use
---
src/util/virlockspace.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
index 163404f..4ff0f3a 100644
--- a/src/util/virlockspace.c
+++ b/src/util/virlockspace.c
@@ -459,8 +459,10 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
virJSONValuePtr resources;
virHashKeyValuePairPtr pairs = NULL, tmp;
- if (!object)
+ if (!object) {
+ virReportOOMError();
return NULL;
+ }
virMutexLock(&lockspace->lock);
@@ -483,6 +485,11 @@ virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr
lockspace)
virJSONValuePtr owners = NULL;
size_t i;
+ if (!child) {
+ virReportOOMError();
+ goto error;
+ }
+
if (virJSONValueArrayAppend(resources, child) < 0) {
virJSONValueFree(child);
goto error;
--
1.7.11.7
1:17 p.m.
New subject: [libvirt] [PATCH 06/10] qemu: Check valid activeDev before calling pciDeviceSetUsedBy
---
src/qemu/qemu_hostdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 896ad9b..ba6dadd 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -511,7 +511,8 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
dev = pciDeviceListGet(pcidevs, i);
activeDev = pciDeviceListFind(driver->activePciHostdevs, dev);
- pciDeviceSetUsedBy(activeDev, name);
+ if (activeDev)
+ pciDeviceSetUsedBy(activeDev, name);
}
/* Loop 8: Now set the original states for hostdev def */
--
1.7.11.7
2:31 p.m.
New subject: [libvirt] [PATCH 06/10] qemu: Check valid activeDev before calling pciDeviceSetUsedBy
On 01/17/13 20:17, John Ferlan wrote:
---
src/qemu/qemu_hostdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 896ad9b..ba6dadd 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -511,7 +511,8 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
dev = pciDeviceListGet(pcidevs, i);
activeDev = pciDeviceListFind(driver->activePciHostdevs, dev);
- pciDeviceSetUsedBy(activeDev, name);
+ if (activeDev)
+ pciDeviceSetUsedBy(activeDev, name);
Strange that no-one complained about this. This is either a false
positive or nobody used that yet :)
ACK
Peter
1:17 p.m.
New subject: [libvirt] [PATCH 07/10] selinux: Resolve resource leak using the default disk label
Commit id a994ef2d1 changed the mechanism to store/update the default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label.
---
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 +++---
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b9ba13..0b3427f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16064,3 +16064,32 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char
*model)
return seclabel;
}
+
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+{
+ virSecurityDeviceLabelDefPtr seclabel = NULL;
+
+ if (VIR_ALLOC(seclabel) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+
+ if (model) {
+ seclabel->model = strdup(model);
+ if (seclabel->model == NULL) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ return NULL;
+ }
+ }
+
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ return NULL;
+ }
+ def->seclabels[def->nseclabels - 1] = seclabel;
+
+ return seclabel;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce36003..9770ffb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const
char *model);
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b5e1a9a..511e923 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- if (VIR_ALLOC(disk_seclabel) < 0) {
- virReportOOMError();
+ disk_seclabel =
+ virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ if (!disk_seclabel)
return -1;
- }
disk_seclabel->norelabel = true;
ret = 0;
}
--
1.7.11.7
1:27 p.m.
New subject: [libvirt] [PATCH 07/10] selinux: Resolve resource leak using the default disk label
On 01/17/13 20:17, John Ferlan wrote:
Commit id a994ef2d1 changed the mechanism to store/update the
default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label.
---
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 +++---
3 files changed, 35 insertions(+), 3 deletions(-)
This patch might be the fix for:
https://bugzilla.redhat.com/show_bug.cgi?id=891324 and
https://bugzilla.redhat.com/show_bug.cgi?id=891324
6:16 a.m.
New subject: [libvirt] [PATCH 07/10] selinux: Resolve resource leak using the default disk label
On 01/17/13 20:17, John Ferlan wrote:
Commit id a994ef2d1 changed the mechanism to store/update the
default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label.
---
src/conf/domain_conf.c | 29 +++++++++++++++++++++++++++++
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 +++---
3 files changed, 35 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b9ba13..0b3427f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16064,3 +16064,32 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char
*model)
return seclabel;
}
+
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+{
+ virSecurityDeviceLabelDefPtr seclabel = NULL;
+
+ if (VIR_ALLOC(seclabel) < 0) {
+ virReportOOMError();
all ...
+ return NULL;
+ }
+
+ if (model) {
+ seclabel->model = strdup(model);
+ if (seclabel->model == NULL) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
.. these ..
+ return NULL;
+ }
+ }
+
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
... error paths can be merged under a no_memory label saving a few lines
of code. (virSecurityDeviceLabelDefFree handles NULL arguments gracefully)
+ return NULL;
+ }
+ def->seclabels[def->nseclabels - 1] = seclabel;
+
+ return seclabel;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce36003..9770ffb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const
char *model);
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b5e1a9a..511e923 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- if (VIR_ALLOC(disk_seclabel) < 0) {
- virReportOOMError();
+ disk_seclabel =
+ virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ if (!disk_seclabel)
return -1;
- }
disk_seclabel->norelabel = true;
ret = 0;
}
Otherwise this seems to me be a good approach how to fix it, but I'm not
that familiar with the selinux code. Hopefully, if this fix is wrong
somebody will chime in on the v2.
Peter
8:34 a.m.
New subject: [libvirt] [PATCH v2 7/10] selinux: Resolve resource leak using the default disk label
Commit id a994ef2d1 changed the mechanism to store/update the default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
a copy of the virDomainDefAddSecurityLabelDef().
---
src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++-----------
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 ++---
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b9ba13..7640af7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16041,26 +16041,51 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char
*model)
{
virSecurityLabelDefPtr seclabel = NULL;
- if (VIR_ALLOC(seclabel) < 0) {
- virReportOOMError();
- return NULL;
- }
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
if (model) {
seclabel->model = strdup(model);
- if (seclabel->model == NULL) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
- }
+ if (seclabel->model == NULL)
+ goto no_memory;
}
- if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
+ def->seclabels[def->nseclabels - 1] = seclabel;
+
+ return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityLabelDefFree(seclabel);
+ return NULL;
+}
+
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+{
+ virSecurityDeviceLabelDefPtr seclabel = NULL;
+
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
+
+ if (model) {
+ seclabel->model = strdup(model);
+ if (seclabel->model == NULL)
+ goto no_memory;
}
+
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
def->seclabels[def->nseclabels - 1] = seclabel;
return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ return NULL;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce36003..9770ffb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const
char *model);
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b5e1a9a..511e923 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- if (VIR_ALLOC(disk_seclabel) < 0) {
- virReportOOMError();
+ disk_seclabel =
+ virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ if (!disk_seclabel)
return -1;
- }
disk_seclabel->norelabel = true;
ret = 0;
}
--
1.7.11.7
8:56 a.m.
New subject: [libvirt] [PATCH v2 7/10] selinux: Resolve resource leak using the default disk label
The fix seems correct to me.
On Fri, Jan 18, 2013 at 09:34:13AM -0500, John Ferlan wrote:
Commit id a994ef2d1 changed the mechanism to store/update the
default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
a copy of the virDomainDefAddSecurityLabelDef().
---
src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++-----------
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 ++---
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b9ba13..7640af7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16041,26 +16041,51 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char
*model)
{
virSecurityLabelDefPtr seclabel = NULL;
- if (VIR_ALLOC(seclabel) < 0) {
- virReportOOMError();
- return NULL;
- }
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
if (model) {
seclabel->model = strdup(model);
- if (seclabel->model == NULL) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
- }
+ if (seclabel->model == NULL)
+ goto no_memory;
}
- if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
+ def->seclabels[def->nseclabels - 1] = seclabel;
+
+ return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityLabelDefFree(seclabel);
+ return NULL;
+}
+
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+{
+ virSecurityDeviceLabelDefPtr seclabel = NULL;
+
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
+
+ if (model) {
+ seclabel->model = strdup(model);
+ if (seclabel->model == NULL)
+ goto no_memory;
}
+
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
def->seclabels[def->nseclabels - 1] = seclabel;
return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ return NULL;
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce36003..9770ffb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const
char *model);
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b5e1a9a..511e923 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- if (VIR_ALLOC(disk_seclabel) < 0) {
- virReportOOMError();
+ disk_seclabel =
+ virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ if (!disk_seclabel)
return -1;
- }
disk_seclabel->norelabel = true;
ret = 0;
}
--
1.7.11.7
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
7:33 a.m.
New subject: [libvirt] [PATCH v2 7/10] selinux: Resolve resource leak using the default disk label
On 01/18/13 15:34, John Ferlan wrote:
Commit id a994ef2d1 changed the mechanism to store/update the
default
security label from using disk->seclabels[0] to allocating one on the
fly. That change allocated the label, but never saved it. This patch
will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
a copy of the virDomainDefAddSecurityLabelDef().
---
src/conf/domain_conf.c | 51 ++++++++++++++++++++++++++++++-----------
src/conf/domain_conf.h | 3 +++
src/security/security_selinux.c | 6 ++---
3 files changed, 44 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0b9ba13..7640af7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -16041,26 +16041,51 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char
*model)
{
virSecurityLabelDefPtr seclabel = NULL;
- if (VIR_ALLOC(seclabel) < 0) {
- virReportOOMError();
- return NULL;
- }
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
if (model) {
seclabel->model = strdup(model);
- if (seclabel->model == NULL) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
- }
+ if (seclabel->model == NULL)
+ goto no_memory;
}
- if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
- virReportOOMError();
- virSecurityLabelDefFree(seclabel);
- return NULL;
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
+ def->seclabels[def->nseclabels - 1] = seclabel;
+
+ return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityLabelDefFree(seclabel);
+ return NULL;
+}
This first part would be better in a separate patch ...
+
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
+{
+ virSecurityDeviceLabelDefPtr seclabel = NULL;
+
+ if (VIR_ALLOC(seclabel) < 0)
+ goto no_memory;
+
+ if (model) {
+ seclabel->model = strdup(model);
+ if (seclabel->model == NULL)
+ goto no_memory;
}
+
+ if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
+ goto no_memory;
+
def->seclabels[def->nseclabels - 1] = seclabel;
return seclabel;
+
+no_memory:
+ virReportOOMError();
+ virSecurityDeviceLabelDefFree(seclabel);
+ return NULL;
}
but I see why you did it.
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ce36003..9770ffb 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const
char *model);
virSecurityLabelDefPtr
virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
+virSecurityDeviceLabelDefPtr
+virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
+
typedef const char* (*virEventActionToStringFunc)(int type);
typedef int (*virEventActionFromStringFunc)(const char *type);
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index b5e1a9a..511e923 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
if (ret == 1 && !disk_seclabel) {
/* If we failed to set a label, but virt_use_nfs let us
* proceed anyway, then we don't need to relabel later. */
- if (VIR_ALLOC(disk_seclabel) < 0) {
- virReportOOMError();
+ disk_seclabel =
+ virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
+ if (!disk_seclabel)
return -1;
- }
disk_seclabel->norelabel = true;
ret = 0;
}
ACK.
Peter
1:17 p.m.
New subject: [libvirt] [PATCH 08/10] esx: No need to check for objectSpec
Coverity complains that the objectSpec != NULL check was unnecessary because
there was no way to get to the label with objectSpec = NULL.
---
src/esx/esx_vi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 99c1eb1..d84ce19 100644
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -2168,11 +2168,10 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
/*
* Remove values given by the caller from the data structures to prevent
* them from being freed by the call to esxVI_PropertyFilterSpec_Free().
+ * objectSpec cannot be NULL here.
*/
- if (objectSpec != NULL) {
- objectSpec->obj = NULL;
- objectSpec->selectSet = NULL;
- }
+ objectSpec->obj = NULL;
+ objectSpec->selectSet = NULL;
if (propertySpec != NULL) {
propertySpec->type = NULL;
--
1.7.11.7
1:52 p.m.
New subject: [libvirt] [PATCH 08/10] esx: No need to check for objectSpec
On 01/17/13 20:17, John Ferlan wrote:
Coverity complains that the objectSpec != NULL check was unnecessary
because
there was no way to get to the label with objectSpec = NULL.
---
src/esx/esx_vi.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
Yep, commit 041aac86 made that code path impossible.
ACK
Peter
1:17 p.m.
New subject: [libvirt] [PATCH 09/10] virobject: Remove the bogus ! from call to virObjectInitialize()
---
src/util/virobject.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 838b5cd..7f08a11 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -74,7 +74,7 @@ VIR_ONCE_GLOBAL_INIT(virObject);
*/
virClassPtr virClassForObject(void)
{
- if (!virObjectInitialize() < 0)
+ if (virObjectInitialize() < 0)
return NULL;
return virObjectClass;
@@ -88,7 +88,7 @@ virClassPtr virClassForObject(void)
*/
virClassPtr virClassForObjectLockable(void)
{
- if (!virObjectInitialize() < 0)
+ if (virObjectInitialize() < 0)
return NULL;
return virObjectLockableClass;
--
1.7.11.7
1:49 p.m.
New subject: [libvirt] [PATCH 09/10] virobject: Remove the bogus ! from call to virObjectInitialize()
On 01/17/13 20:17, John Ferlan wrote:
---
src/util/virobject.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virobject.c b/src/util/virobject.c
index 838b5cd..7f08a11 100644
--- a/src/util/virobject.c
+++ b/src/util/virobject.c
@@ -74,7 +74,7 @@ VIR_ONCE_GLOBAL_INIT(virObject);
*/
virClassPtr virClassForObject(void)
{
- if (!virObjectInitialize() < 0)
+ if (virObjectInitialize() < 0)
Funny bug :) either of those works, but not together.
ACK
Peter
1:17 p.m.
New subject: [libvirt] [PATCH 10/10] network: Remove dead code getting, but not using ipdef
The fetch of 'ipdef' in networkRefreshDhcpDaemon() when the loop to fill
in ipv4def fails to find an ipv4 address with dhcp defined. The filled in
ipdef value was not used. Code was made unnecessary with commit it 2d5cd1.
---
src/network/bridge_driver.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 6f3c839..268dada 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1156,9 +1156,6 @@ networkRefreshDhcpDaemon(struct network_driver *driver,
if (!ipv4def && (ipdef->nranges || ipdef->nhosts))
ipv4def = ipdef;
}
- /* If no IPv4 addresses had dhcp info, pick the first (if there were any). */
- if (!ipdef)
- ipdef = virNetworkDefGetIpByIndex(network->def, AF_INET, 0);
ipv6def = NULL;
for (ii = 0;
--
1.7.11.7
1:44 p.m.
New subject: [libvirt] [PATCH 10/10] network: Remove dead code getting, but not using ipdef
On 01/17/13 20:17, John Ferlan wrote:
The fetch of 'ipdef' in networkRefreshDhcpDaemon() when the
loop to fill
in ipv4def fails to find an ipv4 address with dhcp defined. The filled in
ipdef value was not used. Code was made unnecessary with commit it 2d5cd1.
---
src/network/bridge_driver.c | 3 ---
1 file changed, 3 deletions(-)
ACK. I will push the patch later.
Peter
5:01 p.m.
I pushed patches 6,8,9 and 10 and I will review patch 7 tomorrow. The
others will require a V2.
Peter
4320
days inactive
4327
days old
37 comments
4 participants
participants (4)
-
Eric Blake -
John Ferlan -
Marcelo Cerri -
Peter Krempa