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 <edef@edef.eu>
This commit is contained in:
Félix Baylac-Jacqué 2020-09-07 12:26:28 +02:00
parent 2ff0bdfb52
commit ecb73fd555
No known key found for this signature in database
GPG Key ID: EFD315F31848DBA4

View File

@ -217,7 +217,7 @@ class Machine:
match = re.search("run-(.+)-vm$", cmd) match = re.search("run-(.+)-vm$", cmd)
if match: if match:
self.name = match.group(1) self.name = match.group(1)
self.logger = args["log"]
self.script = args.get("startCommand", self.create_startcommand(args)) self.script = args.get("startCommand", self.create_startcommand(args))
tmp_dir = os.environ.get("TMPDIR", tempfile.gettempdir()) tmp_dir = os.environ.get("TMPDIR", tempfile.gettempdir())
@ -227,7 +227,10 @@ class Machine:
os.makedirs(path, mode=0o700, exist_ok=True) os.makedirs(path, mode=0o700, exist_ok=True)
return path 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.shared_dir = create_dir("shared-xchg")
self.booted = False self.booted = False
@ -235,7 +238,6 @@ class Machine:
self.pid: Optional[int] = None self.pid: Optional[int] = None
self.socket = None self.socket = None
self.monitor: Optional[socket.socket] = None self.monitor: Optional[socket.socket] = None
self.logger: Logger = args["log"]
self.allow_reboot = args.get("allowReboot", False) self.allow_reboot = args.get("allowReboot", False)
@staticmethod @staticmethod
@ -780,9 +782,10 @@ class Machine:
self.log("QEMU running (pid {})".format(self.pid)) self.log("QEMU running (pid {})".format(self.pid))
def cleanup_statedir(self) -> None: def cleanup_statedir(self) -> None:
self.log("delete the VM state directory") if os.path.isdir(self.state_dir):
if os.path.isfile(self.state_dir):
shutil.rmtree(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: def shutdown(self) -> None:
if not self.booted: if not self.booted:
@ -940,10 +943,10 @@ if __name__ == "__main__":
for nr, vde_socket, _, _ in vde_sockets: for nr, vde_socket, _, _ in vde_sockets:
os.environ["QEMU_VDE_SOCKET_{}".format(nr)] = vde_socket os.environ["QEMU_VDE_SOCKET_{}".format(nr)] = vde_socket
machines = [create_machine({"startCommand": s}) for s in vm_scripts] machines = [
for machine in machines: create_machine({"startCommand": s, "keepVmState": cli_args.keep_vm_state})
if not cli_args.keep_vm_state: for s in vm_scripts
machine.cleanup_statedir() ]
machine_eval = [ machine_eval = [
"{0} = machines[{1}]".format(m.name, idx) for idx, m in enumerate(machines) "{0} = machines[{1}]".format(m.name, idx) for idx, m in enumerate(machines)
] ]