Change in vdsm[master]: storage: add discard support for vm disks

2016-10-26 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: add discard support for vm disks
..


Patch Set 1:

I'm not entirely sure why not just add it by default and be done with it.
Perhaps a VDSM option to turn it off if we find issues with it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I215c12260f819538e40056ec16d0b9378287ccee
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
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: prepare iscsi for IPv6 targets

2016-10-26 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: prepare iscsi for IPv6 targets
..


Patch Set 7:

(1 comment)

https://gerrit.ovirt.org/#/c/65696/7/vdsm/storage/iscsi.py
File vdsm/storage/iscsi.py:

Line 57: 
Line 58: def getTargetString(portal, tpgt=None, iqn=None):
Line 59: hostname = portal.hostname
Line 60: # check for IPv6 address
Line 61: if hostname.count(':') > 2:
What about ::1.2.3.4 - that's an IPv6 address with only 2 ':' chars.
Line 62: hostname = "[%s]" % hostname
Line 63: 
Line 64: portalStr = "%s:%d" % (hostname, portal.port)
Line 65: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e1e1ffe1c5d09b7bc3767d84a99711986eaef97
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Dominik Holler 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
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: iscsiadm parses IPv6 iSCSI addresses

2016-10-26 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: iscsiadm parses IPv6 iSCSI addresses
..


Patch Set 6:

I suggest adding tests to this. For example, from 
https://www.ietf.org/rfc/rfc3721.txt, we can test that all work:
   192.0.2.2
   192.0.2.23:5003
   [FEDC:BA98:7654:3210:FEDC:BA98:7654:3210]
   [1080:0:0:0:8:800:200C:417A]
   [3ffe:2a00:100:7031::1]
   [1080::8:800:200C:417A]
   [1080::8:800:200C:417A]:3260
   [::192.0.2.5]
   mydisks.example.com
   moredisks.example.com:5003

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0e9fa8d8f5ac34753ad6d40625e64dfeee1a3c02
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dominik Holler 
Gerrit-Reviewer: Dominik Holler 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
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]: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.

2016-09-20 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: [QCOW1.1]qemuimg: Add kwargs optional parameter for qcow.
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/64169/4/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 309: 
Line 310: 
Line 311: def _qcow2_compat(**kwargs):
Line 312: sd_version = kwargs.get('version')
Line 313: if sd_version == '4':
>= 4 perhaps?
Line 314: return _QCOW2_1_1
Line 315: value = config.get('irs', 'qcow2_compat')
Line 316: if value not in _QCOW2_COMPAT_SUPPORTED:
Line 317: raise exception.InvalidConfiguration(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8bbf8a60d0af1f99b3fae2c30ac06b36d5986180
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
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 inplace virt-sparsify support

2016-09-19 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 13:

And the initial discussion - 
https://www.redhat.com/archives/libguestfs/2015-November/msg00031.html

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: Yaniv Kaul 
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 inplace virt-sparsify support

2016-09-19 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 13:

I'm quite sure this is not the case. If it is only sparsifying the top layer, 
it is not very efficient - it may actually cause the overlay to become bigger.
See https://bugzilla.redhat.com/show_bug.cgi?id=1277705#c9

This is exactly why the feature was supposed to be implemented only in case you 
don't have a 'tree-like' structure.

I agree it is changing the past. We were hoping for a better future ;-)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: Yaniv Kaul 
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 inplace virt-sparsify support

2016-09-18 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: Add inplace virt-sparsify support
..


Patch Set 13:

1. I was under the impression it can sparsify all the way down to the parent 
(so if there are snaps, it'll sparsify in the correct layer, not just the 
current layer)?
2. I assume it can work on all storages (local, block, NFS, Gluster) - and just 
won't do the 'hole punching' on those not supporting DISCARD silently, which I 
think is OK for us.
3. not sure what the '--tmp prebuilt:file' is for - is it for our qcow2 (vs. 
qcow2v3) ? Can it run on /dev/shm/ (I assume every server will have 
few gigs available for this)?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6ac2bb1fbd2acbe0fc47694d17313c6ccd01a227
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shmuel Leib Melamud 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Maor Lipchuk 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shahar Havivi 
Gerrit-Reviewer: Shmuel Leib Melamud 
Gerrit-Reviewer: Shmuel Melamud 
Gerrit-Reviewer: Yaniv Kaul 
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: fix disconnecting multiple iSCSI sessions

2016-09-14 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: fix disconnecting multiple iSCSI sessions
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/55578/2/vdsm/storage/storageServer.py
File vdsm/storage/storageServer.py:

Line 593: return False
Line 594: 
Line 595: try:
Line 596: mySids = self.getSessionIds()
Line 597: hidSids = other.getSessionIds()
hidSids -> hisSids
Line 598: return mySids == hisSids
Line 599: except OSError:
Line 600: pass
Line 601: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0d576f6fe262d9576fb99a674ba921dc4a141a32
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Gashev 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
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]: rng: allow urandom as virtio rng entropy source

2016-09-08 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: rng: allow urandom as virtio rng entropy source
..


Patch Set 10:

Should we add a spec dependency on a newer libvirt for this to work?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d46f1f3cff63e6443f5154577bd57b5ffc5f4ce
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Polednik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jakub Niedermertl 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Milan Zamazal 
Gerrit-Reviewer: Yaniv Kaul 
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: Remove unneeded multipath call

2016-09-01 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: Remove unneeded multipath call
..


Patch Set 2:

What causes a new device to be added to multipath though?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie99da5ad7ec46f4a69a3d09a811e875d5e3a5b44
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Freddy Rolland 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Fred Rolland 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
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]: Vdsm supports only x86, ppc and ppc64

2016-08-30 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Vdsm supports only x86, ppc and ppc64
..


Patch Set 2:

Do we have any reason to block building (upstream) on ARM and other interesting 
platforms? What is the advantage (or does it fail on them and Koji is upset?)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic6f7a7d9aac6033b60fe592749bab2613b62466a
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Douglas Schilling Landgraf 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Define the StorageDomain.reduce API

2016-08-29 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Define the StorageDomain.reduce API
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/62853/3/lib/api/vdsm-api.yml
File lib/api/vdsm-api.yml:

Line 8216: -   description: A UUID to be used for tracking the job progress
Line 8217: name: jobID
Line 8218: type: *UUID
Line 8219: 
Line 8220: -   description: The UUID of the Storage Domain
Why isn't the SD UUID the first param? (I see in most cases it is, that's all).
Line 8221: name: storagedomainID
Line 8222: type: *UUID
Line 8223: 
Line 8224: -   description: A block device GUID


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5e41b9fa2df4ffef1f3cbb9fbfc57022ffedd9a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
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[ovirt-4.0]: utils: atomic file write

2016-08-23 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: utils: atomic file write
..


Patch Set 3:

(2 comments)

Why not use something like https://github.com/untitaker/python-atomicwrites ?

https://gerrit.ovirt.org/#/c/61950/3/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 949: prefix=os.path.basename(filename) + '.',
Line 950: suffix='.tmp')
Line 951: os.close(fd)
Line 952: try:
Line 953: if os.path.exists(filename):
race between this line and the next line?
Line 954: shutil.copyfile(filename, tmp_filename)
Line 955: with open(tmp_filename, flag) as f:
Line 956: yield f
Line 957: except:


Line 956: yield f
Line 957: except:
Line 958: rmFile(tmp_filename)
Line 959: raise
Line 960: else:
else of what if statement?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icbb5a2d3ac439a334db2c9075376f219c356762c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Petr Horáček 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: virt: enable glusterfs access through libgfapi interface

2016-08-17 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: virt: enable glusterfs access through libgfapi interface
..


Patch Set 8:

(1 comment)

I'm quite sure this patch has to depend on the cluster level being 4.1 or above 
(i.e, RHEL 7.3 and above), no? We don't want older hosts without this mixed 
with newer hosts with this functionality, or is it local to the host? Will live 
migration and live storage migration work between old and new host?

https://gerrit.ovirt.org/#/c/44061/8/vdsm/clientIF.py
File vdsm/clientIF.py:

Line 321: if drive.get('diskType') == DISK_TYPE.NETWORK:
Line 322: volinfo = res['info']
Line 323: volPath = volinfo['path']
Line 324: drive['protocol'] = volinfo['protocol']
Line 325: # currently, single host is provided due to this 
bug:
since this patch is for 4.1, and 4.1 will be on top of 7.3, I think we need to 
remove this limitation.
Line 326: # https://bugzilla.redhat.com/1247521
Line 327: drive['hosts'] = [volinfo['hosts'][0]]
Line 328: else:
Line 329: volPath = res['path']


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I54b81e87b959b0b49c0f06810f88410e7c75de1d
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ala Hino 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Federico Simoncelli 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Prasanna Kumar Kalever 
Gerrit-Reviewer: Sahina Bose 
Gerrit-Reviewer: Yaniv Kaul 
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]: lvm: Separate lv reduce and extend

2016-08-17 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: lvm: Separate lv reduce and extend
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/62369/3/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 1177: _lvminfo._invalidatevgs(vgName)
Line 1178: _lvminfo._invalidatelvs(vgName, lvName)
Line 1179: 
Line 1180: 
Line 1181: def reduceLV(vgName, lvName, size_mb):
Who's using reduce? I've only see it used in the flow of sparsify, 
(specifically shrink) which is a really old code that:
1. Is not really used (I think) - I thought we were going to do it differently.
2. Looks really odd (it extends the volume to its virtual size - really?!
Line 1182: cmd = ("lvreduce",) + LVM_NOBACKUP
Line 1183: cmd += ("--size", "%sm" % (size_mb,), "%s/%s" % (vgName, lvName))
Line 1184: rc, out, err = _lvminfo.cmd(cmd, _lvminfo._getVGDevs((vgName,)))
Line 1185: if rc != 0:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0021d380fb26318ed565b3fae0205404d90bea28
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
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]: net: don't accept nameservers on a non-default network

2016-08-17 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: net: don't accept nameservers on a non-default network
..


Patch Set 10:

Is there a RFE to improve this limitation? I'm quite sure people asked about 
this feature.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib42a5d5a7aeb1169d510a3de8d04645efbab505e
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ondřej Svoboda 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Petr Horáček 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding memTotal metric

2016-08-16 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding memTotal metric
..


Patch Set 4:

(2 comments)

https://gerrit.ovirt.org/#/c/62154/4/lib/vdsm/host/api.py
File lib/vdsm/host/api.py:

Line 93: data[storage_prefix + '.last_check'] = 
dom_info['lastCheck']
Line 94: 
Line 95: data[prefix + '.memory.available'] = hoststats['memAvailable']
Line 96: data[prefix + '.memory.committed'] = hoststats['memCommitted']
Line 97: data[prefix + '.memory.free_mb'] = hoststats['memFree']
what is the _mb suffix here?
Line 98: data[prefix + '.memory.usage_percent'] = hoststats['memUsed']
Line 99: data[prefix + '.memory.total_M'] = hoststats['memTotal']
Line 100: data[prefix + '.memory.anon_huge_pages'] = 
hoststats['anonHugePages']
Line 101: 


https://gerrit.ovirt.org/#/c/62154/4/lib/vdsm/host/stats.py
File lib/vdsm/host/stats.py:

Line 77: stats['cpuSys'] = jiffies / interval / last_sample.ncpus
Line 78: stats['cpuIdle'] = max(0.0,
Line 79:100.0 - stats['cpuUser'] - stats['cpuSys'])
Line 80: stats['memUsed'] = last_sample.memUsed
Line 81: stats['memTotal'] = last_sample.MemTotal / 1024
is this now in bytes? why do we perform the division here, but not in memUsed 
above?
Line 82: stats['anonHugePages'] = last_sample.anonHugePages
Line 83: stats['cpuLoad'] = last_sample.cpuLoad
Line 84: 
Line 85: stats['diskStats'] = last_sample.diskStats


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d3b237aa559f8b0ca835fb55c1205b2766c656a
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding units to memory metrics name according to metrics2.0 ...

2016-08-16 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding units to memory metrics name according to metrics2.0 spec
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/62155/4/lib/vdsm/metrics/__init__.py
File lib/vdsm/metrics/__init__.py:

Line 23: import importlib
Line 24: from ..config import config
Line 25: 
Line 26: # METRIC UNIT SUFFIX REPRESENTATION
Line 27: MB = '_M'
I was not convinced it is _M and not _Mi - please check.
Line 28: 
Line 29: _reporter = None
Line 30: 
Line 31: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1020b89c5ebb2412750352d6c2131097dcbaf407
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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[ovirt-4.0]: Collect ksm values only if exist

2016-08-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Collect ksm values only if exist
..


Patch Set 4:

(1 comment)

https://gerrit.ovirt.org/#/c/62142/4/lib/vdsm/host/api.py
File lib/vdsm/host/api.py:

Line 113: 
Line 114: # only available when mom is configured to run
Line 115: if 'ksmPages' in hoststats:
Line 116: report[prefix + '.cpu.ksm_pages'] = hoststats['ksmPages']
Line 117: report[prefix + '.cpu.ksm_cpu_precent'] = 
hoststats['ksmCpu']
typo: precent -> percent
Line 118: 
Line 119: if hoststats['haStats']['configured']:
Line 120: report[prefix + '.ha_score'] = hoststats['haScore']
Line 121: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5d1aa7830bff26c688a7c0322152e7c96ab4ecfa
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-4.0
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Irit Goihman 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding memTotal metric

2016-08-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding memTotal metric
..


Patch Set 3:

What's the value of adding this metric?
Don't we expect it to be collected by a general host agent that'll send this 
metric? (same as CPU load and others, btw).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3d3b237aa559f8b0ca835fb55c1205b2766c656a
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding units to cpu metrics name according to metrics2.0 spec

2016-08-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding units to cpu metrics name according to metrics2.0 spec
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/62156/3/lib/vdsm/host/api.py
File lib/vdsm/host/api.py:

Line 104: 
Line 105: data[prefix + '.vms.active'] = hoststats['vmActive']
Line 106: data[prefix + '.vms.total'] = hoststats['vmCount']
Line 107: 
Line 108: data[prefix + '.cpu.load_5M'] = hoststats['cpuLoad']
> every 5 minutes
CPU load is actually how many processes are waiting for CPU in a given time (in 
this case, averaged over the last 5 minutes). It's an absolute number (if you 
are talking about the same value as shown in top and 'uptime' commands.
Line 109: data[prefix + '.cpu.user_jiff'] = hoststats['cpuUser']
Line 110: data[prefix + '.cpu.sys_jiff'] = hoststats['cpuSys']
Line 111: data[prefix + '.cpu.idle_jiff'] = hoststats['cpuIdle']
Line 112: data[prefix + '.cpu.sys_vdsmd_jiff'] = 
hoststats['cpuSysVdsmd']


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I366280bf1c3783fdbbdd7459e49ae7fcf5e8b598
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding units to cpu metrics name according to metrics2.0 spec

2016-08-14 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding units to cpu metrics name according to metrics2.0 spec
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/62156/3/lib/vdsm/host/api.py
File lib/vdsm/host/api.py:

Line 113: data[prefix + '.cpu.user_vdsmd_jiff'] = 
hoststats['cpuUserVdsmd']
Line 114: 
Line 115: if 'ksmPages' in hoststats:
Line 116: data[prefix + '.cpu.ksm_pages'] = hoststats['ksmPages']
Line 117: data[prefix + '.cpu.ksm_cpu_precent'] = 
hoststats['ksmCpu']
typo: precent -> percent
Line 118: 
Line 119: if hoststats['haStats']['configured']:
Line 120: data[prefix + '.ha_score'] = hoststats['haScore']
Line 121: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I366280bf1c3783fdbbdd7459e49ae7fcf5e8b598
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: Adding units to memory metrics name according to metrics2.0 ...

2016-08-14 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding units to memory metrics name according to metrics2.0 spec
..


Patch Set 3:

_m is mili, where _M is mega, no?
Moreover, _M is 10^6 - while Mi is mebi, 1024^2. What are we reporting?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I1020b89c5ebb2412750352d6c2131097dcbaf407
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Shirly Radco 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
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]: multipath: Do not fail I/O after short outage

2016-07-25 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: multipath: Do not fail I/O after short outage
..


Patch Set 1:

In the worst case scenario, the timeout really depends on the path selection 
algorithm (a combination of path_grouping_policy, path_selector, failback, 
prio, rr_weight and some other witchcraft no one really knows). So assuming it 
has (for example) an active-passive set of paths, it'll retry each path 4 more 
times for 5 seconds, one by one in the worst case scenario - so the delay might 
be worse. To make things even worse, there's also the SCSI layer timeout 
(/sys/block//device/timeout). See 
https://www.flagword.net/2014/06/default-linux-io-multipathd-configuration-and-oracle-rac-caveat/
 for some explanation.

I think the bottom line is (and that is a rule of a thumb) that if the timeout 
overall will be larger than 30 seconds, we may face application timeout - 
therefore I'm in favor of 'no_path_retry fail' - hoping it'll move quickly to 
the next path, hoping not all path failed. If they did, we have the SCSI layer 
queuing to overcome small outages (I hope) and if not - so be it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6496dbdaafca6b110c952fcc5d51bf9ac04d49b4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Kaul 
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 getDeviceList discard related fields

2016-07-14 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: add getDeviceList discard related fields
..


Patch Set 1: -Code-Review

Ok, makes sense.

We may want to pull in optimal_io_size as well, btw, for other purposes.

When do we perform this check, btw? What happens when the storage (that did not 
support this) now is upgraded (non disruptively)to support discard?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ff477dab23a195bd9ce10c36defa35e8a0749d8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
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 getDeviceList discard related fields

2016-07-13 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: add getDeviceList discard related fields
..


Patch Set 1: Code-Review-1

(1 comment)

I'm quite positive the key queried should be discard_max_hw_bytes and not 
discard_max_bytes.

https://gerrit.ovirt.org/#/c/60628/1/vdsm/storage/multipath.py
File vdsm/storage/multipath.py:

Line 165: 
Line 166: 
Line 167: def getDeviceDiscardMaxBytes(physDev):
Line 168: return int(file(os.path.join("/sys", "block/", physDev,
Line 169:  "queue", 
"discard_max_bytes")).read())
should be discard_max_hw_bytes !
Line 170: 
Line 171: 
Line 172: def getDeviceDiscardZeroesData(physDev):
Line 173: return int(file(os.path.join("/sys", "block/", physDev,


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7ff477dab23a195bd9ce10c36defa35e8a0749d8
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
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]: Modify reports to metrics

2016-07-11 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Modify reports to metrics
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/60450/1/lib/vdsm/config.py.in
File lib/vdsm/config.py.in:

Line 443: ('enabled', 'false',
Line 444: 'Enable metric reporting (default false)'),
Line 445: 
Line 446: ('collector_address', 'localhost',
Line 447: 'Reports collector address (default localhost)'),
Remove 'Reports' here?
Line 448: 
Line 449: ('collector_type', 'statsd',
Line 450: 'Reports collector type (supporting statsd or hawkular)'),
Line 451: 


Line 446: ('collector_address', 'localhost',
Line 447: 'Reports collector address (default localhost)'),
Line 448: 
Line 449: ('collector_type', 'statsd',
Line 450: 'Reports collector type (supporting statsd or hawkular)'),
Remove 'Reports' here?
Line 451: 
Line 452: ('queue_size', '100',
Line 453: 'Number of metrics to queue if collector is not 
responsive.'
Line 454: ' When the queue is full, oldest metrics are dropped. 
Used only'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib0aa10042b1c50fbdd94ae69053c74b57c3e6f40
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
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]: [wip] Adding report_stats to host.api

2016-05-09 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: [wip] Adding report_stats to host.api
..


Patch Set 1:

(2 comments)

https://gerrit.ovirt.org/#/c/56880/1/lib/vdsm/host.py
File lib/vdsm/host.py:

Line 456: Send gauge values to statsd
Line 457: The format
Line 458: """
Line 459: report = statsd.Gauge(".host." + uuid())
Line 460: report.send('memAvailable', _current_stats['memAvailable'])
> yes, that is the plan
I think we need to look at how the metrics storage expects to see them. There 
is a convention of ... For example: 
host.memory.free.megabytes
Line 461: report.send('memCommitted', _current_stats['memCommitted'])
Line 462: report.send('memFree', _current_stats['memFree'])
Line 463: report.send('swapTotal', _current_stats['swapTotal'])
Line 464: report.send('swapFree', _current_stats['swapFree'])


https://gerrit.ovirt.org/#/c/56880/1/lib/vdsm/virt/sampling.py
File lib/vdsm/virt/sampling.py:

PS1, Line 551: reportStats
> Do we want to send a delta with the changes or complete set of metrics each
We need to send raw, complete data.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-05 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/35629/6/lib/vdsm/storage/blkdiscard.py
File lib/vdsm/storage/blkdiscard.py:

Line 43: def run_blkdiscard(device):
Line 44: cmd = [_blkdiscard.cmd]
Line 45: cmd.append(device)
Line 46: 
Line 47: rc, out, err = commands.execCmd(cmd, deathSignal=signal.SIGKILL)
> This module seems like a perfect candidate for StorageJob integration.  Fro
Since I'm hoping to see this in 4.0.x, not sure we can do that right now. I 
also hope that there won't be a need for progress - I expect blkdiscard to run 
fast (Gigs/sec - faster than line speed) in most storages.
Line 48: 
Line 49: if rc != 0:


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: storage: add support for blkdiscard command

2016-05-04 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: storage: add support for blkdiscard command
..


Patch Set 6:

How do you pass parameters (such as --zero) to blkdiscard?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ea7dd19fadc600b8fe78fb436ae430d35f52165
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Idan Shaby 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Adding reportStats

2016-05-04 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding reportStats
..


Patch Set 1:

Well, that's strange.
If those stats are irrelevant, why do we bother collect them at all?
Shirly has done great work on mapping them, required, not required, etc. - the 
bottom line was that many are going to be needed, though possibly not by the 
engine. I'd parse (of course, not for this POC).

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Adding reportStats

2016-05-01 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Adding reportStats
..


Patch Set 1:

How do we get all stats? Somehow in an automated manner, so when we add more 
stats we don't need to manually add more code?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I23c1141f097f740441d085f99e0bf76eb7f718c9
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[ovirt-3.6]: configurator: libvirt: do not jump on virtlogd

2016-04-12 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: configurator: libvirt: do not jump on virtlogd
..


Patch Set 4:

This sounds like a bad idea.
If the feature is not stable, it should either be tech-preview or disabled by 
default, and certainly not in RHEL 7.3.
Please discuss this patch on virt-devel.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I548560af8e78a69210917c36d229206663beaaf7
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: kvmstream: tool for streaming images from libvirt

2016-04-07 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: kvmstream: tool for streaming images from libvirt
..


Patch Set 1:

(3 comments)

Few notes in the code, but generally I'd give -1 since I think it's critical to 
use the imageio stuff here.

https://gerrit.ovirt.org/#/c/55797/1/kvmstream.py
File kvmstream.py:

Line 69: vol.download(stream, 0, 0, 0)
Line 70: write_output('(0/100%%)\r')
Line 71: counter = 0
Line 72: while pos < size:
Line 73: buf = s.recv(1024)
The 1024 is probably a low value - I'd increase it (but in any case, I think 
Nir is completely right in his above comment - you must use the imageio stuff.
Line 74: f.write(buf)
Line 75: f.flush()
Line 76: if not pos % jump:
Line 77: counter = counter + 1


Line 82: write_output('(100/100%%)\r')
Line 83: 
Line 84: 
Line 85: options = parse_arguments()
Line 86: con = libvirt.open(options.uri)
Some error handling?
if con == None: ...
Line 87: 
Line 88: diskno = 1
Line 89: disksitems = len(options.source)
Line 90: init_progres()


Line 87: 
Line 88: diskno = 1
Line 89: disksitems = len(options.source)
Line 90: init_progres()
Line 91: for item in izip(options.source, options.dest):
I wonder if we should test downloading multiple disks in parallel. Not sure we 
need to do it one by one, though it adds complexity.
Line 92: vol = con.storageVolLookupByPath(item[0])
Line 93: s = con.newStream()
Line 94: download_volume(s, vol, item[1], diskno, disksitems)
Line 95: diskno = diskno + 1


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9d95c3bf4b2605e71f899171259d4721204eb8e2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks:checkips: add checkips hook

2016-04-03 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: hooks:checkips: add checkips hook
..


Patch Set 11:

(1 comment)

https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/checkipsd
File vdsm_hooks/checkips/checkipsd:

Line 58: def _ping_address(address, address_type, net):
Line 59: ping_cmd = 'ping' if address_type == CHECKIPV4 else 'ping6'
Line 60: command = [
Line 61: ping_cmd,
Line 62: '-c', '1',
> I checked a little behaviour of ping command:
This is actually interesting. Suppose we have '-c 3' - and only two pings 
returned. What do we say about the connectivity? (I don't have a very good 
answer for 3. For 10 pings, I expect 9/10 - but I wonder in what error code 
ping would come out with!).
Line 63: '-w', str(PING_TIMEOUT),
Line 64: '-I', net,
Line 65: address
Line 66: ]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I53cec37310f0f1844d6fe244419fd8c10e9b7ebb
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Artyom Lukianov 
Gerrit-Reviewer: Artyom Lukianov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: hooks:checkips: add checkips hook

2016-04-03 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: hooks:checkips: add checkips hook
..


Patch Set 11:

(9 comments)

Some minor comments added.

https://gerrit.ovirt.org/#/c/54102/11/configure.ac
File configure.ac:

Line 400:   vdsm/virt/Makefile
Line 401:   vdsm/virt/vmdevices/Makefile
Line 402:   vdsm_hooks/Makefile
Line 403:   vdsm_hooks/allocate_net/Makefile
Line 404:   vdsm_hooks/checkips/Makefile
checkips should come after checkimages. I believe we sort alphabetically?
Line 405:   vdsm_hooks/checkimages/Makefile
Line 406:   vdsm_hooks/diskunmap/Makefile
Line 407:   vdsm_hooks/ethtool_options/Makefile
Line 408:   vdsm_hooks/extnet/Makefile


https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/Makefile.am
File vdsm_hooks/Makefile.am:

Line 34: # Additional hooks
Line 35: if HOOKS
Line 36: SUBDIRS += \
Line 37:allocate_net \
Line 38:checkips \
same as previously - should come after checkimages.
Line 39:checkimages \
Line 40:diskunmap \
Line 41:extnet \
Line 42:extra_ipv4_addrs \


https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/README
File vdsm_hooks/checkips/README:

Line 34: In the host setup network configuration window, choose edit 
assigned
Line 35: logical network, select custom property checkipv4 or checkipv6 and
Line 36: enter VLAN IP or FQDN that you want to check for
Line 37: connectivity(examples 8.8.8.8,2001:4860:4860::). So if you 
defined
Line 38: checipv4 and checipv6 custom properties and if the host fails to 
ping
typo: checipv4 -> checkipv4
Line 39: both IP's, it will drop the network's state to down.
Line 40: If the network is defined as "required",


https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/after_get_stats.py
File vdsm_hooks/checkips/after_get_stats.py:

Line 47: 
Line 48: CONNECTIVITY_TIMEOUT = 60
Line 49: 
Line 50: 
Line 51: def _is_network_accessible(net, stats_dir):
the name of the function is not related to what it does?
Line 52: file_path = os.path.join(stats_dir, net)
Line 53: if os.path.exists(file_path):
Line 54: return (
Line 55: time.time() - os.stat(file_path).st_mtime <=


Line 124: print(
Line 125: 'test %s: interface %s has state %s' %
Line 126: (test_msg, interface, state)
Line 127: )
Line 128: os.unlink(os.path.join(temp_dir, 'check_ipv4'))
Perhaps worth putting all this in a finally (and the above in a try, so any 
exception would still remove the files and the directory?
Line 129: os.unlink(os.path.join(temp_dir, 'check_ipv6'))
Line 130: os.rmdir(temp_dir)
Line 131: 
Line 132: 


https://gerrit.ovirt.org/#/c/54102/11/vdsm_hooks/checkips/checkipsd
File vdsm_hooks/checkips/checkipsd:

Line 1: #!/usr/bin/python
Shouldn't this file have a .py extension?
Line 2: #
Line 3: # Copyright 2008-2016 Red Hat, Inc.
Line 4: #
Line 5: # This program is free software; you can redistribute it and/or modify


Line 35: CHECKIPV6 = 'checkipv6'
Line 36: VDSM_CHECKIPS = 'vdsm-checkips'
Line 37: 
Line 38: 
Line 39: def get_ping_addresses(net_attrs):
Isn't this function available in the utils?
Line 40: ping_addresses = []
Line 41: if 'custom' in net_attrs:
Line 42: for address_type in (CHECKIPV4, CHECKIPV6):
Line 43: if address_type in net_attrs['custom']:


Line 54: with open(file_path, 'a'):
Line 55: os.utime(file_path, None)
Line 56: 
Line 57: 
Line 58: def _ping_address(address, address_type, net):
rename 'net' to 'interface' ?
Line 59: ping_cmd = 'ping' if address_type == CHECKIPV4 else 'ping6'
Line 60: command = [
Line 61: ping_cmd,
Line 62: '-c', '1',


Line 58: def _ping_address(address, address_type, net):
Line 59: ping_cmd = 'ping' if address_type == CHECKIPV4 else 'ping6'
Line 60: command = [
Line 61: ping_cmd,
Line 62: '-c', '1',
There were several comments that a single ping is not enough. For example, in 
hosted-engine setup - there was a request to try with more pings. I'd either 
have it as a variable, or by default use more than a single ping.
Line 63: '-w', str(PING_TIMEOUT),
Line 64: '-I', net,
Line 65: address
Line 66: ]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I53cec37310f0f1844d6fe244419fd8c10e9b7ebb
Gerrit-PatchSet: 11
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Artyom Lukianov 
Gerrit-Reviewer: Artyom Lukianov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes

Change in vdsm[master]: hooks: Add fcoe hook

2016-03-22 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: hooks: Add fcoe hook
..


Patch Set 10:

(4 comments)

https://gerrit.ovirt.org/#/c/55029/10/vdsm.spec.in
File vdsm.spec.in:

Line 622: VDSM hook used for applying IPv6 configuration through custom network
Line 623: properties
Line 624: 
Line 625: %package hook-fcoe
Line 626: Summary:Hook for enable fcoe support
for -> to
fcoe -> FCoE
Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
Line 630: 


Line 625: %package hook-fcoe
Line 626: Summary:Hook for enable fcoe support
Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
What about lldpad?
Line 630: 
Line 631: %description hook-fcoe
Line 632: %{summary}
Line 633: 


Line 627: BuildArch:  noarch
Line 628: Requires:   %{name} = %{version}-%{release}
Line 629: Requires:   fcoe-utils
Line 630: 
Line 631: %description hook-fcoe
thin description...
Line 632: %{summary}
Line 633: 
Line 634: %if 0%{?with_gluster_mgmt}
Line 635: %package gluster


https://gerrit.ovirt.org/#/c/55029/10/vdsm_hooks/fcoe/README
File vdsm_hooks/fcoe/README:

Line 1: fcoe vdsm hook
Line 2: =
Line 3: This hook allow to configure one or more NIC as FCoE interface(s)
NIC -> NICs
Line 4: 
Line 5: Intallation:
Line 6: * Use engine-config to append the appropriate custom property:
Line 7:   sudo engine-config -s 
UserDefinedNetworkCustomProperties='fcoe=^(true|false)$' --cver=3.6


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: migration: Add retry on full capacity

2016-03-19 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: migration: Add retry on full capacity
..


Patch Set 19:

Again, unsure why VDSM would do that and not engine. Only in the remote case 
where engine disconnected and VDSM has to do the retry, it'll make sense.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I988fa2e501eb77d121668b22cc533b744a3dc755
Gerrit-PatchSet: 19
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Tomas Jelinek 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: qemuimg: Make QCOW2_COMPAT configurable

2016-03-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: qemuimg: Make QCOW2_COMPAT configurable
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/54759/1/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 39: 
Line 40: 
Line 41: class FORMAT:
Line 42: QCOW2 = "qcow2"
Line 43: QCOW = "qcow"
> Sure we need to clean this sometimes.
1. We need to support exporting to 0.1 - so the disk can be imported to an 
older oVirt environment.
2. We need to support conversion when importing.
Line 44: QED = "qed"
Line 45: RAW = "raw"
Line 46: VMDK = "vmdk"
Line 47: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5272171a4b4931698911ab251e3a34c286908be4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: qemuimg: Make QCOW2_COMPAT configurable

2016-03-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: qemuimg: Make QCOW2_COMPAT configurable
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/54759/1/lib/vdsm/qemuimg.py
File lib/vdsm/qemuimg.py:

Line 39: 
Line 40: 
Line 41: class FORMAT:
Line 42: QCOW2 = "qcow2"
Line 43: QCOW = "qcow"
Can clean some of those, while at it. We are not going to support QED, donno 
even what VMDK is doing here.
Line 44: QED = "qed"
Line 45: RAW = "raw"
Line 46: VMDK = "vmdk"
Line 47: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5272171a4b4931698911ab251e3a34c286908be4
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Ala Hino 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Freddy Rolland 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: migration: Add retry on full capacity

2016-02-25 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: migration: Add retry on full capacity
..


Patch Set 18:

Why is this a VDSM decision and not engine? What if by now, there's a better 
chance to migrate to a different host? Looks like a scheduling task to me, not 
a VDSM local task to do the retry.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I988fa2e501eb77d121668b22cc533b744a3dc755
Gerrit-PatchSet: 18
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: repoplot: Optimize log parsing

2016-02-24 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: repoplot: Optimize log parsing
..


Patch Set 2:

(1 comment)

https://gerrit.ovirt.org/#/c/53949/2/contrib/repoplot
File contrib/repoplot:

Line 82: Parse repoStat from vdsm log. Return dict of DomainStats objects.
Line 83: """
Line 84: stats = defaultdict(DomainStats)
Line 85: 
Line 86: for line in fileinput.input(files):
Perhaps set a buffer? See 
https://hg.python.org/cpython/file/2.7/Lib/fileinput.py#l67
Line 87: pos = line.find(REPOSTAT)
Line 88: if pos == -1:
Line 89: continue
Line 90: timestamp = line.split("::", 3)[2]


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iac2b2e675d1c4397a5659e89a34689ce7e4004a6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: repoplot: Visualize storage domain monitoring

2016-02-22 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: repoplot: Visualize storage domain monitoring
..


Patch Set 2:

- Isn't something like Splunk or even ELK better suited for such task 
(especially when we'll want to extend this to more than storage - for example, 
tasks) ? 
- Is readDelay of any value? (I remember we discussed in the past it should be 
removed, as it is probably a cached read anyway)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic0366d45b1fb59aab75843bff4224c8b572dd265
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Mordechai Lehrer 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Skipping networkTests if alien bonds presents in the system

2016-02-17 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Skipping networkTests if alien bonds presents in the system
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/53498/6//COMMIT_MSG
Commit Message:

Line 7: Skipping networkTests if alien bonds presents in the system
Line 8: 
Line 9: Alien bonds causes to tests to report incorrect results
Line 10: testing with alien bonds doesnt test vdsm networking
Line 11: It make sence since ovirt host shouldn't be used for other
sense
Line 12: purposes
Line 13: 
Line 14: Change-Id: I5326305fac74a43ca7cab259133e7a8861fa9261


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5326305fac74a43ca7cab259133e7a8861fa9261
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ilia Meerovich 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Ilia Meerovich 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Skipping networkTests if alien bonds presents in the system

2016-02-17 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Skipping networkTests if alien bonds presents in the system
..


Patch Set 4:

Why won't we test with bonds?
(at least the commit message should explain it)

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5326305fac74a43ca7cab259133e7a8861fa9261
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ilia Meerovich 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: ipv6: ifcfg: explicitly disable dhcpv6

2016-01-26 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: ipv6: ifcfg: explicitly disable dhcpv6
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/52759/1/lib/vdsm/network/configurators/ifcfg.py
File lib/vdsm/network/configurators/ifcfg.py:

Line 539: ipv6autoconf
> Should be dhcpv6
Are you sure? Surprisingly, it is DHCPV6C for some odd reason.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Icfca4008f19e1f369b67da6ec958576278d4fd50
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg 
Gerrit-Reviewer: Edward Haas 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Ondřej Svoboda 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: Yevgeny Zaspitsky 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: guestagent: Simplify and modernize object filtering

2016-01-06 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: guestagent: Simplify and modernize object filtering
..


Patch Set 5:

Do we need to filter for non-compliant XML content, when we moved from XML-RPC 
to JSON-RPC?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I753d97ffe992400728aebed0352661306a1619c7
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: guestagent: Speed up xml character filtering

2016-01-05 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: guestagent: Speed up xml character filtering
..


Patch Set 3:

(1 comment)

What about the note @ http://www.w3.org/TR/xml11/#charsets about additional 
ones to be avoided?

https://gerrit.ovirt.org/#/c/50945/3/vdsm/virt/guestagent.py
File vdsm/virt/guestagent.py:

Line 53: the
'the the' -> 'the'


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic7990dbe9787089b15f63e89de284c8695472b66
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: block: add blkdiscard on zero image

2015-11-16 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: block: add blkdiscard on zero image
..


Patch Set 1:

(1 comment)

UNMAP is not a secure method for erasure, based on the below.
Therefore, we should use WRITE_SAME, not UNMAP.

https://gerrit.ovirt.org/#/c/35631/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 219: size = multipath.getDeviceSize(lvm.lvDmDev(sdUUID, volUUID))
Line 220: 
Line 221: try:
Line 222: misc.ddWatchCopy("/dev/zero", path, aborting, size)
Line 223: utillinux.blkdiscard(path)
> It certainly does.
Correction: according to SBC-4:
4.7.3.4.2 Processing unmap requests

Application clients should not rely on an UNMAP command (see 5.30) to cause 
specific data (e.g., zeros) to
be returned by subsequent read operations on the specified LBAs. To produce 
consistent results for
subsequent read operations, a write command (e.g., the WRITE SAME command) 
should be used to write
user data.
EXAMPLE - To ensure that subsequent read operations return all zeros in a 
logical block, use the WRITE SAME (16)
command with the NDOB bit set to one. If the UNMAP bit is set to one, then the 
device server may unmap the logical blocks
specified by the WRITE SAME (16) command as described in 4.7.3.4.4.
Line 224: except Exception as e:
Line 225: log.exception('zeroing operation failed')
Line 226: raise se.VolumesZeroingError(path)
Line 227: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e059c556c550440727b36b8af8e5dfc29ce2ccb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: block: add blkdiscard on zero image

2015-11-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: block: add blkdiscard on zero image
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/35631/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 219: size = multipath.getDeviceSize(lvm.lvDmDev(sdUUID, volUUID))
Line 220: 
Line 221: try:
Line 222: misc.ddWatchCopy("/dev/zero", path, aborting, size)
Line 223: utillinux.blkdiscard(path)
Are we discarding AFTER we perform the 'dd'? What's the point?
Line 224: except Exception as e:
Line 225: log.exception('zeroing operation failed')
Line 226: raise se.VolumesZeroingError(path)
Line 227: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e059c556c550440727b36b8af8e5dfc29ce2ccb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: block: add blkdiscard on zero image

2015-11-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: block: add blkdiscard on zero image
..


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/35631/3/vdsm/storage/blockVolume.py
File vdsm/storage/blockVolume.py:

Line 228: 
Looking at blkdiscard's man:
-p, --step length
  The number of bytes to discard within one iteration. The default
  is to discard all by one ioctl call.


So for example, let's assume we have 1TB LV to discard - it cannot do it on one 
IOCTL. What happens? Different storages have:
1. Different number of ranges they can unmap (most support 1 only, this is also 
what VMware does and I believe Linux kernel's UNMAP. sg_unmap can do multiple 
ranges)
2. Maximum length of UNMAP (can be found via some VPD page).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e059c556c550440727b36b8af8e5dfc29ce2ccb
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: block: add blkdiscard on zero image

2015-11-15 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: block: add blkdiscard on zero image
..


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/35631/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 219: size = multipath.getDeviceSize(lvm.lvDmDev(sdUUID, volUUID))
Line 220: 
Line 221: try:
Line 222: misc.ddWatchCopy("/dev/zero", path, aborting, size)
Line 223: utillinux.blkdiscard(path)
> We want to remove the data before we discard. Does discard ensure that the 
It certainly does.
It's not secure, in the sense that if there's a bug in the storage the date may 
re-appear, but other than that - absolutely. For the bug in the storage we may 
wish to introduce secure deletion, but this is a different item.
If we do unmap, we can and should rely on the data being deleted.

Also see https://rwmj.wordpress.com/tag/unmap/
Line 224: except Exception as e:
Line 225: log.exception('zeroing operation failed')
Line 226: raise se.VolumesZeroingError(path)
Line 227: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4e059c556c550440727b36b8af8e5dfc29ce2ccb
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli 
Gerrit-Reviewer: Allon Mureinik 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Tal Nisan 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: gerrit-hooks 
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: multipath: Write multipath.conf atomically

2015-09-20 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: multipath: Write multipath.conf atomically
..


Patch Set 6:

(1 comment)

https://gerrit.ovirt.org/#/c/44869/6/lib/vdsm/tool/configurators/multipath.py
File lib/vdsm/tool/configurators/multipath.py:

Line 79: # "defaults" section), or to devices defined in the "devices" 
section.
Line 80: # Note: This is not available yet on Fedora 21. For more info 
see
Line 81: # https://bugzilla.redhat.com/1253799
Line 82: all_devsyes
Line 83: no_path_retry   fail
I thought this parameter should be getting numeric values. See 
https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html-single/DM_Multipath/#config_file_defaults
Line 84: }
Line 85: }
Line 86: 
Line 87: # Enable when this section is available on all supported platforms.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I850d621b7cb09f2732b8b3eb2cb2897e87547ddb
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Fabian Deutsch 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: Yeela Kaplan 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: scale: limit cpu usage using cpu-affinity

2015-09-13 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: scale: limit cpu usage using cpu-affinity
..


Patch Set 13:

(1 comment)

https://gerrit.ovirt.org/#/c/45738/13/lib/vdsm/utils.py
File lib/vdsm/utils.py:

Line 629: SC_NPROCESSORS_CONF
_SC_NPROCESSORS_ONLN perhaps?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I3f7f68d65eddb5a21afbc3809ea79cd1dee67984
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani 
Gerrit-Reviewer: Adam Litke 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Francesco Romani 
Gerrit-Reviewer: Ido Barkan 
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik 
Gerrit-Reviewer: Martin Sivák 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Michal Skrivanek 
Gerrit-Reviewer: Nir Soffer 
Gerrit-Reviewer: Piotr Kliczewski 
Gerrit-Reviewer: Vinzenz Feenstra 
Gerrit-Reviewer: Yaniv Bronhaim 
Gerrit-Reviewer: Yaniv Kaul 
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Add shutdown based on qemu-ga(qemu guest agent) in vdsm

2012-12-04 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Add shutdown based on qemu-ga(qemu guest agent) in vdsm
..


Patch Set 2: (1 inline comment)


File vdsm/vm.py
Line 998: return status
Line 999: if not status:
Line 1000: return {'status':
Line 1001: {'code': errCode['exist']['status']['code'],
Line 1002:  'message': 'VM without ACPI or active SolidICE 
tools. '
SolidICE - oVirt
Line 1003: 'Try Forced Shutdown.'}}
Line 1004: 
Line 1005: return status
Line 1006: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I86977c1b717d63de21ba4818c6b66e43976d65de
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: ShaoHe Feng shao...@linux.vnet.ibm.com
Gerrit-Reviewer: Barak Azulay bazu...@redhat.com
Gerrit-Reviewer: Bing Bu Cao m...@linux.vnet.ibm.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Michal Skrivanek michal.skriva...@redhat.com
Gerrit-Reviewer: ShaoHe Feng shao...@linux.vnet.ibm.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
Gerrit-Reviewer: Zhou Zheng Sheng zhshz...@linux.vnet.ibm.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Implement SSL session cache

2012-10-02 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Implement SSL session cache
..


Patch Set 5:

Was tested against a 3.0 RHEVM as well, make sure it inter-operates with it?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic75adee4070b415b8855af1f2ea289825496fbc1
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Implement SSL session cache

2012-09-23 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Implement SSL session cache
..


Patch Set 2: (1 inline comment)


File vdsm/SecureXMLRPCServer.py
Line 70: SSL.VERIFY_CLIENT_ONCE,
Line 71: SSLServerSocket.verify)
Line 72: 
Line 73: @staticmethod
Line 74: def verify(connection, certificate, x, y, z):
I thought 'z' is actually return code and should be checked.
See http://twistedmatrix.com/documents/current/core/howto/ssl.html#auto8 or 
http://stackoverflow.com/questions/9089957/validating-client-certificates-in-pyopenssl
Line 75: # No need for additional verifications:
Line 76: return True
Line 77: 
Line 78: def __getattr__(self, name):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic75adee4070b415b8855af1f2ea289825496fbc1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: Implement SSL session cache

2012-09-23 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: Implement SSL session cache
..


Patch Set 2:

I forgot to ask, does that expose us in any way to the infamous TLS 
renegotiation vulnerability? I remember the workaround in ALL cases was to 
disable renegotiation.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic75adee4070b415b8855af1f2ea289825496fbc1
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Juan Hernandez juan.hernan...@redhat.com
Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: BZ#846609: bootstrap: remove dh key generation

2012-08-09 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: BZ#846609: bootstrap: remove dh key generation
..


Patch Set 1:

I'd still just change:
deployUtil.py:_logExec([EX_OPENSSL, dhparam, -out, dhKey, 
str(DEF_KEY_LEN)])

to:
deployUtil.py:_logExec([EX_OPENSSL, dhparam, -out, dhKey, str(1024)])

and be done with it.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I820c8599516cd4c4e3bf7ad73ee2c1c6dee22c47
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev alo...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
Gerrit-Reviewer: oVirt Jenkins CI Server
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: bootstrap: m2crypto is no longer used

2012-07-11 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: bootstrap: m2crypto is no longer used
..


Patch Set 2:

From the code it looked like the bootstrap is used to install any VDSM version, 
new or old. So even if the mgmt is new, the host may be old ('RHEV 3.0').

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I161fad88b021575056b03fbbe11474dd18078b02
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Igor Lvovsky ilvov...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: bootstrap: m2crypto is no longer used

2012-07-02 Thread ykaul
Yaniv Kaul has posted comments on this change.

Change subject: bootstrap: m2crypto is no longer used
..


Patch Set 1:

It's not directly related to this patch, but we should not require pixman and 
mesa - let Spice handle its dependencies.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I161fad88b021575056b03fbbe11474dd18078b02
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Douglas Schilling Landgraf dougsl...@redhat.com
Gerrit-Reviewer: Yaniv Kaul yk...@redhat.com
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/vdsm-patches