Change in vdsm[master]: virt: Initial support for post-copy migration
From Dan Kenigsberg : Dan Kenigsberg has submitted this change and it was merged. Change subject: virt: Initial support for post-copy migration .. virt: Initial support for post-copy migration This patch introduces the basics of switching to post-copy migration when present in the convergence schedule. It is an initial step towards post-copy migration feature in oVirt. Further changes, most notably support in Engine, are needed. As for Vdsm itself, there are several problems to solve in the followup patches, especially: - Destination actions that are be performed after pre-copy migration finishes should be also performed when pre-copy migration switches to post-copy. The VM is already running on the destination at the moment so it must be properly set up immediately, without waiting for migration completion. - The source host doesn't provide (real) stats after switching to post-copy mode. The stats are available on the destination host, with the exception of migration progress and migration downtime, after switching to post-copy so they must be retrieved from there. Additionally, downtime is reported in a bit different way than for pre-copy migrations. We must inform Engine what's happening so that it knows where and how to get all the data. There are some circumstances under which post-copy migration doesn't work, such as when using (non-transparent) huge pages or RDMA. If libvirt can detect it, it may fail already at the start of the migration when VIR_MIGRATE_POSTCOPY flag is present. We don't want to block migrations completely if something like that happens unexpectedly. So we add VIR_MIGRATE_POSTCOPY flag only when post-copy migration is actually requested. This way, if the migration can't be started with post-copy, another schedule may be attempted. Please note that (correctly working) post-copy migration requires recent QEMU and sufficiently new libvirt. The crucial dependencies are handled in another patch. Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Signed-off-by: Milan Zamazal Bug-Url: https://bugzilla.redhat.com/1354343 --- M lib/api/vdsm-api.yml M vdsm/virt/migration.py M vdsm/virt/vm.py 3 files changed, 63 insertions(+), 4 deletions(-) Approvals: Jenkins CI: Passed CI tests Francesco Romani: Looks good to me, approved Milan Zamazal: Verified -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 18 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek Gerrit-Reviewer: gerrit-hooks ___ 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 12: Code-Review+1 (1 comment) we just need the packages. https://gerrit.ovirt.org/#/c/62873/12/vdsm/virt/migration.py File vdsm/virt/migration.py: PS12, Line 485: # Migration may fail immediately when VIR_MIGRATE_POSTCOPY is : # present in the following situations: : # - The transport is not capable of full bidirectional : # connectivity: RDMA, tunnelled, pipe. : # - Huge pages are used (doesn't apply to transparent huge pages). : # - QEMU uses a file as a backing for memory. : # - Perhaps non-shared block storage may cause some trouble. : if self._use_convergence_schedule and \ :not self._tunneled: : for s in self._convergence_schedule.get('stalling', []): : action = s.get('action', {}).get('name') : if action == CONVERGENCE_SCHEDULE_POST_COPY: : flags |= libvirt.VIR_MIGRATE_POSTCOPY : break it would be nice to factor this code in a method, because I want to refactor all the flags selection code in a not so distant feature. It's just a "nice to have", however, so I'm not pushing for thiss -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 12: I moved the migration switching and the post-copy flag to Vm as was suggested in and as a preparation for the followup patches. (Please note that I rebased the whole topic on master, so don't get confused if you check diffs against last versions.) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 12: * Update Tracker::#1354343::OK, status: POST * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1354343::OK, public bug * Check Product::IGNORE, not relevant for branch: master * Check TM::IGNORE, not relevant for branch: master * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 11: Code-Review+1 (1 comment) ok once the packages are available https://gerrit.ovirt.org/#/c/62873/11/vdsm.spec.in File vdsm.spec.in: Line 181: Requires: libvirt-client Line 182: Requires: libvirt-daemon-config-nwfilter Line 183: Requires: libvirt-lock-sanlock Line 184: Line 185: %if 0%{?fedora} my comment in the old code still stands :) Line 186: Requires: libvirt-python >= 1.3.5 Line 187: Requires: libvirt-daemon-kvm >= 1.3.5 Line 188: %else # rhel Line 189: Requires: libvirt-daemon-kvm >= 1.3.3 -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 11: We are now more careful with POSTCOPY migration flag. -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 11: * Update Tracker::#1354343::OK, status: POST * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1354343::OK, public bug * Check Product::IGNORE, not relevant for branch: master * Check TM::IGNORE, not relevant for branch: master * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 10: (1 comment) https://gerrit.ovirt.org/#/c/62873/10/vdsm/virt/migration.py File vdsm/virt/migration.py: PS10, Line 720: if not self._in_post_copy and\ : lastDataRemaining is not None and\ : lastDataRemaining < progress.data_remaining: I hate this block. Totally not your fault, obviously, but this code is begging for a refactor. Let's schedule this after your work is merged, hopefully the addition of the post copy will not make this code much worse. -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 10: (4 comments) https://gerrit.ovirt.org/#/c/62873/8/vdsm.spec.in File vdsm.spec.in: Line 187: Requires: libvirt-daemon-kvm >= 1.3.5 Line 188: %else # rhel Line 189: Requires: libvirt-daemon-kvm >= 1.3.3 Line 190: Requires: libvirt-python >= 1.3.4 Line 191: %endif > ok, this may need to be split in a separate change, depending on the time o We'll see. The problem is that once we removed the backward compatibility code from this patch, we must require corresponding libvirt versions, otherwise we crash on libvirt.VIR_MIGRATE_POSTCOPY (we can make a workaround if it's still a problem at the time of merge). Line 192: Line 193: # iscsi-intiator versions Line 194: %if 0%{?rhel} Line 195: Requires: iscsi-initiator-utils https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: (libvirt.VIR_MIGRATE_COMPRESSED if Line 481: self._compressed else 0) | Line 482: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 483: self._autoConverge else 0)) Line 484: > Vdsm *should* be run in F24, or, to word it differently, I'm not aware of a OK, thanks for confirming we should be fine in this regard. Line 485: self._vm._dom.migrateToURI3(duri, params, flags) Line 486: else: Line 487: self._raiseAbortError() Line 488: https://gerrit.ovirt.org/#/c/62873/8/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 478 Line 479 Line 480 Line 481 Line 482 > unrelated, please drop this change if you resubmit Done PS8, Line 778: self._vm.log.info('Switching to post-copy mig > ugh, this reads so C-ish :) Well, I don't know how to identify the C-ishness of the condition. Thinking about it I tend to use Lisp style and to avoid Prolog style (to make clear we are in the procedural and not declarative programming area). I admit I do some C++ programming, but I may be blamed for excessive use of virtual methods there, so I'd say I'm more influenced by Python than C/C++. Dancing among those subtle and delicate issues, I may indeed overlook that I use C-ish constructs in completely inappropriate places. So I changed it as you suggest, to be consistent with the places where we use a similar construct. -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 10: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 9: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 482: self._autoConverge else 0)) Line 483: if self._use_convergence_schedule: Line 484: # TODO: Replace this whole block with simple flag setting once > I'm not sure Vdsm is already capable of running on Fedora 24, but OK, this Vdsm *should* be run in F24, or, to word it differently, I'm not aware of any bug in this regard. Line 485: # Vdsm depends on proper versions of QEMU and libvirt. Line 486: try: Line 487: flags |= libvirt.VIR_MIGRATE_POSTCOPY Line 488: except AttributeError: -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 8: (3 comments) partial review https://gerrit.ovirt.org/#/c/62873/8/vdsm.spec.in File vdsm.spec.in: Line 187: Requires: libvirt-daemon-kvm >= 1.3.5 Line 188: %else # rhel Line 189: Requires: libvirt-daemon-kvm >= 1.3.3 Line 190: Requires: libvirt-python >= 1.3.4 Line 191: %endif ok, this may need to be split in a separate change, depending on the time of availability of the packages Line 192: Line 193: # iscsi-intiator versions Line 194: %if 0%{?rhel} Line 195: Requires: iscsi-initiator-utils https://gerrit.ovirt.org/#/c/62873/8/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 478 Line 479 Line 480 Line 481 Line 482 unrelated, please drop this change if you resubmit PS8, Line 778: if self._vm._dom.migrateStartPostCopy(0) < 0: ugh, this reads so C-ish :) I'd prefer to use this idiom: ret = self._vm.dom.migrateStartPostCopy(0) if ret < 0: #... but this is mostly taste and personal preference, so I'll not press much for this. -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Michal Skrivanek has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 827: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED], Line 828: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING], Line 829: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_BPS], Line 830: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT], Line 831: stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, 0), > Documentation doesn't say under what circumstances the attribute is present thanks, then that patch has quite a priority since for legacy profiles we do not use compression, and since https://gerrit.ovirt.org/#/c/64178/ we do not enable it either for the default profile Line 832: # available since libvirt 1.3 Line 833: stats.get('memory_dirty_rate', -1), Line 834: # available since libvirt 1.3 Line 835: stats.get('memory_iteration', -1), -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: (4 comments) https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 482: self._autoConverge else 0)) Line 483: if self._use_convergence_schedule: Line 484: # TODO: Replace this whole block with simple flag setting once > sure we do, but those are using upstream qemu-kvm which has it already I'm not sure Vdsm is already capable of running on Fedora 24, but OK, this is for the future, let's bump the version requirements and remove this piece of code. Line 485: # Vdsm depends on proper versions of QEMU and libvirt. Line 486: try: Line 487: flags |= libvirt.VIR_MIGRATE_POSTCOPY Line 488: except AttributeError: Line 490: pass Line 491: # There is a QEMU bug causing post-copy migration fail when Line 492: # (XBZRLE) compression is enabled. So let's disable the Line 493: # compression until we depend on a fixed QEMU version. Line 494: # See https://bugzilla.redhat.com/1368422. > for now, correct. So we won't include it in any profile. OK, done. Line 495: if flags & libvirt.VIR_MIGRATE_POSTCOPY: Line 496: flags &= ~libvirt.VIR_MIGRATE_COMPRESSED Line 497: self._vm._dom.migrateToURI3(duri, params, flags) Line 498: else: Line 787:downtime) Line 788: self._vm._dom.migrateSetMaxDowntime(downtime, 0) Line 789: elif action == CONVERGENCE_SCHEDULE_POST_COPY: Line 790: self._vm.log.info('Switching to post-copy migration') Line 791: if not hasattr(self._vm._dom, 'migrateStartPostCopy'): > again, no need for that soon, so please do not handle it in the patch Done Line 792: self._vm.log.warn('Failed to switch to post-copy migration: ' Line 793: 'not supported') Line 794: elif self._vm._dom.migrateStartPostCopy(0) < 0: Line 795: # Do nothing for now; the next action will be invoked after a Line 827: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED], Line 828: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING], Line 829: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_BPS], Line 830: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT], Line 831: stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, 0), > hm, isn't it a bug then? can you please onfirm, i would expect it's 0 when Documentation doesn't say under what circumstances the attribute is present or not, I can confirm the problem exists (with pre-copy too), and looking into the libvirt source code the non-presence of any compression stats when compression is not enabled is clearly intentional. Moved to a separate patch. Line 832: # available since libvirt 1.3 Line 833: stats.get('memory_dirty_rate', -1), Line 834: # available since libvirt 1.3 Line 835: stats.get('memory_iteration', -1), -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 8: * update_tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Michal Skrivanek has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: (3 comments) https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 482: self._autoConverge else 0)) Line 483: if self._use_convergence_schedule: Line 484: # TODO: Replace this whole block with simple flag setting once > Do we care about non-RHEL/CentOS systems (e.g. Fedora)? sure we do, but those are using upstream qemu-kvm which has it already Line 485: # Vdsm depends on proper versions of QEMU and libvirt. Line 486: try: Line 487: flags |= libvirt.VIR_MIGRATE_POSTCOPY Line 488: except AttributeError: Line 490: pass Line 491: # There is a QEMU bug causing post-copy migration fail when Line 492: # (XBZRLE) compression is enabled. So let's disable the Line 493: # compression until we depend on a fixed QEMU version. Line 494: # See https://bugzilla.redhat.com/1368422. > So are we "never" going to use compression with post-copy? for now, correct. So we won't include it in any profile. Later on once it works we people can enable it without any vdsm change Line 495: if flags & libvirt.VIR_MIGRATE_POSTCOPY: Line 496: flags &= ~libvirt.VIR_MIGRATE_COMPRESSED Line 497: self._vm._dom.migrateToURI3(duri, params, flags) Line 498: else: Line 827: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED], Line 828: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING], Line 829: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_BPS], Line 830: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT], Line 831: stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, 0), > When compression is disabled (at least with post-copy enabled), this item i hm, isn't it a bug then? can you please onfirm, i would expect it's 0 when compression is diabled regardless pre or post opy Line 832: # available since libvirt 1.3 Line 833: stats.get('memory_dirty_rate', -1), Line 834: # available since libvirt 1.3 Line 835: stats.get('memory_iteration', -1), -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 7: (3 comments) https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 482: self._autoConverge else 0)) Line 483: if self._use_convergence_schedule: Line 484: # TODO: Replace this whole block with simple flag setting once > we're going to do that very soon in 4.0.z, probably before this patch is me Do we care about non-RHEL/CentOS systems (e.g. Fedora)? Line 485: # Vdsm depends on proper versions of QEMU and libvirt. Line 486: try: Line 487: flags |= libvirt.VIR_MIGRATE_POSTCOPY Line 488: except AttributeError: Line 490: pass Line 491: # There is a QEMU bug causing post-copy migration fail when Line 492: # (XBZRLE) compression is enabled. So let's disable the Line 493: # compression until we depend on a fixed QEMU version. Line 494: # See https://bugzilla.redhat.com/1368422. > since the policy wouldn't include it we can leave it as a qemu issue only So are we "never" going to use compression with post-copy? Line 495: if flags & libvirt.VIR_MIGRATE_POSTCOPY: Line 496: flags &= ~libvirt.VIR_MIGRATE_COMPRESSED Line 497: self._vm._dom.migrateToURI3(duri, params, flags) Line 498: else: Line 827: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED], Line 828: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING], Line 829: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_BPS], Line 830: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT], Line 831: stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, 0), > why? When compression is disabled (at least with post-copy enabled), this item is not present (resulting in KeyError) and 0 bytes are compressed. It should probably be moved to a separate patch. Line 832: # available since libvirt 1.3 Line 833: stats.get('memory_dirty_rate', -1), Line 834: # available since libvirt 1.3 Line 835: stats.get('memory_iteration', -1), -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Michal Skrivanek has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: (6 comments) https://gerrit.ovirt.org/#/c/62873/4/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 473: flags = (libvirt.VIR_MIGRATE_LIVE | Line 474: libvirt.VIR_MIGRATE_PEER2PEER | Line 475: (libvirt.VIR_MIGRATE_TUNNELLED if Line 476: self._tunneled else 0) | Line 477: (libvirt.VIR_MIGRATE_ABORT_ON_ERROR if 7.3 is coming out really soon, and will be adopted in 4.0.z already. Likely before this patch gets in Line 478: self._abortOnError else 0) | Line 479: (libvirt.VIR_MIGRATE_COMPRESSED if Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 803: self.stop() Line 804: Line 805: Line 806: _Progress = collections.namedtuple('_Progress', [ Line 807: 'job_type', 'time_elapsed', 'data_total', ? Line 808: 'data_processed', 'data_remaining', Line 809: 'mem_total', 'mem_processed', 'mem_remaining', Line 810: 'mem_bps', 'mem_constant', 'compression_bytes', Line 811: 'dirty_rate', 'mem_iteration' https://gerrit.ovirt.org/#/c/62873/6/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 480: self._compressed else 0) | Line 481: (libvirt.VIR_MIGRATE_AUTO_CONVERGE if Line 482: self._autoConverge else 0)) Line 483: if self._use_convergence_schedule: Line 484: # TODO: Replace this whole block with simple flag setting once we're going to do that very soon in 4.0.z, probably before this patch is merged Line 485: # Vdsm depends on proper versions of QEMU and libvirt. Line 486: try: Line 487: flags |= libvirt.VIR_MIGRATE_POSTCOPY Line 488: except AttributeError: Line 490: pass Line 491: # There is a QEMU bug causing post-copy migration fail when Line 492: # (XBZRLE) compression is enabled. So let's disable the Line 493: # compression until we depend on a fixed QEMU version. Line 494: # See https://bugzilla.redhat.com/1368422. since the policy wouldn't include it we can leave it as a qemu issue only Line 495: if flags & libvirt.VIR_MIGRATE_POSTCOPY: Line 496: flags &= ~libvirt.VIR_MIGRATE_COMPRESSED Line 497: self._vm._dom.migrateToURI3(duri, params, flags) Line 498: else: Line 787:downtime) Line 788: self._vm._dom.migrateSetMaxDowntime(downtime, 0) Line 789: elif action == CONVERGENCE_SCHEDULE_POST_COPY: Line 790: self._vm.log.info('Switching to post-copy migration') Line 791: if not hasattr(self._vm._dom, 'migrateStartPostCopy'): again, no need for that soon, so please do not handle it in the patch Line 792: self._vm.log.warn('Failed to switch to post-copy migration: ' Line 793: 'not supported') Line 794: elif self._vm._dom.migrateStartPostCopy(0) < 0: Line 795: # Do nothing for now; the next action will be invoked after a Line 827: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_PROCESSED], Line 828: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_REMAINING], Line 829: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_BPS], Line 830: stats[libvirt.VIR_DOMAIN_JOB_MEMORY_CONSTANT], Line 831: stats.get(libvirt.VIR_DOMAIN_JOB_COMPRESSION_BYTES, 0), why? Line 832: # available since libvirt 1.3 Line 833: stats.get('memory_dirty_rate', -1), Line 834: # available since libvirt 1.3 Line 835: stats.get('memory_iteration', -1), -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 7: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 6: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 5: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 5: Another safety check: Let's not crash on post-copy switch when it is in the schedule but not supported by libvirt. -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 4: (2 comments) I additionally disabled compression with post-copy due to the QEMU bug, in order to support various environments properly until we start depending on new QEMU. https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: PS2, Line 466: flags = (libvirt.VIR_MIGRATE_LIVE | : libvirt.VIR_MIGRATE_PEER2PEER | : (libvirt.VIR_MIGRATE_TUNNELLED if : self._tunneled else 0) | : (libvirt.VIR_MIGRATE_ABORT_ON_ERROR if : self._abortOnError else 0) | : (libvirt.VIR_MIGRATE_COMPRESSED if : self._compressed else 0) | : (libvirt.VIR_MIGRATE_AUTO_CONVERGE if : self._autoConverge else 0)) : if self._use_convergence_schedule: : # TODO: Replace this whole block with simple flag setting once : # Vdsm depends on proper versions of QEMU and libvirt. : try: : flags |= libvirt.VIR_MIGRATE_POSTCOPY : except A > this code is begging for a cleanup, but I'll leave entirely to your good wi Not sure what kind of cleanup would you like. It'd be for a separate patch anyway. Line 691: (0 < migrationMaxTime < now - self._startTime): Line 692: self._vm.log.warn('The migration took %d seconds which is ' Line 693: 'exceeding the configured maximum time ' Line 694: 'for migrations of %d seconds. The ' Line 695: 'migration will be aborted.', > OK, this makes sense: once we attempt "postcopy", there is no way back. So, > I'm fine with that, we just need to document this. Comment added. > It could be a good idea to document somewhere (Vdsm or Engine) the > convergence schedule actions. Yes. I'm thinking about the proper place. Line 696: now - self._startTime, Line 697: migrationMaxTime) Line 698: self._vm._dom.abortJob() Line 699: self.stop() -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 4: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 669: Line 670: now = time.time() Line 671: if self._in_post_copy: Line 672: self._vm.log.debug('Post-copy migration still in progress: %d', Line 673:progress.data_remaining) > OK, at this stage "progress" is probably the only thing we need. As for stats, according to Arik, it's not a problem if they are not available in one sampling cycle (run every 15 seconds), but it's not acceptable if they are not available for let's say one minute. While post-copy migration is currently supposed to be fast (migration bandwith limits don't apply, which is a QEMU/libvirt bug), there's no guarantee it doesn't take long time for whatever reason. So we should be ready to handle that. I suggest making a followup patch that will ask the destination host for the VM stats if the post-copy phase takes more than some time to finish. What do you think? Do we already use some sort of direct communication between the hosts anywhere in Vdsm? Line 674: elif not self._use_conv_schedule and\ Line 675: (0 < migrationMaxTime < now - self._startTime): Line 676: self._vm.log.warn('The migration took %d seconds which is ' Line 677: 'exceeding the configured maximum time ' -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: -Code-Review (2 comments) https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 669: Line 670: now = time.time() Line 671: if self._in_post_copy: Line 672: self._vm.log.debug('Post-copy migration still in progress: %d', Line 673:progress.data_remaining) > To be clear: We need to fetch stats, progress is available here. OK, at this stage "progress" is probably the only thing we need. Line 674: elif not self._use_conv_schedule and\ Line 675: (0 < migrationMaxTime < now - self._startTime): Line 676: self._vm.log.warn('The migration took %d seconds which is ' Line 677: 'exceeding the configured maximum time ' Line 691: ' > lowmark (%sMiB).' Line 692: ' Refer to RHBZ#919201.', Line 693: progress.data_remaining / Mbytes, lowmark / Mbytes) Line 694: Line 695: if not self._in_post_copy and\ > Actually not: Post-copy migration, once successfully started, can finish in OK, this makes sense: once we attempt "postcopy", there is no way back. So, "postcopy" effectively marks the end of one convergence schedule (everything else that may follow is ignored). I'm fine with that, we just need to document this. It could be a good idea to document somewhere (Vdsm or Engine) the convergence schedule actions. Line 696: lastDataRemaining is not None and\ Line 697: lastDataRemaining < progress.data_remaining: Line 698: iterationCount += 1 Line 699: self._vm.log.debug('new iteration detected: %i', -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 691: ' > lowmark (%sMiB).' Line 692: ' Refer to RHBZ#919201.', Line 693: progress.data_remaining / Mbytes, lowmark / Mbytes) Line 694: Line 695: if not self._in_post_copy and\ > Nobody, in post-copy _next_action is not called after the migration finishe Actually not: Post-copy migration, once successfully started, can finish in only two ways: a) success, b) hard failure. In both the cases the migration is finished, as well as the monitor thread. So it's no longer possible to continue with the convergence schedule. The only case where the convergence schedule still proceeds after the post-copy switch is if switching to post-copy mode fails. This is the case handled at lines 755-757; _in_post_copy flag is not set in that case and we proceed with the convergence schedule normally (aborting the migration is probably the right followup schedule action). Line 696: lastDataRemaining is not None and\ Line 697: lastDataRemaining < progress.data_remaining: Line 698: iterationCount += 1 Line 699: self._vm.log.debug('new iteration detected: %i', -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 2: (1 comment) https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 669: Line 670: now = time.time() Line 671: if self._in_post_copy: Line 672: self._vm.log.debug('Post-copy migration still in progress: %d', Line 673:progress.data_remaining) > No, libvirt isn't that kind, we must fetch it from the destination ourselve To be clear: We need to fetch stats, progress is available here. Line 674: elif not self._use_conv_schedule and\ Line 675: (0 < migrationMaxTime < now - self._startTime): Line 676: self._vm.log.warn('The migration took %d seconds which is ' Line 677: 'exceeding the configured maximum time ' -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
Milan Zamazal has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: (2 comments) https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: Line 669: Line 670: now = time.time() Line 671: if self._in_post_copy: Line 672: self._vm.log.debug('Post-copy migration still in progress: %d', Line 673:progress.data_remaining) > I guess this is the part on which we need data transfer from dest, right? No, libvirt isn't that kind, we must fetch it from the destination ourselves. Line 674: elif not self._use_conv_schedule and\ Line 675: (0 < migrationMaxTime < now - self._startTime): Line 676: self._vm.log.warn('The migration took %d seconds which is ' Line 677: 'exceeding the configured maximum time ' Line 691: ' > lowmark (%sMiB).' Line 692: ' Refer to RHBZ#919201.', Line 693: progress.data_remaining / Mbytes, lowmark / Mbytes) Line 694: Line 695: if not self._in_post_copy and\ > so in post copy mode who's taking care of the convergence schedule? Nobody, in post-copy _next_action is not called after the migration finishes and that must be fixed. Line 696: lastDataRemaining is not None and\ Line 697: lastDataRemaining < progress.data_remaining: Line 698: iterationCount += 1 Line 699: self._vm.log.debug('new iteration detected: %i', -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Milan Zamazal Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: Code-Review-1 please check my comments to PS2 -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
Francesco Romani has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 2: (3 comments) I'm obviously fine with the idea, and the general approach seems OK; I want to make sure we cover all sides, so few comments inside and -1 for visbility https://gerrit.ovirt.org/#/c/62873/2/vdsm/virt/migration.py File vdsm/virt/migration.py: PS2, Line 466: flags = (libvirt.VIR_MIGRATE_LIVE | : libvirt.VIR_MIGRATE_PEER2PEER | : (libvirt.VIR_MIGRATE_TUNNELLED if : self._tunneled else 0) | : (libvirt.VIR_MIGRATE_ABORT_ON_ERROR if : self._abortOnError else 0) | : (libvirt.VIR_MIGRATE_COMPRESSED if : self._compressed else 0) | : (libvirt.VIR_MIGRATE_AUTO_CONVERGE if : self._autoConverge else 0)) : if self._autoConverge: : try: : flags |= libvirt.VIR_MIGRATE_POSTCOPY : except AttributeError: : # Not present in libvirt < 1.3.3 : pass this code is begging for a cleanup, but I'll leave entirely to your good will :) Line 669: Line 670: now = time.time() Line 671: if self._in_post_copy: Line 672: self._vm.log.debug('Post-copy migration still in progress: %d', Line 673:progress.data_remaining) I guess this is the part on which we need data transfer from dest, right? Or is libvirt kind enough to let us have updated information on source side? Line 674: elif not self._use_conv_schedule and\ Line 675: (0 < migrationMaxTime < now - self._startTime): Line 676: self._vm.log.warn('The migration took %d seconds which is ' Line 677: 'exceeding the configured maximum time ' Line 691: ' > lowmark (%sMiB).' Line 692: ' Refer to RHBZ#919201.', Line 693: progress.data_remaining / Mbytes, lowmark / Mbytes) Line 694: Line 695: if not self._in_post_copy and\ so in post copy mode who's taking care of the convergence schedule? Line 696: lastDataRemaining is not None and\ Line 697: lastDataRemaining < progress.data_remaining: Line 698: iterationCount += 1 Line 699: self._vm.log.debug('new iteration detected: %i', -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Tomas Jelinek 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: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 3: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 2: * #1354343::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1354343::OK, public bug * Check Product::#1354343::OK, Correct classification oVirt * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Tomas Jelinek 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]: virt: Initial support for post-copy migration
gerrit-hooks has posted comments on this change. Change subject: virt: Initial support for post-copy migration .. Patch Set 1: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/62873 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4c9911f47331120a1b78326044a4949abf35d5fe Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal 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