Skip to content

Commit b25a94e

Browse files
committed
Address review feedback for qube_connection changes
1 parent 868b1f8 commit b25a94e

2 files changed

Lines changed: 30 additions & 23 deletions

File tree

vmupdate/qube_connection.py

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,13 @@
2222
import shutil
2323
import signal
2424
import tempfile
25-
import asyncio
2625
import concurrent.futures
2726
from os.path import join
2827
from subprocess import CalledProcessError
2928
from typing import List
3029

3130
import qubesadmin
32-
from qubesadmin.events.utils import wait_for_domain_shutdown
31+
import qubesadmin.exc
3332
from vmupdate.agent.source.args import AgentArgs
3433
from vmupdate.agent.source.log_config import LOGPATH, LOG_FILE
3534
from vmupdate.agent.source.status import StatusInfo, FinalStatus
@@ -94,10 +93,15 @@ def __exit__(self, exc_type, exc_val, exc_tb):
9493
self.dest_dir, str(err))
9594

9695
if self.qube.is_running() and not self._initially_running:
97-
self.logger.info('Shutdown %s', self.qube.name)
98-
self.qube.shutdown()
9996
if self._has_assigned_pci_devices(self.qube):
100-
asyncio.run(wait_for_domain_shutdown([self.qube]))
97+
self.logger.info(
98+
'Shutdown %s and wait for full shutdown '
99+
'(PCI devices assigned)', self.qube.name)
100+
from vmupdate.vmupdate import shutdown_domains
101+
shutdown_domains([self.qube], self.logger)
102+
else:
103+
self.logger.info('Shutdown %s', self.qube.name)
104+
self.qube.shutdown()
101105

102106
self.__connected = False
103107

@@ -106,7 +110,7 @@ def _has_assigned_pci_devices(vm) -> bool:
106110
"""Return True when VM has assigned PCI devices."""
107111
try:
108112
return any(vm.devices['pci'].get_assigned_devices())
109-
except Exception: # best effort check only
113+
except qubesadmin.exc.QubesDaemonAccessError:
110114
return False
111115

112116
def transfer_agent(self, src_dir: str) -> ProcessResult:

vmupdate/tests/test_qube_connection.py

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,29 @@
22
#
33
# The Qubes OS Project, https://www.qubes-os.org
44
#
5-
# Copyright (C) 2026
5+
# Copyright (C) 2025 Jayant Sankar <jayant@example.com>
66
#
77
# This program is free software; you can redistribute it and/or
88
# modify it under the terms of the GNU General Public License
99
# as published by the Free Software Foundation; either version 2
1010
# of the License, or (at your option) any later version.
1111
#
12+
# This program is distributed in the hope that it will be useful,
13+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
14+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
15+
# GNU General Public License for more details.
16+
#
17+
# You should have received a copy of the GNU General Public License
18+
# along with this program; if not, write to the Free Software
19+
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
20+
# USA.
1221
from unittest.mock import Mock, patch
1322

1423
from vmupdate.qube_connection import QubeConnection
1524

1625

17-
@patch("vmupdate.qube_connection.wait_for_domain_shutdown")
18-
@patch("vmupdate.qube_connection.asyncio.run")
19-
def test_wait_for_shutdown_when_vm_started_by_update(_arun, wait_shutdown):
26+
@patch("vmupdate.vmupdate.shutdown_domains")
27+
def test_wait_for_shutdown_when_vm_started_by_update(shutdown_domains):
2028
vm = Mock()
2129
vm.name = "hvm1"
2230
vm.is_running.side_effect = [False, True]
@@ -30,14 +38,12 @@ def test_wait_for_shutdown_when_vm_started_by_update(_arun, wait_shutdown):
3038
show_progress=False, status_notifier=status_notifier):
3139
pass
3240

33-
vm.shutdown.assert_called_once_with()
34-
wait_shutdown.assert_called_once_with([vm])
35-
_arun.assert_called_once()
41+
shutdown_domains.assert_called_once_with([vm], logger)
42+
vm.shutdown.assert_not_called()
3643

3744

38-
@patch("vmupdate.qube_connection.wait_for_domain_shutdown")
39-
@patch("vmupdate.qube_connection.asyncio.run")
40-
def test_do_not_wait_for_shutdown_without_assigned_pci(_arun, wait_shutdown):
45+
@patch("vmupdate.vmupdate.shutdown_domains")
46+
def test_do_not_wait_for_shutdown_without_assigned_pci(shutdown_domains):
4147
vm = Mock()
4248
vm.name = "hvm2"
4349
vm.is_running.side_effect = [False, True]
@@ -52,13 +58,11 @@ def test_do_not_wait_for_shutdown_without_assigned_pci(_arun, wait_shutdown):
5258
pass
5359

5460
vm.shutdown.assert_called_once_with()
55-
wait_shutdown.assert_not_called()
56-
_arun.assert_not_called()
61+
shutdown_domains.assert_not_called()
5762

5863

59-
@patch("vmupdate.qube_connection.wait_for_domain_shutdown")
60-
@patch("vmupdate.qube_connection.asyncio.run")
61-
def test_do_not_shutdown_if_vm_was_already_running(_arun, wait_shutdown):
64+
@patch("vmupdate.vmupdate.shutdown_domains")
65+
def test_do_not_shutdown_if_vm_was_already_running(shutdown_domains):
6266
vm = Mock()
6367
vm.name = "hvm3"
6468
vm.is_running.return_value = True
@@ -73,5 +77,4 @@ def test_do_not_shutdown_if_vm_was_already_running(_arun, wait_shutdown):
7377
pass
7478

7579
vm.shutdown.assert_not_called()
76-
wait_shutdown.assert_not_called()
77-
_arun.assert_not_called()
80+
shutdown_domains.assert_not_called()

0 commit comments

Comments
 (0)