Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-22 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 2:

* #1373930::Update tracker: OK

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-22 Thread frolland 
Freddy Rolland has abandoned this change.

Change subject: storage: add more info to NFS SD getInfo
..


Abandoned

Not the right solution

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

Gerrit-MessageType: abandon
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
___
vdsm-patches mailing list -- vdsm-patches@lists.fedorahosted.org
To unsubscribe send an email to vdsm-patches-le...@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/63636/2/vdsm/storage/nfsSD.py
File vdsm/storage/nfsSD.py:

Line 128: # First call parent getInfo() - it fills in all the common 
details
Line 129: info = fileSD.FileStorageDomain.getInfo(self)
Line 130: # Now add nfsSD specific data
Line 131: try:
Line 132: mntrcd = 
mount.getMountFromTarget(self.mountpoint).getRecord()
> getMountFromTarget iterate over all records, getRecord will iterate again o
If you do any change in mount module, please rebase on 
https://gerrit.ovirt.org/56551
Line 133: info['vfsType'] = mntrcd.fs_vfstype
Line 134: info['mountOptions'] = mntrcd.fs_mntops
Line 135: except mount.MountError:
Line 136: return info


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread nsoffer 
Nir Soffer has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 2: Code-Review-1

(4 comments)

https://gerrit.ovirt.org/#/c/63636/2//COMMIT_MSG
Commit Message:

Line 19: 'wsize=1048576', 'namlen=255', 'soft', 
'nosharecache',
Line 20: 'proto=tcp', 'timeo=600', 'retrans=6', 'sec=sys',
Line 21: 'mountaddr=10.35.0.181', 'mountvers=3',
Line 22: 'mountport=20048', 'mountproto=udp',
Line 23: 'local_lock=none', 'addr=10.35.0.181']
This include options that vdsm (see stoageServer.py) adds and were not sent by 
engine. This may cause trouble if engine use this options to create a new 
storage domain.

Since vdsm adds these options, I think it should also remove them when 
reporting mount options to engine. Otherwise, we will have to keep the list of 
default options in both vdsm and engine to create storage domain from existing 
mount.

I think our goal is to restore a storage domain in engine from the existing 
mount in vdsm.
Line 24: type = NFS
Line 25: class = Data
Line 26: pool = ['0001-0001-0001-0001-0325']
Line 27: name = NFSSD


https://gerrit.ovirt.org/#/c/63636/2/vdsm/storage/nfsSD.py
File vdsm/storage/nfsSD.py:

Line 128: # First call parent getInfo() - it fills in all the common 
details
Line 129: info = fileSD.FileStorageDomain.getInfo(self)
Line 130: # Now add nfsSD specific data
Line 131: try:
Line 132: mntrcd = 
mount.getMountFromTarget(self.mountpoint).getRecord()
getMountFromTarget iterate over all records, getRecord will iterate again over 
all records so this code is not efficient.

Even worse, you may get a Mont object that its getRecord will raise! since 
getRecord require that both fs_spec and fs_file match, but getMountFromTarget 
does not.

We need to improve the mount infrastructure before we can depend on it for a 
new feature.

"mntrcd" is too cryptic, "record" would be better, matching the api.
Line 133: info['vfsType'] = mntrcd.fs_vfstype
Line 134: info['mountOptions'] = mntrcd.fs_mntops
Line 135: except mount.MountError:
Line 136: return info


Line 130: # Now add nfsSD specific data
Line 131: try:
Line 132: mntrcd = 
mount.getMountFromTarget(self.mountpoint).getRecord()
Line 133: info['vfsType'] = mntrcd.fs_vfstype
Line 134: info['mountOptions'] = mntrcd.fs_mntops
These must be documented in the schema.
Line 135: except mount.MountError:
Line 136: return info
Line 137: 
Line 138: return info


Line 132: mntrcd = 
mount.getMountFromTarget(self.mountpoint).getRecord()
Line 133: info['vfsType'] = mntrcd.fs_vfstype
Line 134: info['mountOptions'] = mntrcd.fs_mntops
Line 135: except mount.MountError:
Line 136: return info
Please log the error here instead of returning the info, the info is returned 
anyway below.
Line 137: 
Line 138: return info
Line 139: 
Line 140: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 2:

* #1373930::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1373930::OK, public bug
* Check Product::#1373930::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Liron Aravot 
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread rgolan 
Roy Golan has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 1:

(1 comment)

- can this reveal secure info of some sort?
- Thinking on the client side, the engine should consume that and possibly 
store that as the mount option on that domain and obviously not all the info is 
relevant. Maor, Liron how do you see this?

https://gerrit.ovirt.org/#/c/63636/1/vdsm/storage/nfsSD.py
File vdsm/storage/nfsSD.py:

Line 131: try:
Line 132: mntrcd = 
mount.getMountFromTarget(self.mountpoint).getRecord()
Line 133: info['vfsType'] = mntrcd.fs_vfstype
Line 134: info['mountOptions']  = mntrcd.fs_mntops
Line 135: except mount.MountError:
log the err?
Line 136: return info
Line 137: 
Line 138: return info
Line 139: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Roy Golan 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread automation 
gerrit-hooks has posted comments on this change.

Change subject: storage: add more info to NFS SD getInfo
..


Patch Set 1:

* #1373930::Update tracker: OK
* Check Bug-Url::OK
* Check Public Bug::#1373930::OK, public bug
* Check Product::#1373930::OK, Correct classification oVirt
* Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0)
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 
'ovirt-4.0'])

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org

Change in vdsm[master]: storage: add more info to NFS SD getInfo

2016-09-11 Thread frolland 
Hello Fred Rolland,

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

https://gerrit.ovirt.org/63636

to review the following change.

Change subject: storage: add more info to NFS SD getInfo
..

storage: add more info to NFS SD getInfo

Adding vfs type and mount options to getInfo for NFS storage domains.

vdsClient -s 0 getStorageDomainInfo aca80499-88a3-42e8-ab38-4552a554d715

vfsType = nfs
uuid = aca80499-88a3-42e8-ab38-4552a554d715
version = 3
role = Master
remotePath = 10.35.0.181:/home/storage_domains/sd1
mountOptions = ['rw', 'relatime', 'vers=3', 'rsize=1048576',
'wsize=1048576', 'namlen=255', 'soft', 'nosharecache',
'proto=tcp', 'timeo=600', 'retrans=6', 'sec=sys',
'mountaddr=10.35.0.181', 'mountvers=3',
'mountport=20048', 'mountproto=udp',
'local_lock=none', 'addr=10.35.0.181']
type = NFS
class = Data
pool = ['0001-0001-0001-0001-0325']
name = NFSSD

Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Bug-Url: https://bugzilla.redhat.com/1373930
Signed-off-by: Fred Rolland 
---
M vdsm/storage/nfsSD.py
1 file changed, 16 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/36/63636/1

diff --git a/vdsm/storage/nfsSD.py b/vdsm/storage/nfsSD.py
index 07d0a41..1861319 100644
--- a/vdsm/storage/nfsSD.py
+++ b/vdsm/storage/nfsSD.py
@@ -121,6 +121,22 @@
 except mount.MountError:
 return ""
 
+def getInfo(self):
+"""
+Get storage domain info
+"""
+# First call parent getInfo() - it fills in all the common details
+info = fileSD.FileStorageDomain.getInfo(self)
+# Now add nfsSD specific data
+try:
+mntrcd = mount.getMountFromTarget(self.mountpoint).getRecord()
+info['vfsType'] = mntrcd.fs_vfstype
+info['mountOptions']  = mntrcd.fs_mntops
+except mount.MountError:
+return info
+
+return info
+
 
 def findDomain(sdUUID):
 return NfsStorageDomain(NfsStorageDomain.findDomainPath(sdUUID))


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id54d735a43871f94684e94395b1569c54c03e8ce
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Fred Rolland 
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org