On 15.02.22 23:19, Eric Blake wrote:
On Tue, Feb 15, 2022 at 02:57:26PM +0100, Hanna Reitz wrote:
Add a parameter to optionally open a QMP connection when creating a
QemuStorageDaemon instance.

Signed-off-by: Hanna Reitz <hre...@redhat.com>
---
  tests/qemu-iotests/iotests.py | 29 ++++++++++++++++++++++++++++-
  1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 6ba65eb1ff..47e3808ab9 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -39,6 +39,7 @@
from qemu.machine import qtest
  from qemu.qmp import QMPMessage
+from qemu.aqmp.legacy import QEMUMonitorProtocol
I thought we were trying to get rid of aqmp.legacy usage, so this
feels like a temporary regression.  Oh well, not the end of the
testing world.

I fiddled around with the non-legacy interface and wasn’t very successful...  I thought since machine.py still uses qemu.aqmp.legacy for QEMUMachine, when one is reworked to get rid of it (if that ever becomes necessary), then we can just do it here, too.


      def stop(self, kill_signal=15):
          self._p.send_signal(kill_signal)
          self._p.wait()
          self._p = None
+ if self._qmp:
+            self._qmp.close()
+
          try:
+            if self._qmpsock is not None:
+                os.remove(self._qmpsock)
              os.remove(self.pidfile)
          except OSError:
              pass
Do we need two try: blocks here, to remove self.pidfile even if
os.remove(self._qmpsock) failed?

Honestly, no reason not to use two blocks except it being longer. You’re right, I should’ve just done that.

Otherwise, makes sense to me.

Thanks for reviewing!

Hanna


Reply via email to