[NEW PATCH] BZ#720981 - Sometimes devices have whitespace in their name file (via gerrit-bot)

New patch submitted by Saggi Mizrahi (smizrahi(a)redhat.com)

You can review this change at: http://gerrit.usersys.redhat.com/751

commit 92be0f2bb11e069bde0348c0f00edd705db6d4ad
Author: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Date:   Wed Jul 27 12:37:39 2011 +0300

    BZ#720981 - Sometimes devices have whitespace in their name file
    
    Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553

diff --git a/vdsm/storage/devicemapper.py b/vdsm/storage/devicemapper.py
index 76d31a3..ed672b7 100644
--- a/vdsm/storage/devicemapper.py
+++ b/vdsm/storage/devicemapper.py
@@ -21,7 +21,7 @@ def getDmId(deviceMultipathName):
     for nameFile in glob("/sys/block/dm-*/dm/name"):
         try:
             with open(nameFile, "r") as f:
-                if f.read() == deviceMultipathName:
+                if f.read().strip() == deviceMultipathName:
                     return nameFile.split("/")[3]
         except (IOError, OSError):
             pass

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


BZ#720981 - Sometimes devices have whitespace in their name file

Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
---
M vdsm/storage/devicemapper.py
M vdsm/storage/multipath.py
2 files changed, 3 insertions(+), 3 deletions(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved


--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 2: Verified

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 2: Looks good to me, approved

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

Saggi Mizrahi has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 2: (1 inline comment)

....................................................
File vdsm/storage/multipath.py
Line 313:                 guid = nf.read().rstrip("\n")
Yea, actually the strip was here because the original code ignored white spaces because of
how output parsing works. This is actually an improvement over all the VDSM before it.

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/multipath.py
Line 313:                 guid = nf.read().rstrip("\n")
was this the only reason you needed strip() here? Is there a third place expecting
stripped names?

This is one of these places where an elaborate commit message could have made me happier.

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

Saggi Mizrahi has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File vdsm/storage/devicemapper.py
Line 24:                 if f.read().strip() == deviceMultipathName:
Well the dev name doesn't contain the space. It's just in the file to confuse us.
I'll change it to ignore last char if it's "\n"

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com&gt;

Change in vdsm[master]: BZ#720981 - Sometimes devices have whitespace in their name ...

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: BZ#720981 - Sometimes devices have whitespace in their name file
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vdsm/storage/devicemapper.py
Line 24:                 if f.read().strip() == deviceMultipathName:
'wouldn't it be more correct to avoid stripping the device name in
multipath.getMPDevsIter() ?
Think about two devices, one named "  " and the other "     ". If
it's a legal naming, we should differentiate between the two.

--
To view, visit http://gerrit.usersys.redhat.com/751
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id99ab9b8b210616ef13b8acc7ddf2d5ce3f14553
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com&gt;
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com&gt;
Gerrit-Reviewer: Eduardo Warszawski <ewarszaw(a)redhat.com&gt;