Change in vdsm[master]: Added blkid tool into constants.

Hello Saggi Mizrahi, Dan Kenigsberg,

I'd like you to do a code review.  Please visit

http://gerrit.ovirt.org/2185

to review the following change.

Change subject: Added blkid tool into constants.
......................................................................

Added blkid tool into constants.

blkid tool is used to get block device attributes

Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Signed-off-by: Bala.FA 
---
M configure.ac
M vdsm/constants.py.in
M vdsm/sudoers.vdsm.in
3 files changed, 3 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/85/2185/1
--
To view, visit http://gerrit.ovirt.org/2185
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool into constants.
......................................................................

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

Hi Bala,

Just adding constants is a bit pointless - we'd better include some code that uses them. Particularly when this adds stuff to our evil sudoers file.

Best practice would be to add a blkid.py file exposing blkid functionality in python. Then, preferably add a superVdsm function that uses it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 2: (2 inline comments)

two minor comments

....................................................
File vdsm/blkid.py
Line 29:         rc, out, err = utils.execCmd([constants.EXT_BLKID, "-c", "/dev/null"],
this case is redundant - it is just like the former when devList is empty.

Line 31:     if rc:
it would be more pythonic to raise a BlockIDError exception here, I believe.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

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

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 33:     blkIdDict = {}
let's keep unittest'ing in mind: how about separating the parsing to a different function, so we can add a little test for its handling of blkid's output?

P.S. are we sure there's no evil escaping issues in blkid's output?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 2: (3 inline comments)

....................................................
File vdsm/blkid.py
Line 29:         rc, out, err = utils.execCmd([constants.EXT_BLKID, "-c", "/dev/null"],
That's right.  How do we react on empty value like None,(),'' etc passed for devList?  Can we ignore and let exception occurs?

Line 31:     if rc:
Done

Line 33:     blkIdDict = {}
Parsing will be done in a separate function. It works fine for 'blkid -c /dev/null' output.
I would like to know is whether we need to consider non-blkid output and other variant output of blkid?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 33:     blkIdDict = {}
it should handle the craziest output of Fedora blkid, and raise an exception in any other case.

the man page for blkid speaks about
       -d     Don't  encode  non-printing characters. The non-printing charac‐
              ters are encoded by ^ and M- notation by default.

You should decode this, or at least add a comment why this is not important for our use case.

What happens if a disk label has the space or the double-quote character inside it? It would break your parsing if space is not encoded by blkid (I do not know if it is).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 33:     blkIdDict = {}
I found that 'blkid with no device' output is "that kind of" parse-able ie its possible to break.  I changed the function usability which returns values reliably.  New patch is on the way.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

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

(4 inline comments)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
I think out is expected to have empty lines, which would explode here

Line 49:     regex = re.compile(r'\x(\w{2})')
this should be compiled once, in import time.

Line 51:         blkIdDict[k] = regex.sub(lambda x: binascii.unhexlify(x.group()[2:]), v)
I would trust you for this decoding, as I know nothing about blkid's -o udev. However, I do not see the benefit of replacing dict values in place.

for line in out:
  k, v = line.split
  d[k] = decode(v)

would be one clearer pass.

Line 51:         blkIdDict[k] = regex.sub(lambda x: binascii.unhexlify(x.group()[2:]), v)
I would trust you for this decoding, as I know nothing about blkid's -o udev. However, I do not see the benefit of replacing dict values in place.

for line in out:
  k, v = line.split
  d[k] = decode(v)

would be one clearer pass.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 3: (3 inline comments)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
'blkid -c /dev/null -o udev DEVICE' doesn't give blank lines.  Do I need to consider other string values or other blkid formats here?

Line 49:     regex = re.compile(r'\x(\w{2})')
Done

Line 51:         blkIdDict[k] = regex.sub(lambda x: binascii.unhexlify(x.group()[2:]), v)
Done

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 3: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
oh, I did not notice that you've dropped the multiple-device/ all-device option and move to a single device. Why have you done that? Wouldn't it waste too much executions of blkid?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 3: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
parser for default output of blkid for multiple devices is breakable.  Its possible to have keyword/delimiters/quotes in device label.

Similarly udev format for multiple devices is not usable (We don't get device name itself in the output)

Reliable way is that getting properties of single block device ie known block device

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 3: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
from what I read in the man page, the standard output has some quoting of funny chars, you just need to understand that encoding and decode it. Maybe I am wrong, but please take a look at the blkid code (or even try a disk label with spaces etc.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 3: (1 inline comment)

....................................................
File vdsm/blkid.py
Line 48:     blkIdDict = dict(map(lambda x: x.split('=', 1), out))
You are right.  A device label can be any string including unicode.  The default blkid output does some translation.  But we are using '-o udev' format which does translation to hex for non-printing characters including '/' and white spaces.  Also '-o udev' format is udev/shell friendly.

These things were tested before submitting the patch.  However I am going to send new patch with the change you suggested.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 4: Looks good to me, but someone else must approve

Don't youmind the need to call getBlkId for each and every device?

How many devices are expected to be using this function? maybe if you follow this patch with another one giving me a fuller picture, would make my understanding easier.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 4:

blkid doesn't give info of each and every devices (ie not all) and it just prints information of devices those are probed by blkid.

Its upto the user to decide to which device blkid info is needed, eg, blkid info of devices in /proc/partitions can be retrieved by parsing /proc/partitions for devices, lvdisplay for lvm etc.

with open("/proc/partitions") as f:
    f.next()
    f.next()
    yield getBlkId('/dev/' + f.next().split()[-1])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Eduardo has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

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

Please unify clientIF._getUUIDSpecPath() with uses blkid too.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 4:

I simply fear that if there are tens (hundreds?) of partitions, we'd end up executing blkid too many times.

p.s. the piece of code above could be made prettier with a helper function that just reads /proc/partitions

def partitionDevices():
  with open("/proc/partitions") as f:
     f.next()
     f.next()
     for line in f:
       yield line.split()[-1]

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 4:

Dan:
I do have the same feeling about repeated execution of blkid command.  I am going to ask blkid developer list about encoding label and other stuff for all devices.

Eduardo:
I will add this functionality

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 4:

After discussion with util-linux people regarding blkid, there is another interesting tool called lsblk which gives attributes of all devices.

However lsblk doesn't work like blkid for clientIF._getUUIDSpecPath().  There are two solutions.

1. Use blkid for this purpose.
2. get all devices using lsblk and locate device for uuid.

Expecting your suggestion.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

Patch Set 5:

This patch uses blkid for device lookup by uuid.  However gluster extension will use lsblk instead of blkid.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support in supervdsm.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support in supervdsm.
......................................................................

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

(1 inline comment)

I know this was not your original intention, but if your doing this, let's do it correctly until completion.

....................................................
File vdsm/clientIF.py
Line 164:             return svdsm.getProxy().getDeviceByUuid(uuid)
actually, this use case does not need root user.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid. getDeviceBy...

Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid. getDeviceByUuid - accepts uuid and returns block device of the uuid.
......................................................................

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

(2 inline comments)

Just minor style points.

....................................................
Commit Message
Line 8: getDeviceByUuid - accepts uuid and returns block device of the uuid.
The convention is to leave an empty line between the first and second

....................................................
File vdsm/blkid.py
Line 40:             return self.message
Inconsistent indenting

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid. getDeviceBy...

Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid. getDeviceByUuid - accepts uuid and returns block device of the uuid.
......................................................................

Patch Set 7: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid. getDeviceBy...

Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid. getDeviceByUuid - accepts uuid and returns block device of the uuid.
......................................................................

Patch Set 7: No score

(1 inline comment)

....................................................
Commit Message
Line 8: getDeviceByUuid - accepts uuid and returns block device of the uuid.
I see you still missed this one.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

Ewoud Kohl van Wijngaarden has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Patch Set 8: Looks good to me, but someone else must approve

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Patch Set 9: Looks good to me, but someone else must approve

(1 inline comment)

Bala, I did minor changes for your patch since I suppose you are tired of it just as I am. All I wanted is to avoid supervdsm for the clientIF call...

Never mind. Please ack if you're happy with the current patch.

....................................................
File vdsm/clientIF.py
Line 170:             self.log.info('Error finding path for device', exc_info=True)
calling str() is waiting for a unicode error to happen in the future.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Patch Set 9: (1 inline comment)

....................................................
File vdsm/clientIF.py
Line 170:             self.log.info('Error finding path for device', exc_info=True)
Got it.  BTW there is a typo in 'except' line.  I will fix and send new patch :)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

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

fixing my typos is great, but why undo my sorting, too?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

Bala.FA has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Patch Set 10:

Oh! Sorry, I didn't notice those changes.  Sending new patch set.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Added blkid tool support to get device for uuid.

getDeviceByUuid accepts uuid and returns block device of the uuid.

Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Signed-off-by: Bala.FA 
---
M configure.ac
M vdsm.spec.in
M vdsm/Makefile.am
A vdsm/blkid.py
M vdsm/clientIF.py
M vdsm/constants.py.in
6 files changed, 56 insertions(+), 6 deletions(-)

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

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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi

Change in vdsm[master]: Added blkid tool support to get device for uuid.

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Added blkid tool support to get device for uuid.
......................................................................

Patch Set 11: Verified; Looks good to me, approved

Thanks! Please add yourself as an AUTHOR.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ad51df4f4b98e2aff5db1d9c02512045977a60
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Bala.FA 
Gerrit-Reviewer: Bala.FA 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Ewoud Kohl van Wijngaarden 
Gerrit-Reviewer: Saggi Mizrahi