Martin Betak has posted comments on this change.

Change subject: migration: Add Dynamic Bounded Semaphores
......................................................................


Patch Set 22:

(7 comments)

https://gerrit.ovirt.org/#/c/53304/22/tests/vmUtilsTests.py
File tests/vmUtilsTests.py:

Line 157:         self.assertTrue(vm.eventToString(code))
Line 158: 
Line 159: 
Line 160: class DynamicSemaphoreTests(TestCaseBase):
Line 161: 
> let's have a constants here
Done
Line 162:     def setUp(self):
Line 163:         self.sem = utils.DynamicBoundedSemaphore(5)
Line 164: 
Line 165:     def assertAcquirable(self, times=1):


PS22, Line 163: 5
> let's use the new constant here
Done


PS22, Line 177: 5
> and here
Done


PS22, Line 191: 5
> and here
Done


PS22, Line 192: 10
> let's use a constant for this value too
Done


PS22, Line 193: 5)
> express like a difference please
Done


https://gerrit.ovirt.org/#/c/53304/22/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 52: VIR_MIGRATE_PARAM_URI = 'migrate_uri'
Line 53: VIR_MIGRATE_PARAM_BANDWIDTH = 'bandwidth'
Line 54: VIR_MIGRATE_PARAM_GRAPHICS_URI = 'graphics_uri'
Line 55: 
Line 56: 
> spurious line, if you resubmit please drop it.
I was under the assumption that groups of underalted top level definitions are 
supposed to be separated by 2 blank lines. But I don't mind, will drop.
Line 57: incomingMigrations = DynamicBoundedSemaphore(
Line 58:     max(1, config.getint('vars', 'max_incoming_migrations')))
Line 59: 
Line 60: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I67376d0bd990f89e0a013887cef1a0cb05fb855d
Gerrit-PatchSet: 22
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Betak <mbe...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskri...@redhat.com>
Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to