From ecb73fd5557d6d438aa7c155e5b1aad89373c6ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Baylac-Jacqu=C3=A9?= Date: Mon, 7 Sep 2020 12:26:28 +0200 Subject: [PATCH] test-driver.py: fix VM state directory deletion The previous version of the code would only kick in if the state directory path pointed at a *file*, which never occurs. Making that codepath actually work reveals an ordering bug, which this patch fixes as well. It also replaces the confusing, imperative case log message "delete VM state directory" with "deleting VM state directory". Finally, we hint the user about how to prevent this deletion. IE. by passing the --keep-vm-state flag. Bug report: https://github.com/NixOS/nixpkgs/pull/91046#issuecomment-685568750 Credit goes to Edef for the rebase on top of a recent nixpkgs commit and for writing most of this commit message. Co-authored-by: edef --- nixos/lib/test-driver/test-driver.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/nixos/lib/test-driver/test-driver.py b/nixos/lib/test-driver/test-driver.py index 93f94587c0a5..5d3e632fac2c 100644 --- a/nixos/lib/test-driver/test-driver.py +++ b/nixos/lib/test-driver/test-driver.py @@ -217,7 +217,7 @@ class Machine: match = re.search("run-(.+)-vm$", cmd) if match: self.name = match.group(1) - + self.logger = args["log"] self.script = args.get("startCommand", self.create_startcommand(args)) tmp_dir = os.environ.get("TMPDIR", tempfile.gettempdir()) @@ -227,7 +227,10 @@ class Machine: os.makedirs(path, mode=0o700, exist_ok=True) return path - self.state_dir = create_dir("vm-state-{}".format(self.name)) + self.state_dir = os.path.join(tmp_dir, f"vm-state-{self.name}") + if not args["keepVmState"]: + self.cleanup_statedir() + os.makedirs(self.state_dir, mode=0o700, exist_ok=True) self.shared_dir = create_dir("shared-xchg") self.booted = False @@ -235,7 +238,6 @@ class Machine: self.pid: Optional[int] = None self.socket = None self.monitor: Optional[socket.socket] = None - self.logger: Logger = args["log"] self.allow_reboot = args.get("allowReboot", False) @staticmethod @@ -780,9 +782,10 @@ class Machine: self.log("QEMU running (pid {})".format(self.pid)) def cleanup_statedir(self) -> None: - self.log("delete the VM state directory") - if os.path.isfile(self.state_dir): + if os.path.isdir(self.state_dir): shutil.rmtree(self.state_dir) + self.logger.log(f"deleting VM state directory {self.state_dir}") + self.logger.log("if you want to keep the VM state, pass --keep-vm-state") def shutdown(self) -> None: if not self.booted: @@ -940,10 +943,10 @@ if __name__ == "__main__": for nr, vde_socket, _, _ in vde_sockets: os.environ["QEMU_VDE_SOCKET_{}".format(nr)] = vde_socket - machines = [create_machine({"startCommand": s}) for s in vm_scripts] - for machine in machines: - if not cli_args.keep_vm_state: - machine.cleanup_statedir() + machines = [ + create_machine({"startCommand": s, "keepVmState": cli_args.keep_vm_state}) + for s in vm_scripts + ] machine_eval = [ "{0} = machines[{1}]".format(m.name, idx) for idx, m in enumerate(machines) ]