Change in vdsm[master]: virt: Initial support for post-copy migration

2016-11-22 Thread Code Review
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

2016-10-26 Thread fromani
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

2016-10-21 Thread mzamazal
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

2016-10-21 Thread automation
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

2016-10-17 Thread fromani
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

2016-10-14 Thread mzamazal
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

2016-10-14 Thread automation
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

2016-09-27 Thread fromani
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

2016-09-26 Thread mzamazal
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

2016-09-26 Thread automation
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

2016-09-26 Thread automation
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

2016-09-26 Thread fromani
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

2016-09-26 Thread fromani
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

2016-09-26 Thread michal . skrivanek
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

2016-09-26 Thread mzamazal
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

2016-09-26 Thread automation
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

2016-09-26 Thread michal . skrivanek
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

2016-09-26 Thread mzamazal
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

2016-09-23 Thread michal . skrivanek
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

2016-09-23 Thread automation
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

2016-09-08 Thread automation
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

2016-08-29 Thread automation
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

2016-08-29 Thread mzamazal
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

2016-08-29 Thread mzamazal
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

2016-08-29 Thread automation
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

2016-08-29 Thread mzamazal
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

2016-08-28 Thread fromani
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

2016-08-26 Thread mzamazal
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

2016-08-26 Thread mzamazal
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

2016-08-26 Thread mzamazal
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

2016-08-26 Thread fromani
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

2016-08-26 Thread fromani
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

2016-08-26 Thread automation
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

2016-08-26 Thread automation
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

2016-08-26 Thread automation
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