[libvirt] [PATCH 0/2] Fix some Coverity issues
*** BLURB HERE *** Michal Privoznik (2): qemuMigrationRun: Don't leak @fd cmdVcpuPin: Remove dead code src/qemu/qemu_migration.c | 2 +- tools/virsh-domain.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.3.6
If we are migrating to an UNIX socket, we accept() a connection from qemu and use that FD to set up a tunnel. However, the FD is not closed as often as it should be. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_migration.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index d789110..f5866c4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4375,8 +4375,8 @@ qemuMigrationRun(virQEMUDriverPtr driver, if (spec->fwdType != MIGRATION_FWD_DIRECT) { if (iothread && qemuMigrationStopTunnel(iothread, ret < 0) < 0) ret = -1; - VIR_FORCE_CLOSE(fd); } + VIR_FORCE_CLOSE(fd); if (priv->job.completed) { qemuDomainJobInfoUpdateTime(priv->job.completed); -- 2.3.6
There's this condition: flags & VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom) which can never be true since VIR_DOMAIN_AFFECT_CURRENT has hardcoded value of zero. Therefore virDomainIsActive() is a dead code. However, the condition could make sense if it is rewritten as the following: !(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ac04ded..f7edeeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6499,7 +6499,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd) if (got_vcpu && vcpu >= ncpus) { if (flags & VIR_DOMAIN_AFFECT_LIVE || - (flags & VIR_DOMAIN_AFFECT_CURRENT && + (!(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom) == 1)) vshError(ctl, _("vcpu %d is out of range of live cpu count %d"), -- 2.3.6
On 07/14/2015 10:37 AM, Michal Privoznik wrote:
There's this condition:
flags & VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom)
which can never be true since VIR_DOMAIN_AFFECT_CURRENT has hardcoded value of zero. Therefore virDomainIsActive() is a dead code. However, the condition could make sense if it is rewritten as the following:
!(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
I had seen and filtered locally but didn't send since I was working through perhaps removing some sa_assert()'s
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ac04ded..f7edeeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6499,7 +6499,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (got_vcpu && vcpu >= ncpus) { if (flags & VIR_DOMAIN_AFFECT_LIVE || - (flags & VIR_DOMAIN_AFFECT_CURRENT && + (!(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom) == 1))
Wouldn't another option be: (current && virDomainIsActive(dom) == 1) Which is what I think was trying to be tested anyway ACK to both patches in any case, John
vshError(ctl, _("vcpu %d is out of range of live cpu count %d"),
On Tue, Jul 14, 2015 at 16:37:01 +0200, Michal Privoznik wrote:
There's this condition:
flags & VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom)
which can never be true since VIR_DOMAIN_AFFECT_CURRENT has hardcoded value of zero. Therefore virDomainIsActive() is a dead code. However, the condition could make sense if it is rewritten as the following:
!(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ac04ded..f7edeeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6499,7 +6499,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (got_vcpu && vcpu >= ncpus) { if (flags & VIR_DOMAIN_AFFECT_LIVE || - (flags & VIR_DOMAIN_AFFECT_CURRENT && + (!(flags & VIR_DOMAIN_AFFECT_CONFIG) &&
This still is dead code since VIR_DOMAIN_AFFECT_CONFIG is 0. If you mask it with any flags you'll get 0, then invert it, it's always true. Peter
On Wed, Jul 15, 2015 at 09:08:45 +0200, Peter Krempa wrote:
On Tue, Jul 14, 2015 at 16:37:01 +0200, Michal Privoznik wrote:
There's this condition:
flags & VIR_DOMAIN_AFFECT_CURRENT && virDomainIsActive(dom)
which can never be true since VIR_DOMAIN_AFFECT_CURRENT has hardcoded value of zero. Therefore virDomainIsActive() is a dead code. However, the condition could make sense if it is rewritten as the following:
!(flags & VIR_DOMAIN_AFFECT_CONFIG) && virDomainIsActive(dom)
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tools/virsh-domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ac04ded..f7edeeb 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6499,7 +6499,7 @@ cmdVcpuPin(vshControl *ctl, const vshCmd *cmd)
if (got_vcpu && vcpu >= ncpus) { if (flags & VIR_DOMAIN_AFFECT_LIVE || - (flags & VIR_DOMAIN_AFFECT_CURRENT && + (!(flags & VIR_DOMAIN_AFFECT_CONFIG) && ^^^^^^ Erm, never mind.
This still is dead code since VIR_DOMAIN_AFFECT_CONFIG is 0. If you mask it with any flags you'll get 0, then invert it, it's always true.
Peter
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
John Ferlan -
Michal Privoznik -
Peter Krempa