Bug 1531066

Summary:	vdsm: use INFO log level for all operation that change system state
Product:	[oVirt] vdsm	Reporter:	Nir Soffer <nsoffer>
Component:	Core	Assignee:	Eyal Shenitzky <eshenitz>
Status:	CLOSED CURRENTRELEASE	QA Contact:	Evelina Shames <eshames>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	4.20.15	CC:	amureini, bugs, nsoffer, ylavi
Target Milestone:	ovirt-4.2.2	Flags:	rule-engine: ovirt-4.2+ ylavi: exception+
Target Release:	4.20.20
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	v4.20.20	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-04-18 12:26:27 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	Storage	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Nir Soffer 2018-01-04 13:55:42 UTC

Description of problem:

Some storage operations like removing a directory (e.g. fileUtils.cleanupdir)
log the operation only in DEBUG level. This makes debugging issue like missing
directories in /run/vdsm/storage hard (e.g bug 1527394).

In QE setup, it is easy to reproduce with debug logs. In the field, not having
the logs decrease the chance to understand the issue and increase the cost of
supporting the system.

Version-Release number of selected component (if applicable):
4.1 (we moved to INFO logs by default)

How reproducible:
Always

Steps to Reproduce:
1. Run flows that call fileUtils.cleanupdir

Actual results:
Removing directory is not logged.

Expected results:
Removing directory should be logged.

Additional info:
Removing a directory is just one example, we should audit the code and ensure that
any operation changing system state is logged in INFO level.

Comment 1 Nir Soffer 2018-01-04 13:56:43 UTC

Suggest for 4.2.1 as this is easy and low risk change.

Comment 2 Nir Soffer 2018-01-04 16:26:19 UTC

In HSM.__cleanStorageRepository, we have no logs about removed directories and
files. I don't think it should be related to bug 1527394 unless we have code
creating link from /rhev/data-center/ to /run/vdsm/storage instead of the correct
link from /run/vdsm/storage to /rhev/data-center, but we should at least have logs
for the cleanup code.

Comment 3 Nir Soffer 2018-01-04 16:35:53 UTC

Logs also missing in:
- BlockVolume.validateImagePath - remove symlink, create directory
- BlockVolume.validateVolumePath - create symlink

Comment 4 Eyal Shenitzky 2018-01-14 10:30:54 UTC

Hi Nir,

I am not sure what is the bug purpose, is it about refactor the DEBUG log messages to INFO level if the operations change the system state? or, is it about adding missing log messages?

Comment 5 Nir Soffer 2018-01-14 10:39:52 UTC

(In reply to Eyal Shenitzky from comment #4)
> Hi Nir,
> 
> I am not sure what is the bug purpose, is it about refactor the DEBUG log
> messages to INFO level 

This is not a refactoring, refactoring is changing the code without changing
the behavior.

> if the operations change the system state? or, is it
> about adding missing log messages?

Both, we need info log when we add/remove files and directories. The logs we have
should use INFO level, and the log we don't have should be added.

Comment 6 Eyal Shenitzky 2018-02-19 13:03:36 UTC

Move back to post due to fixes that should add to the patch.

Comment 7 Allon Mureinik 2018-02-27 14:19:00 UTC

This patch isn't included in any tag yet, moving back to MODIFIED

Comment 8 Evelina Shames 2018-04-10 12:42:27 UTC

Verified.
vdsm-4.20.22
ovirt-engine-4.2.2.6

Comment 9 Sandro Bonazzola 2018-04-18 12:26:27 UTC

This bugzilla is included in oVirt 4.2.2 release, published on March 28th 2018.

Since the problem described in this bug report should be
resolved in oVirt 4.2.2 release, it has been closed with a resolution of CURRENT RELEASE.

If the solution does not work for you, please open a new bug report.