Francesco Romani has uploaded a new change for review.
Change subject: WIP virt: add and use vm.acpiReboot() method ......................................................................
WIP virt: add and use vm.acpiReboot() method
WRITEME
Change-Id: I5d6dd3a81cd4ad981a839849a4a2914e35a53fa6 Related-To: https://bugzilla.redhat.com/1154389 Signed-off-by: Francesco Romani fromani@redhat.com --- M vdsm/virt/vm.py M vdsm/virt/vmpowerdown.py 2 files changed, 14 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/23/44923/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 8fc185f..c9f23d2 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3811,6 +3811,18 @@ else: return response.success()
+ def acpiReboot(self): + try: + self._dom.reboot(libvirt.VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) + except NotConnectedError: + # the VM was already shut off asynchronously, + # so ignore error and quickly exit + self.log.warning('failed to invoke acpiReboot: ' + 'domain not connected') + return response.error('down') + else: + return response.success() + def setBalloonTarget(self, target):
if not self._dom.connected: diff --git a/vdsm/virt/vmpowerdown.py b/vdsm/virt/vmpowerdown.py index ffa10c9..0d46b71 100644 --- a/vdsm/virt/vmpowerdown.py +++ b/vdsm/virt/vmpowerdown.py @@ -106,8 +106,8 @@ return self.event.wait(self.delay + self.timeout)
def acpiCallback(self): - # TODO: fix like acpiShutdown - self.vm._dom.reboot(libvirt.VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) + if response.is_error(self.vm.acpiReboot()): + return False return self.event.wait(self.timeout)
def forceCallback(self):
automation@ovirt.org has posted comments on this change.
Change subject: WIP virt: add and use vm.acpiReboot() method ......................................................................
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.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: WIP virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 2:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 3:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 3: Verified+1
verified with 34879, the same way: sent qith vdsClient reboot command ASAP after boot, intermixed with kills from kill command.
Sometimes the call blown up with
Thread-525::ERROR::2015-08-19 10:44:35,086::utils::1056::utils.Callback::(__call__) acpiCallback failed Traceback (most recent call last): File "/usr/lib/python2.7/site-packages/vdsm/utils.py", line 1054, in __call__ result = self.func(*self.args, **self.kwargs) File "/usr/share/vdsm/virt/vmpowerdown.py", line 109, in acpiCallback if response.is_error(self.vm.acpiReboot()): File "/usr/share/vdsm/virt/vm.py", line 3818, in acpiReboot self._dom.reboot(libvirt.VIR_DOMAIN_REBOOT_ACPI_POWER_BTN) File "/usr/share/vdsm/virt/vm.py", line 231, in f ret = attr(*args, **kwargs) File "/usr/lib/python2.7/site-packages/vdsm/libvirtconnection.py", line 124, in wrapper ret = f(*args, **kwargs) File "/usr/lib64/python2.7/site-packages/libvirt.py", line 1897, in reboot if ret == -1: raise libvirtError ('virDomainReboot() failed', dom=self) libvirtError: Domain not found: no domain with matching uuid '42e30017-b29c-4646-aa75-e42c4d0fe659' (ftest1)
but this is related but different issue, which will addressed in a separate patch. This one seems to work as expected.
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 4:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 5:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 6:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
Dan Kenigsberg has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 6: Code-Review-1
(3 comments)
only one nit, and English fixes.
https://gerrit.ovirt.org/#/c/44923/6//COMMIT_MSG Commit Message:
Line 13: access accesses
Line 14: in impolite
https://gerrit.ovirt.org/#/c/44923/6/tests/vmfakelib.py File tests/vmfakelib.py:
Line 264: @recorded unused
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 7:
* 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.5', 'ovirt-3.4', 'ovirt-3.3'])
Francesco Romani has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 7: Verified+1
the changes since last verification are covered by tests. V+1 again.
Dan Kenigsberg has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 7: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
virt: add and use vm.acpiReboot() method
The VM powerdown flow, implemented into the vmpowerdown.py module, needs to call libvirt APIs to trigger the actual shutdown or reboot operations.
To do so, it directly accesses the Vm._dom private attribute, which is impolite and evil.
This patch adds acpiReboot public method, with built-in protection against asynchronous QEMU death. This of course adds more code to the already fat vm.py, but improves isolation among modules, which is a greater good.
Change-Id: I5d6dd3a81cd4ad981a839849a4a2914e35a53fa6 Related-To: https://bugzilla.redhat.com/1154389 Signed-off-by: Francesco Romani fromani@redhat.com Reviewed-on: https://gerrit.ovirt.org/44923 Continuous-Integration: Jenkins CI Reviewed-by: Dan Kenigsberg danken@redhat.com --- M tests/vmTests.py M tests/vmfakelib.py M vdsm/virt/vm.py M vdsm/virt/vmpowerdown.py 4 files changed, 27 insertions(+), 3 deletions(-)
Approvals: Jenkins CI: Passed CI tests Dan Kenigsberg: Looks good to me, approved Francesco Romani: Verified
automation@ovirt.org has posted comments on this change.
Change subject: virt: add and use vm.acpiReboot() method ......................................................................
Patch Set 8:
* Update tracker::IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found.
vdsm-patches@lists.fedorahosted.org