Francesco Romani has posted comments on this change.

Change subject: migration: wait properly for migration to begin
......................................................................


Patch Set 3:

(1 comment)

https://gerrit.ovirt.org/#/c/60073/3/vdsm/virt/migration.py
File vdsm/virt/migration.py:

Line 766:             stats.get(libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED, -1),
Line 767:             stats.get(libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING, -1),
Line 768:             stats.get(libvirt.VIR_DOMAIN_JOB_MEMORY_BPS, -1),
Line 769:             stats.get(libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT, -1),
Line 770:             stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, -1),
> well, in order to make the call on 658 work, we need this not to fail in fr
Right, /me read uncarefully.

I'd say:
1. let's extract ongoing() and make it a free function like

def ongoing(stats):
    try:
        job_type = stats['type']
    except KeyError:
        return False
    else:
        return job_type != libvirt.VIR_DOMAIN_JOB_NONE

Let's use in line 658 like this

            job_stats = self._vm._dom.jobStats()
            if not ongoing(job_stats):
                # migration not yet started
                continue

            progress = Progress.from_job_stats(job_stats)

Now we should be good, we can drop this parsing change and keep the one on line 
704.
Would this work for you?
Line 771:             # available since libvirt 1.3
Line 772:             stats.get('memory_dirty_rate', -1),
Line 773:             # available since libvirt 1.3
Line 774:             stats.get('memory_iteration', -1),


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If1ba369a0992c2473acf1395fad0b0c260c250ba
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Tomas Jelinek <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Milan Zamazal <[email protected]>
Gerrit-Reviewer: Tomas Jelinek <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/admin/lists/[email protected]

Reply via email to