Francesco Romani has posted comments on this change.

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


Patch Set 6:

(2 comments)

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")
> I log explicit elapsed time for actions that may block.
Sure, and this is nice, but elapsed time tells us how much it took _after_ an 
operation is done, or canceled. Timestamp in log messages helps reminding when 
an operation is started, so one can easily see how much an operation is taking, 
while it is running.

While I like that, I cannot really claim this is objectively "friendlier" or 
"helpful", so I don't really mind if code stays as it is.
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
> I can use 1 for general errors (what you get when exception is unhandled), 
If you think this doesn't help troubleshooting, I'm fine with existing error 
codes: I'm not experienced enough with multipath.
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