Change in vdsm[master]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 12: * update_tracker: OK * Set MODIFIED::bug 1382578#1382578::IGNORE, skipping for branch 'master' * Set MODIFIED::bug 1382578bug 1382583#1382583::IGNORE, skipping for branch 'master' -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Dan Kenigsberg has submitted this change and it was merged. Change subject: vm: introduce a `monitorable' attribute .. vm: introduce a `monitorable' attribute In the change I3e61626625a2e0517d55dc61e361f3f5eb690c00 we fixed the stats_age reporting, in order to correctly report the real age of VM monitoring samples. In the same change we acknowledged a possible change in behaviour: depending on the timing of operations and on the load of the host, VMs could be reported 'Unresponsive' for a little time while starting up or shutting down. This information is tecnhically correct: the monitoring subsystem dutifully reports unknown sampling data, which is translated into outdated values, thus unresponsive VM, but it is misleading for both Engine and users. There are two root causes here: 1. creation, destruction and monitoring of a VM are all operations which are asynchronous to each other, for both performance and historical reasons - and both reasons are still relevant today. 2. the semantic of this reporting (VM unresponsive for stats too old) is poorly defined, so the right thing is to keep backward compatibility, lacking better description. To solve this issue we generalize the old VM.incomingMigrationPending attribute into a 'monitorable' attribute. A VM is monitorable if it is in a meaningful state, so after the creation process is completed, or before the shutdown process is initiated. We should adjust responsiveness for stats too old only if a VM is monitorable, not inconditionally. Otherwise we can have misreports on slow startup or slow shutdowns. Please note that the `monitorable' state share similarities with the VM state (VM.lastStatus attribute), but the two concepts overlap only for a small part. Lacking a precise definition, we use two independent variables to track those attributes. Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Backport-To: 4.0 Backport-To: 3.6 Bug-Url: https://bugzilla.redhat.com/1382578 Bug-Url: https://bugzilla.redhat.com/1382583 Signed-off-by: Francesco Romani Reviewed-on: https://gerrit.ovirt.org/65590 Reviewed-by: Milan Zamazal Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg --- M lib/vdsm/virt/periodic.py M lib/vdsm/virt/vmstats.py M tests/periodicTests.py M tests/vmStatsTests.py M vdsm/virt/vm.py 5 files changed, 67 insertions(+), 27 deletions(-) Approvals: Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified Milan Zamazal: Looks good to me, but someone else must approve -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 12 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Dan Kenigsberg has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 11: Code-Review+2 raising score -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 11: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Francesco Romani has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 10: Verified+1 The purpose of this patch is 1. make sure VM is not misreported (actually: overzelously reported) unresponsive when starting up slowly or shutting down sloly 2. reduce misleading "XXX operation cannot run on ABC" messages ot check this, I verified again that both of the above doesn't happen if a VM starts up or shuts down slowly. Overloading one host is racy and tricky, so I tried reproducing in a more controlled environment, using just one VM, but injecting sleep()s to ensure the right sequence or events - or just slow operations. - startup: slow down domDependentInit right at the beginning. VM is still reported as responsive even if sleeps for 100s. No extra messages in the logs - shutdown: started one VM, stopped Engine, killed QEMU. Did a few tries to catch the small race here (a Down VM doesn't report responsiveness), looks ok as well. - non regression: SIGSTOP'd QEMU while Vdsm was running, VM is reported unresponsive after a timeout, and responsive again when QEMU is SIGCONT'd. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Milan Zamazal has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 10: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 10: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Milan Zamazal has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 9: Code-Review+1 (3 comments) I suggest polishing the comment, otherwise it looks fine. https://gerrit.ovirt.org/#/c/65590/9/vdsm/virt/vm.py File vdsm/virt/vm.py: PS9, Line 1275: p Prevent PS9, Line 1276: as monitorable the VM the VM as monitorable PS9, Line 1278: vm VM -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 9: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Francesco Romani has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 8: (1 comment) https://gerrit.ovirt.org/#/c/65590/8/tests/periodicTests.py File tests/periodicTests.py: PS8, Line 384: self.ready = True > Missed this one before, I think you could just return True in isDomainReady Better to simplify. Minor, but should be addressed, and it will. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Martin Polednik has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 8: Code-Review+1 (1 comment) https://gerrit.ovirt.org/#/c/65590/8/tests/periodicTests.py File tests/periodicTests.py: PS8, Line 384: self.ready = True Missed this one before, I think you could just return True in isDomainReadyForCommands unless there is any fancier hidden state change somewhere. Minor. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 8: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Francesco Romani has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/65590/7/tests/periodicTests.py File tests/periodicTests.py: PS7, Line 269: self.assertEqual(_Visitor.VMS.get(vm_id), None) > You're asking for apples and getting oranges: skip_ids are NOT the vm ids. Indeed I am. Fixing. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Martin Polednik has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 7: (1 comment) https://gerrit.ovirt.org/#/c/65590/7/tests/periodicTests.py File tests/periodicTests.py: PS7, Line 269: self.assertEqual(_Visitor.VMS.get(vm_id), None) You're asking for apples and getting oranges: skip_ids are NOT the vm ids. self.assertEqual(_Visitor.VMS.get(_fake_vm_id(vm_id)), None) To compare same (semantic) objects. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Francesco Romani has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 7: Verified+1 changes since last verification: fixed tests (now they are simpler, and works OK) -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 7: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Bug-Url::IGNORE, not relevant for 'Red Hat' classification * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 6: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Francesco Romani has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 5: Verified+1 works, but it is not enough to fix the bugs. -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 5: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 4: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Milan Zamazal has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 3: Code-Review+1 -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 3: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
Milan Zamazal has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 2: Code-Review+1 (Except for the style check.) -- To view, visit https://gerrit.ovirt.org/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 2: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik Gerrit-Reviewer: Michal Skrivanek Gerrit-Reviewer: Milan Zamazal 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]: vm: introduce a `monitorable' attribute
gerrit-hooks has posted comments on this change. Change subject: vm: introduce a `monitorable' attribute .. Patch Set 1: * Update Tracker::#1382578::IGNORE, not relevant for Red Hat classification * Update Tracker::#1382583::IGNORE, not relevant for Red Hat classification * Check Bug-Url::IGNORE, not relevant for branch: master * Check Public Bug::#1382578::OK, public bug * Check Public Bug::#1382583::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/65590 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani 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]: vm: introduce a `monitorable' attribute
Francesco Romani has uploaded a new change for review.
Change subject: vm: introduce a `monitorable' attribute
..
vm: introduce a `monitorable' attribute
In the change I3e61626625a2e0517d55dc61e361f3f5eb690c00
we fixed the stats_age reporting, in order to correctly report
the real age of VM monitoring samples.
In the same change we acknowledged a possible change in behaviour:
depending on the timing of operations and on the load of the host,
VMs could be reported 'Unresponsive' for a little time while
starting up or shutting down.
This information is tecnhically correct: the monitoring subsystem
dutifully reports unknown sampling data, which is translated into
outdated values, thus unresponsive VM, but it is misleading for
both Engine and users.
There are two root causes here:
1. creation, destruction and monitoring of a VM are all operations
which are asynchronous to each other, for both performance and
historical reasons - and both reasons are still relevant today.
2. the semantic of this reporting (VM unresponsive for stats too old)
is poorly defined, so the right thing is to keep backward
compatibility, lacking better description.
To solve this issue we generalize the old VM.incomingMigrationPending
attribute into a 'monitorable' attribute.
A VM is monitorable if it is in a meaningful state, so after the
creation process is completed, or before the shutdown process is
initiated.
Please note that the `monitorable' state share similarities with
the VM state (VM.lastStatus attribute), but the two concepts
overlap only for a small part.
Lacking a precise definition, we use two independent variables
to track those attributes.
Change-Id: Idb12277a5f0fdaf680032af12fa7c104c3bd6ffb
Backport-To: 4.0
Backport-To: 3.6
Bug-Url: https://bugzilla.redhat.com/1382578
Bug-Url: https://bugzilla.redhat.com/1382583
Signed-off-by: Francesco Romani
---
M lib/vdsm/virt/periodic.py
M lib/vdsm/virt/vmstats.py
M tests/vmStatsTests.py
M vdsm/virt/vm.py
4 files changed, 14 insertions(+), 7 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/90/65590/1
diff --git a/lib/vdsm/virt/periodic.py b/lib/vdsm/virt/periodic.py
index 132fe93..8a89870 100644
--- a/lib/vdsm/virt/periodic.py
+++ b/lib/vdsm/virt/periodic.py
@@ -290,7 +290,7 @@
def required(self):
# Disable everything until the migration destination VM
# is fully started, to avoid false positives log spam.
-return not self._vm.incomingMigrationPending()
+return self._vm.monitorable
@property
def runnable(self):
diff --git a/lib/vdsm/virt/vmstats.py b/lib/vdsm/virt/vmstats.py
index dadbf26..8e271cb 100644
--- a/lib/vdsm/virt/vmstats.py
+++ b/lib/vdsm/virt/vmstats.py
@@ -495,7 +495,7 @@
try:
yield
except KeyError:
-if vm_obj.incomingMigrationPending():
+if not vm_obj.monitorable:
# If a VM is migration destination,
# libvirt doesn't give any disk stat.
pass
diff --git a/tests/vmStatsTests.py b/tests/vmStatsTests.py
index 9827c4e..6c57696 100644
--- a/tests/vmStatsTests.py
+++ b/tests/vmStatsTests.py
@@ -580,8 +580,9 @@
self.drives = drives if drives is not None else []
self.migrationPending = False
-def incomingMigrationPending(self):
-return self.migrationPending
+@property
+def monitorable(self):
+return not self.migrationPending
def getNicDevices(self):
return self.nics
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index dd633e6..ded832e 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -307,6 +307,13 @@
self._numaInfo = {}
self._vmJobs = None
self._clientPort = ''
+self._monitorable = False
+
+@property
+def monitorable(self):
+if 'migrationDest' in self.conf or 'restoreState' in self.conf:
+return False
+return self._monitorable
@property
def start_time(self):
@@ -585,9 +592,6 @@
if acquired:
self.log.debug('Releasing incoming migration semaphore')
migration.incomingMigrations.release()
-
-def incomingMigrationPending(self):
-return 'migrationDest' in self.conf or 'restoreState' in self.conf
def disableDriveMonitor(self):
self._driveMonitorEnabled = False
@@ -1669,6 +1673,7 @@
raise MissingLibvirtDomainError(vmexitreason.LIBVIRT_START_FAILED)
sampling.stats_cache.add(self.id)
+self._monitorable = True
self._guestEventTime = self._startTime
@@ -3936,6 +3941,7 @@
nic.portMirroring.remove(network)
self.log.info('Release VM resources')
+self._monitorable = False
self.lastStatus = vmstatus.POWERING_DOWN
# Terminate the VM's creation thread.
self._incomingMigrationFinished.set()
--
To view,
