Change in ovirt-engine[ovirt-engine-4.1]: core: sync ClientIp and consoleCurrentUserName values

Code Review gerrit at ovirt.org
Wed May 17 08:24:24 UTC 2017


>From Tal Nisan <tnisan at redhat.com>:

Tal Nisan has submitted this change and it was merged.

Change subject: core: sync ClientIp and consoleCurrentUserName values
......................................................................


core: sync ClientIp and consoleCurrentUserName values

There are two issues solved in this patch regarding the update of
vm.clientIp (sent by VDSM) and consoleCurrentUserName, consoleUserId
(done in SetVmTicketCommand):

1. As part of resetVmAttributes() - the consoleCurrentUserName and consoleUserId
are reset but clientIP is not.
The clientp is reset only when vdsm sent it as null for the VM (after
identifying that spice/vnc console connection was closed).
This is not a healthy situation since when the VM is down the clientIp is
set in DB while console user information in DB is null.

Fixing it is done by reset ClientIp as part of resetVmAttributes().

2. There is no vm locking done in SetVmTicketCommand, so there might be a
scenario in which after starting a VM and opening a console, the console
user info is still set to null for this opened console connection.
The scenario is:
a. the VmsMonitoring read the vmDb.consoleCurrentUserName and vmDb.consoleUserId
   as null
b. in the middle and before the VmAnalyzer.updateVmDynamicData() is done,
   a console is opended and SetVmTicketCommand update relevant user info in Db.
c. only then the VmAnalyzer.updateVmDynamicData() will update the
   DB - to null values that it read before.

This may lead to an open console without console user data in DB and to write in
audit log a message like: User <UNKNOWN> got disconnected from VM vm1

Fixing it is done by acquiring the vm's monitoring lock in SetVmTicketCommand.

Change-Id: I17152754a22ad499febdb88259f40aeb9b54a4fe
Bug-Url: https://bugzilla.redhat.com/1434937
Signed-off-by: Sharon Gratch <sgratch at redhat.com>
---
M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/SetVmTicketCommand.java
M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java
2 files changed, 44 insertions(+), 29 deletions(-)

Approvals:
  Jenkins CI: Passed CI tests
  Sharon Gratch: Verified
  Arik Hadas: Looks good to me, approved



-- 
To view, visit https://gerrit.ovirt.org/76895
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I17152754a22ad499febdb88259f40aeb9b54a4fe
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-4.1
Gerrit-Owner: Sharon Gratch <sgratch at redhat.com>
Gerrit-Reviewer: Arik Hadas <ahadas at redhat.com>
Gerrit-Reviewer: Jakub Niedermertl <jniederm at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Sharon Gratch <sgratch at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>


More information about the Engine-commits mailing list