Francesco Romani has posted comments on this change.

Change subject: storage: Add multipath-resize command line tool
......................................................................


Patch Set 6: Code-Review+1

(2 comments)

seems fine, minor possible improvements inside

https://gerrit.ovirt.org/#/c/38467/6/vdsm/storage/multipath-resize
File vdsm/storage/multipath-resize:

Line 81:         args.remove("-v")
Line 82:     else:
Line 83:         loglevel = logging.INFO
Line 84: 
Line 85:     logging.basicConfig(level=loglevel, format="%(name)s: %(message)s")
To debug timeouts and other time-related issues, could be useful to add 
timestamps
Line 86: 
Line 87:     if not args:
Line 88:         log.error("No device name specified")
Line 89:         return 2


Line 101:         log.error("Resizing device %s failed: %s", guid, e)
Line 102:         return 1
Line 103:     except Exception:
Line 104:         log.exception("Unexpected error")
Line 105:         return 1
Maybe add different return code?
Line 106: 
Line 107:     return 0
Line 108: 
Line 109: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I229ef55555fa757989329939a9267041785f2c0f
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Ala Hino <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Darshan N <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Fred Rolland <[email protected]>
Gerrit-Reviewer: Freddy Rolland <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to