Dan Kenigsberg has posted comments on this change. Change subject: periodic: explicitely track domain availability ......................................................................
Patch Set 13: Code-Review-1 (4 comments) https://gerrit.ovirt.org/#/c/47246/13//COMMIT_MSG Commit Message: Line 25: high scale this is a bit vague. was it the consumption of libvirt worker thread by virDomainGetControlInfo? https://gerrit.ovirt.org/#/c/47246/13/vdsm/virt/periodic.py File vdsm/virt/periodic.py: Line 229: self._get_vms = get_vms Line 230: self._executor = executor Line 231: self._create = create Line 232: self._timeout = timeout Line 233: self._busy_doms = ExpiringCache(ttl) I'm a bit confused - why do we need this one *on top* of the existing VMBulkSampler._skip_doms ? (I'm sure this only proves that I don't understand your suggested implementation) Line 234: Line 235: def __call__(self): Line 236: vms = self._get_vms() Line 237: skipped = [] Line 241: False why bother changing the default=None ? Line 245: op = self._create(vm_obj, self._busy_doms) Line 246: Line 247: if not op.required: Line 248: continue Line 249: # When dealing with blocked domains, we also want to avoid this comment should be kept with its code, and probably rephrased. Line 250: # to pile up jobs that libvirt can't handle and eventually Line 251: # clog it. Line 252: # We don't care too much about precise tracking, so it is Line 253: # still OK if occasional misdetection occours, but we -- To view, visit https://gerrit.ovirt.org/47246 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd90750ab36cb90bad401fab0ff7ff98429e78a5 Gerrit-PatchSet: 13 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: Jenkins CI Gerrit-Reviewer: Martin Polednik <mpoled...@redhat.com> Gerrit-Reviewer: Milan Zamazal <mzama...@redhat.com> 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
