Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-10 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 9:

updated commit message with explanation about the involved libvirt calls.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-10 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 9: Verified+1

verified using vdsClient, inspecting output of getAllVmStats.

- booted a VM on a patched VDSM and let it run
- in a shell, did watch vdsClient -s 0 getAllVmStats
- verified that the *rate and *latency values are still present.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-10 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 9:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12873/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1764/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12715/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11924/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-10 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 9: Code-Review+2

(1 comment)

http://gerrit.ovirt.org/#/c/29953/9/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 559
Line 560
Line 561
Line 562
Line 563
oh, you are dropping the code that offended me. cool.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-10 Thread danken
Dan Kenigsberg has submitted this change and it was merged.

Change subject: virt: sampling: consolidate disk statistics
..


virt: sampling: consolidate disk statistics

We used to have two different sampling, and thus two
libvirt calls per sampling, per disk.
One sampling gathers basic disk data and R/W rate;
the other one gathers the disk latency.

Most often, as the default values are unchanged, the
configuration for these sampling is identical, so it is safe
and cheaper to read all the needed information in one go,
saving one libvirt call without losing any information.

These sampling uses two different libvirt calls.
sampleDisk, which gathers the disk I/O rate, uses the older
virDomainGetBlockStats call;
sampleDiskLatency, which gathers disk latency, uses the newer
and more flexible virDomainGetBlockStatsFlags;

Inside libvirt, these two calls are implemented on top
of the same QEMU monitor function (qemuMonitorGetBlockStatsInfo).
The newer call returns all the data gathered by the QEMU monitor,
while the older call returns just a subset of it.

Other differences in the API exists, like the return type
and the presence of the flags, but those can safely be considered
of no interest for VDSM.

Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Signed-off-by: Francesco Romani from...@redhat.com
Reviewed-on: http://gerrit.ovirt.org/29953
Reviewed-by: Nir Soffer nsof...@redhat.com
Reviewed-by: Dan Kenigsberg dan...@redhat.com
---
M vdsm/virt/vm.py
1 file changed, 14 insertions(+), 45 deletions(-)

Approvals:
  Nir Soffer: Looks good to me, but someone else must approve
  Dan Kenigsberg: Looks good to me, approved
  Francesco Romani: Verified



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
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]: virt: sampling: consolidate disk statistics

2014-10-10 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 10:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_merged/4066/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc21-x86_64_merged/52/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el6-x86_64_merged/79/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms_merged_test_debug/274/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_verify-error-codes_merged/5906/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-el7-x86_64_merged/76/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_create-rpms-fc20-x86_64_merged/72/ : 
SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 7:

rebased. Shed more no longer needed code.
documented VIR_TYPED_PARAM_STRING_OKAY as requested.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 7:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12845/ : SUCCESS

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

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1753/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12687/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 7:

(1 comment)

http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 299: # depending on is ok with that. Quoting libvirt header:
Line 300: # Older servers lacked the ability to handle string 
typed
Line 301: # parameters.[...] This flag is automatically set when 
needed,
Line 302: # [...] however, manually setting the flag can be used to
Line 303: # reject servers that cannot return typed strings [...]
Thanks for the verbose explanation!,

Two issues:
- It should come before the line using the cryptic parameter, either the 
blocksStatsFlags() line or the entire loop so the loop is more readable.
- Capitialization: the - The
Line 304: return diskSamples
Line 305: 
Line 306: def _sampleNet(self):
Line 307: netSamples = {}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 7:

(1 comment)

http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 299: # depending on is ok with that. Quoting libvirt header:
Line 300: # Older servers lacked the ability to handle string 
typed
Line 301: # parameters.[...] This flag is automatically set when 
needed,
Line 302: # [...] however, manually setting the flag can be used to
Line 303: # reject servers that cannot return typed strings [...]
 Thanks for the verbose explanation!,
Hmm, according to the current patch, the line with this cryptic flag already 
exists in the older code, but in version 5 
(http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py), it was new. Is this a 
result of a rebase?

If this patch does not add this flag, there is no point to add the 
documentation here, and it is best done in a separate documentation patch 
(which does not need verification).
Line 304: return diskSamples
Line 305: 
Line 306: def _sampleNet(self):
Line 307: netSamples = {}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 7:

(1 comment)

http://gerrit.ovirt.org/#/c/29953/7/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 299: # depending on is ok with that. Quoting libvirt header:
Line 300: # Older servers lacked the ability to handle string 
typed
Line 301: # parameters.[...] This flag is automatically set when 
needed,
Line 302: # [...] however, manually setting the flag can be used to
Line 303: # reject servers that cannot return typed strings [...]
 Hmm, according to the current patch, the line with this cryptic flag alread
Sorry for the rebase noise, I'll rebase less often (here and of course in any 
other future patch).

The cryptic flag was always used since sampleDiskLatency existed; in this patch 
I'm just moving from blockStats to blockStatsFlags in sampleDisk, hence the 
reason why it popped out here.

Will move the explanation in a new separate doc-only patch.
Line 304: return diskSamples
Line 305: 
Line 306: def _sampleNet(self):
Line 307: netSamples = {}


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 8:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12852/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1760/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12694/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit_tests_gerrit_el/11903/ : FAILURE

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-09 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 8: Code-Review+1

Can be nice if you explain the difference between blockStats() and 
blockStatsFlags() in the commit message.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-06 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 5:

(3 comments)

http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 219: (config.getint('vars', 'vm_sample_disk_interval') !=
Line 220:  config.getint('vars', 'vm_sample_disk_latency_interval'))
Line 221: or
Line 222: (config.getint('vars', 'vm_sample_disk_window') !=
Line 223:  config.getint('vars', 'vm_sample_disk_latency_window')))
 Lets remove this unneeded complexity.
OK, will reintroduce, if needed, in a later patch.
Line 224: 
Line 225: self.highWrite = (
Line 226: AdvancedStatsFunction(
Line 227: self._highWrite,


Line 306: 
Line 307: diskSamples = {}
Line 308: for vmDrive in self._vm.getDiskDevices():
Line 309: diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags(
Line 310: vmDrive.name, 
flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY)
 How is this related?
This is the call getDiskLatency uses until this change, and gives us a superset 
of the informations of blockStats().

In the new optimized code paths, we calc both disk stats with the result of one 
blockStatsFlags call here, then this hunk is needed.
Line 311: 
Line 312: return diskSamples
Line 313: 
Line 314: def _sampleDiskLatency(self):


Line 633: def _calcDiskRate(vmDrive, sInfo, eInfo, sampleInterval):
Line 634: return {
Line 635: 'readRate': (
Line 636: (eInfo[vmDrive.name]['rd_bytes'] -
Line 637:  sInfo[vmDrive.name]['rd_bytes'])
 How is this related?
It is because blockStats and blockStatsFlags returns different objects.

blockStats gives back a tuple of longs:

/* convert to a Python tuple of long objects */
if ((info = PyTuple_New(5)) == NULL)
return VIR_PY_NONE;
PyTuple_SetItem(info, 0, libvirt_longlongWrap(stats.rd_req));
PyTuple_SetItem(info, 1, libvirt_longlongWrap(stats.rd_bytes));
PyTuple_SetItem(info, 2, libvirt_longlongWrap(stats.wr_req));
PyTuple_SetItem(info, 3, libvirt_longlongWrap(stats.wr_bytes));
PyTuple_SetItem(info, 4, libvirt_longlongWrap(stats.errs));
return info;

blockStatsFlags returns a dict wrapping libvirt's so-called
'typed parameters'.
Then we must change how we access the fields in the returned object. Maybe I'm 
biased but I find the code a little less obscure with this change applied.
Line 638: / sampleInterval),
Line 639: 'writeRate': (
Line 640: (eInfo[vmDrive.name]['wr_bytes'] -
Line 641:  sInfo[vmDrive.name]['wr_bytes'])


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-06 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 6:

Build Failed 

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12779/ : SUCCESS

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

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1736/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12622/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-06 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-06 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 5:

(1 comment)

http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 306: 
Line 307: diskSamples = {}
Line 308: for vmDrive in self._vm.getDiskDevices():
Line 309: diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags(
Line 310: vmDrive.name, 
flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY)
 This is the call getDiskLatency uses until this change, and gives us a supe
Ok - it would be nice to document this cryptic flag (in another patch if you 
like)
Line 311: 
Line 312: return diskSamples
Line 313: 
Line 314: def _sampleDiskLatency(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-06 Thread mpolednik
Martin Polednik has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 6: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-05 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 5: Code-Review-1

(3 comments)

Please remove unrelated changes.

http://gerrit.ovirt.org/#/c/29953/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 219: (config.getint('vars', 'vm_sample_disk_interval') !=
Line 220:  config.getint('vars', 'vm_sample_disk_latency_interval'))
Line 221: or
Line 222: (config.getint('vars', 'vm_sample_disk_window') !=
Line 223:  config.getint('vars', 'vm_sample_disk_latency_window')))
Lets remove this unneeded complexity.
Line 224: 
Line 225: self.highWrite = (
Line 226: AdvancedStatsFunction(
Line 227: self._highWrite,


Line 306: 
Line 307: diskSamples = {}
Line 308: for vmDrive in self._vm.getDiskDevices():
Line 309: diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags(
Line 310: vmDrive.name, 
flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY)
How is this related?
Line 311: 
Line 312: return diskSamples
Line 313: 
Line 314: def _sampleDiskLatency(self):


Line 633: def _calcDiskRate(vmDrive, sInfo, eInfo, sampleInterval):
Line 634: return {
Line 635: 'readRate': (
Line 636: (eInfo[vmDrive.name]['rd_bytes'] -
Line 637:  sInfo[vmDrive.name]['rd_bytes'])
How is this related?
Line 638: / sampleInterval),
Line 639: 'writeRate': (
Line 640: (eInfo[vmDrive.name]['wr_bytes'] -
Line 641:  sInfo[vmDrive.name]['wr_bytes'])


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-02 Thread nsoffer
Nir Soffer has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 4:

(1 comment)

Partial review

http://gerrit.ovirt.org/#/c/29953/4//COMMIT_MSG
Commit Message:

Line 16: and cheaper to read all the needed information in one go,
Line 17: saving one libvirt call without losing any information.
Line 18: 
Line 19: If the configuration is different, the old behaviour is
Line 20: preserved for the sake of backward compatibility.
Do we need this backward compatibility? I think this configuration is for us, 
so we can modify the behavior easily, not for users. This configuration is not 
documented right?

Lets drop the false backward compatibility and simplify the code.
Line 21: 
Line 22: Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-02 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 4:

(1 comment)

http://gerrit.ovirt.org/#/c/29953/4//COMMIT_MSG
Commit Message:

Line 16: and cheaper to read all the needed information in one go,
Line 17: saving one libvirt call without losing any information.
Line 18: 
Line 19: If the configuration is different, the old behaviour is
Line 20: preserved for the sake of backward compatibility.
 Do we need this backward compatibility? I think this configuration is for u
I agree with you, because the backward compatibility makes the code uglier in 
every way I can conceive.

But we exposed these settings to the user (and long ago), so we are bound to 
keep them for all the 3.x.y cycle, I believe.

Dan?
Line 21: 
Line 22: Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-02 Thread mpolednik
Martin Polednik has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 4: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-02 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 5:

Build Failed 

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

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12737/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1712/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12580/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-10-01 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 4:

Build Failed 

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

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/12699/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1704/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/12544/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Adam Litke ali...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Martin Polednik mpoled...@redhat.com
Gerrit-Reviewer: Nir Soffer nsof...@redhat.com
Gerrit-Reviewer: Vinzenz Feenstra vfeen...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-08-08 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 3:

rebased.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-08-08 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 3:

Build Successful 

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1424/ : 
SUCCESS

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

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11699/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11542/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-07-22 Thread danken
Dan Kenigsberg has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 2: Code-Review+1

(1 comment)

http://gerrit.ovirt.org/#/c/29953/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 185: 
Line 186: def __init__(self, vm):
Line 187: AdvancedStatsThread.__init__(self, log=vm.log, daemon=True)
Line 188: self._vm = vm
Line 189: # ugly hack to preserve backward compatibility
I see no reason to keep a separate vm_sample_disk_latency_interval or ANY 
window != 2. Please merge vm_sample_disk_latency_interval into 
vm_sample_disk_interval in a separate patch.
Line 190: self._splitDiskStats = (
Line 191: (config.getint('vars', 'vm_sample_disk_interval') !=
Line 192:  config.getint('vars', 'vm_sample_disk_latency_interval'))
Line 193: or


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-07-21 Thread fromani
Francesco Romani has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 2: Verified+1

verified with 29951 and 29952 using both the new and old flow.
Stats shows correctly on vdsClient.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-07-14 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 2:

Build Successful 

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

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/11008/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1265/ : 
SUCCESS

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11165/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches


Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-07-11 Thread fromani
Francesco Romani has uploaded a new change for review.

Change subject: virt: sampling: consolidate disk statistics
..

virt: sampling: consolidate disk statistics

We used to have two different sampling, and thus two
libvirt calls for sampling, for disk.
One sampling gathers basic disk data and R/W rate;
the other one gathers the disk latency.

We can read all the needed information in one go, thus saving
one not cheap libvirt call without losing any information.
This patch does that.

Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Signed-off-by: Francesco Romani from...@redhat.com
---
M lib/vdsm/config.py.in
M vdsm/virt/vm.py
2 files changed, 17 insertions(+), 42 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/53/29953/1

diff --git a/lib/vdsm/config.py.in b/lib/vdsm/config.py.in
index 52e0cd9..9f67c25 100644
--- a/lib/vdsm/config.py.in
+++ b/lib/vdsm/config.py.in
@@ -165,10 +165,6 @@
 
 ('vm_sample_disk_window', '2', None),
 
-('vm_sample_disk_latency_interval', '60', None),
-
-('vm_sample_disk_latency_window', '2', None),
-
 ('vm_sample_net_interval', '15', None),
 
 ('vm_sample_net_window', '2', None),
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index 1082d1c..667b9ca 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -205,11 +205,6 @@
 self._sampleDisk,
 config.getint('vars', 'vm_sample_disk_interval'),
 config.getint('vars', 'vm_sample_disk_window')))
-self.sampleDiskLatency = (
-AdvancedStatsFunction(
-self._sampleDiskLatency,
-config.getint('vars', 'vm_sample_disk_latency_interval'),
-config.getint('vars', 'vm_sample_disk_latency_window')))
 self.sampleNet = (
 AdvancedStatsFunction(
 self._sampleNet,
@@ -238,8 +233,8 @@
 
 self.addStatsFunction(
 self.highWrite, self.updateVolumes, self.sampleCpu,
-self.sampleDisk, self.sampleDiskLatency, self.sampleNet,
-self.sampleBalloon, self.sampleVmJobs, self.sampleVcpuPinning,
+self.sampleDisk, self.sampleNet, self.sampleBalloon,
+self.sampleVmJobs, self.sampleVcpuPinning,
 self.sampleCpuTune)
 
 def _highWrite(self):
@@ -265,25 +260,16 @@
 # Avoid queries from storage during recovery process
 return
 
-diskSamples = {}
-for vmDrive in self._vm.getDiskDevices():
-diskSamples[vmDrive.name] = self._vm._dom.blockStats(vmDrive.name)
-
-return diskSamples
-
-def _sampleDiskLatency(self):
-if not self._vm.isDisksStatsCollectionEnabled():
-# Avoid queries from storage during recovery process
-return
 # {'wr_total_times': 0L, 'rd_operations': 9638L,
 # 'flush_total_times': 0L,'rd_total_times': 7622718001L,
 # 'rd_bytes': 85172430L, 'flush_operations': 0L,
 # 'wr_operations': 0L, 'wr_bytes': 0L}
-diskLatency = {}
+diskSamples = {}
 for vmDrive in self._vm.getDiskDevices():
-diskLatency[vmDrive.name] = self._vm._dom.blockStatsFlags(
+diskSamples[vmDrive.name] = self._vm._dom.blockStatsFlags(
 vmDrive.name, flags=libvirt.VIR_TYPED_PARAM_STRING_OKAY)
-return diskLatency
+
+return diskSamples
 
 def _sampleNet(self):
 netSamples = {}
@@ -517,6 +503,13 @@
 # will be None if sampled during recovery
 dStats.update(self._calcDiskRate(vmDrive, sInfo, eInfo,
  sampleInterval))
+dLatency = self._calcDiskLatency(vmDrive, sInfo, eInfo,
+ sampleInterval)
+else:
+dLatency = {'readLatency': '0',
+'writeLatency': '0',
+'flushLatency': '0'}
+dStats.update(dLantency)
 except (AttributeError, KeyError, TypeError, ZeroDivisionError):
 self._log.exception(Disk %s stats not available, dName)
 
@@ -526,29 +519,16 @@
 try:
 return {
 'readRate': (
-(eInfo[vmDrive.name][1] - sInfo[vmDrive.name][1])
+(eInfo[vmDrive.name]['rd_bytes'] -
+ sInfo[vmDrive.name]['rd_bytes'])
  / sampleInterval),
 'writeRate': (
-(eInfo[vmDrive.name][3] - sInfo[vmDrive.name][3])
+(eInfo[vmDrive.name]['wr_bytes'] -
+ sInfo[vmDrive.name]['wr_bytes'])
  / sampleInterval) }
 except (AttributeError, KeyError, TypeError, ZeroDivisionError):
 self._log.exception(Disk %s stats not available, 

Change in vdsm[master]: virt: sampling: consolidate disk statistics

2014-07-11 Thread oVirt Jenkins CI Server
oVirt Jenkins CI Server has posted comments on this change.

Change subject: virt: sampling: consolidate disk statistics
..


Patch Set 1: Code-Review-1 Verified-1

Build Failed 

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

http://jenkins.ovirt.org/job/vdsm_master_pep8_gerrit/10952/ : UNSTABLE

http://jenkins.ovirt.org/job/vdsm_master_virt_functional_tests_gerrit/1240/ : 
There was an infra issue, please contact in...@ovirt.org

http://jenkins.ovirt.org/job/vdsm_master_unit-tests_created/11108/ : SUCCESS

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0dabd079f81270c7099c74469a18f8b23c97cc8c
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani from...@redhat.com
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: No
___
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches