oVirt Jenkins CI Server has posted comments on this change.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

Patch Set 1:

Build Failed

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/15071/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/14902/ : FAILURE

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/14114/ : SUCCESS

Bala.FA has posted comments on this change.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

Patch Set 1: Code-Review+1

(1 comment)

http://gerrit.ovirt.org/#/c/36237/1/vdsm/storage/glusterVolume.py File vdsm/storage/glusterVolume.py:

Line 20: Send info to represent Gluster volume as a network block device Line 21: """ Line 22: rpath = sdCache.produce(self.sdUUID).getRealPath() Line 23: volfileServer, volname = rpath.rsplit(":", 1) Line 24: volname = volname.strip('/')

As there won't be more than one '/' allowed (or possible), is it good idea

I feel my above method complicates more. Line 25: Line 26: # Volume transport to Libvirt transport mapping Line 27: VOLUME_TRANS_MAP = {'TCP': 'tcp', 'RDMA': 'rdma'} Line 28:

Adam Litke has posted comments on this change.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/36237/1/vdsm/storage/glusterVolume.py File vdsm/storage/glusterVolume.py:

Line 20: Send info to represent Gluster volume as a network block device Line 21: """ Line 22: rpath = sdCache.produce(self.sdUUID).getRealPath() Line 23: volfileServer, volname = rpath.rsplit(":", 1) Line 24: volname = volname.strip('/') why not just do:

volfileServer, volname = rpath.rsplit(':/', 1)

Then we are being explicit about there only being a single leading / Line 25: Line 26: # Volume transport to Libvirt transport mapping Line 27: VOLUME_TRANS_MAP = {'TCP': 'tcp', 'RDMA': 'rdma'} Line 28:

Federico Simoncelli has posted comments on this change.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/36237/1/vdsm/storage/glusterVolume.py File vdsm/storage/glusterVolume.py:

Line 20: Send info to represent Gluster volume as a network block device Line 21: """ Line 22: rpath = sdCache.produce(self.sdUUID).getRealPath() Line 23: volfileServer, volname = rpath.rsplit(":", 1) Line 24: volname = volname.strip('/')

why not just do:

Adam, your suggestion would make ":/" mandatory (while it's not).

I wanted to use strip because it's covering both sides of volname (right/left). So we could support without much effort: "host:name", "host:/name", "host:name/" and "host:/name/". Line 25: Line 26: # Volume transport to Libvirt transport mapping Line 27: VOLUME_TRANS_MAP = {'TCP': 'tcp', 'RDMA': 'rdma'} Line 28:

automation@ovirt.org has posted comments on this change.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

Patch Set 2:

* Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Dan Kenigsberg has submitted this change and it was merged.

Change subject: gluster: fix volume name parsing in getVmVolumeInfo ......................................................................

gluster: fix volume name parsing in getVmVolumeInfo

It is permitted to use slashes '/' in gluster mount strings even though they're prohibited in the volume name. In order to recognize and strip them out we should use getRealPath instead of getRemotePath that translates slashes into permitted underscores.

It is now allowed to mount gluster volumes with slashes (serv:/volume) and correctly identifying the volume name and its information.

Change-Id: Icfdbc573ec2aec31b78323253f8178425a07302c Signed-off-by: Federico Simoncelli fsimonce@redhat.com Reviewed-on: https://gerrit.ovirt.org/36237 Reviewed-by: Bala.FA barumuga@redhat.com Reviewed-by: Sahina Bose sabose@redhat.com Reviewed-by: Sandro Bonazzola sbonazzo@redhat.com Tested-by: Sandro Bonazzola sbonazzo@redhat.com Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com --- M vdsm/storage/glusterVolume.py 1 file changed, 3 insertions(+), 4 deletions(-)

Approvals: Sandro Bonazzola: Verified; Looks good to me, but someone else must approve Bala.FA: Looks good to me, but someone else must approve Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Sahina Bose: Looks good to me, but someone else must approve