Change in vdsm[master]: fileUtils: Remove useless warning when creating directories

nsoffer at redhat.com nsoffer at redhat.com
Mon Jan 12 17:50:28 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: fileUtils: Remove useless warning when creating directories
......................................................................


Patch Set 3:

> Why do we call fileUtils.createdir() despite the pre-existence of the directory?

Because we don't care if the directory exists or not before the call. The purpose of this function is to ensure that the directory exists after the call.

If the caller care about existence of a directory, it should not call this function, and handle the EEXIST error. This patch does not change the semantic.

> The warning can be useful in order to track these sloppy pieces of code and fix them.

This is not the purpose of warnings. To track sloppy code, we will use the debug messages. We don't have to punish the users and make the product look bad because there may be sloppy code.

> The fact that we have not done that yet does not make the warning "useless". There's use for mkdir(2)'s EEXIST errno.

What makes this warning useless is that this code does not have enough context to tell if this this is normal condition. Only the caller have enough context about this.

> Can you list flows that reproduce this warning on a normal flow? They should be added to http://www.ovirt.org/Vdsm_TODO .

No. I don't see this this is needed and we don't have resources for this.

> With such a list, and a nicer statement such as: "right now we have no resources to solve the problems exposed by the warning", we can consider reducing the "warning" to "info".

Info log level should not be used for such messages. It should be used for important events.

I will update the commit message to make this more clear.

-- 
To view, visit http://gerrit.ovirt.org/36764
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7800b860eb81334b63abefcca9a21a552331458f
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No


More information about the vdsm-patches mailing list