Use existing QEMU session for copy-in/copy-out

docs/cli/run_create_start_stop.md

@@ -98,6 +98,7 @@ ones common to both subcommands are described below:
                                      (/absolute/path/on/VM:path/on/host)
 - `--copy-timeout COPY_TIMEOUT`: The maximum time to wait for a copy-in-before or
                                  copy-out-after operation to complete
+- `--direct-copy`:    Transfier files specified via `--copy-in-before` or `--copy-in-after` directly to the VM, instead of a background 'builder' VM
 - `--rsync`: Use rsync for copy-in-before/copy-out-after operations
 
 #### VM Creation Flags
@@ -150,7 +151,7 @@ described below:
 ### Create and Start an Alpine VM
 
 ```
-$ transient create --name bar --ssh alpine_rel3,http=https://github.com/ALSchwalm/transient-baseimages/releases/download/5/alpine-3.13.qcow2.xz
+$ transient create --name bar --ssh alpine_rel3,http=https://github.com/ALSchwalm/transient-baseimages/releases/download/6/alpine-3.13.qcow2.xz
 Unable to find image 'alpine' in backend
 Downloading image from 'https://github.com/ALSchwalm/transient-baseimages/releases/download/5/alpine-3.13.qcow2.xz'
 100% |##############################################|  12.7 MiB/s |  35.0 MiB | Time:  0:00:02

test/features/copy-in-copy-out.feature

@@ -5,58 +5,88 @@ Feature: Copy-in and Copy-out Support
   - copy the host file or directory to the guest directory before starting the VM
   - copy the guest file or directory to the host directory after stopping the VM
 
-  Scenario: Copy in a file before starting VM
+  Scenario Outline: Copy in a file before starting VM
     Given a transient run command
       And an http alpine disk image
       And a test file: "artifacts/copy-in-before-test-file"
       And a guest directory: "/home/vagrant/"
       And the test file is copied to the guest directory before starting
+      And using the "<image_type>" image for copying
       And a ssh command "ls /home/vagrant"
      When the vm runs to completion
      Then the return code is 0
       And stdout contains "copy-in-before-test-file"
 
-  Scenario: Copy in a large file before starting VM
+    Examples: Image Types
+      | image_type |
+      | dedicated  |
+      | same       |
+
+  Scenario Outline: Copy in a large file before starting VM
     Given a transient run command
       And an http alpine disk image
       And a large test file: "artifacts/copy-in-large-test-file"
       And a guest directory: "/home/vagrant/"
       And the test file is copied to the guest directory before starting
+      And using the "<image_type>" image for copying
       And a ssh command "ls /home/vagrant"
      When the vm runs to completion
      Then the return code is 0
       And stdout contains "copy-in-large-test-file"
 
-  Scenario: Copy out a file after stopping VM
+    Examples: Image Types
+      | image_type |
+      | dedicated  |
+      | same       |
+
+  Scenario Outline: Copy out a file after stopping VM
     Given a transient run command
       And an http alpine disk image
       And a host directory: "artifacts/"
       And a guest test file: "/home/vagrant/copy-out-after-test-file"
       And the guest test file is copied to the host directory after stopping
+      And using the "<image_type>" image for copying
       And a ssh command "touch /home/vagrant/copy-out-after-test-file"
      When the vm runs to completion
      Then the return code is 0
       And the file "artifacts/copy-out-after-test-file" exists
 
-  Scenario: Copy out a file after stopping VM using rsync
+    Examples: Image Types
+      | image_type |
+      | dedicated  |
+      | same       |
+
+  Scenario Outline: Copy out a file after stopping VM using rsync
     Given a transient run command
       And an http alpine disk image
       And a host directory: "artifacts/"
       And a guest test file: "/home/vagrant/copy-out-after-test-file"
       And the guest test file is copied to the host directory after stopping
+      And using the "<image_type>" image for copying
       And a ssh command "touch /home/vagrant/copy-out-after-test-file"
       And an extra argument "--rsync"
      When the vm runs to completion
      Then the return code is 0
       And the file "artifacts/copy-out-after-test-file" exists
 
-  Scenario: Copy in a symbolic link using rsync
+    Examples: Image Types
+      | image_type |
+      | dedicated  |
+      | same       |
+
+  Scenario Outline: Copy in a symbolic link using rsync
     Given a transient run command
       And an http alpine disk image
       And a symbolic link "artifacts/symlink" to "/etc/hostname"
       And a guest directory: "/home/vagrant/"
       And the test file is copied to the guest directory before starting
+      And using the "<image_type>" image for copying
       And a ssh command "test -L /home/vagrant/symlink"
       And an extra argument "--rsync"
      When the vm runs to completion
      Then the return code is 0
+
+    Examples: Image Types
+      | image_type |
+      | dedicated  |
+      | same       |

test/features/steps/steps.py

@@ -225,7 +225,7 @@ def step_impl(context, image):
 def step_impl(context):
     context.vm_config[
         "transient-image"
-    ] = "alpine_rel3,http=https://github.com/ALSchwalm/transient-baseimages/releases/download/5/alpine-3.13.qcow2.xz"
+    ] = "alpine_rel3,http=https://github.com/ALSchwalm/transient-baseimages/releases/download/6/alpine-3.13.qcow2.xz"
 
 
 @given("an http centos disk image")
@@ -336,6 +336,13 @@ def step_impl(context):
     context.vm_config["transient-args"].extend(["--copy-out-after", directory_mapping])
 
 
+@given('using the "{image_type}" image for copying')
+def step_impl(context, image_type):
+    assert image_type in ("same", "dedicated"), repr(image_type)
+    if image_type == "same":
+        context.vm_config["transient-args"].extend(["--direct-copy"])
+
+
 @given('a qemu flag "{flag}"')
 def step_impl(context, flag):
     context.vm_config["qemu-args"].append(flag)

transient/args.py

@@ -174,6 +174,12 @@ def set_default(value: Any) -> Any:
         type=int,
         help="The maximum time to wait for a copy-in-before or copy-out-after operation to complete",
     )
+    common_oneshot_parser.add_argument(
+        "--direct-copy",
+        action="store_const",
+        const=True,
+        help="Transfier files specified via --copy-in-before or --copy-in-after directly to the VM, instead of a background 'builder' VM",
+    )
     common_oneshot_parser.add_argument(
         "--rsync",
         action="store_const",

transient/configuration.py

@@ -276,3 +276,10 @@ def config_requires_ssh_console(config: Union[RunConfig, CreateConfig]) -> bool:
         or config.ssh_command is not None
         or config.ssh_with_serial is True
     )
+
+
+def config_wants_rsync_transfer(config: Union[RunConfig, BuildConfig]) -> bool:
+    if config.rsync is not None:
+        assert isinstance(config.rsync, bool)
+        return config.rsync is True
+    return False

transient/editor.py

@@ -283,16 +283,21 @@ def run_command_in_guest(
             return None, None
 
     def copy_in(self, host_path: str, guest_path: str) -> None:
-        transfer = ssh.rsync if self.rsync is True else ssh.scp
-        transfer(
-            host_path, utils.join_absolute_paths("/mnt", guest_path), self.ssh_config
+        use_rsync = self.rsync is True
+        ssh.transfer(
+            host_path,
+            utils.join_absolute_paths("/mnt", guest_path),
+            ssh_config=self.ssh_config,
+            copy_from=False,
+            use_rsync=use_rsync,
         )
 
     def copy_out(self, guest_path: str, host_path: str) -> None:
-        transfer = ssh.rsync if self.rsync is True else ssh.scp
-        transfer(
+        use_rsync = self.rsync is True
+        ssh.transfer(
             utils.join_absolute_paths("/mnt", guest_path),
             host_path,
-            self.ssh_config,
+            ssh_config=self.ssh_config,
             copy_from=True,
+            use_rsync=use_rsync,
         )

transient/ssh.py

@@ -298,3 +298,19 @@ def rsync(
             capture_stdout=capture_stdout,
             capture_stderr=capture_stderr,
         )
+
+
+def transfer(
+    host_path: str,
+    guest_path: str,
+    ssh_config: SshConfig,
+    copy_from: bool,
+    use_rsync: bool,
+) -> None:
+    func = rsync if use_rsync is True else scp
+    logging.debug(
+        "Transfer host_path={} guest_path={} copy_from={} func={}".format(
+            host_path, guest_path, copy_from, func
+        )
+    )
+    func(host_path, guest_path, ssh_config, copy_from)

transient/transient.py

@@ -56,6 +56,7 @@ class TransientVm:
     set_ssh_port: Optional[int]
     vmstate: Optional[store.VmPersistentState]
     state: TransientVmState
+    copy_out_done: bool
 
     def __init__(self, config: configuration.RunConfig, vmstore: store.VmStore) -> None:
         self.config = config
@@ -69,6 +70,7 @@ def __init__(self, config: configuration.RunConfig, vmstore: store.VmStore) -> N
         self.data_tempfile = tempfile.TemporaryFile("wb+", buffering=0)
         self.set_ssh_port = None
         self.vmstate = None
+        self.copy_out_done = False
 
     def __use_backend_images(self, names: List[str]) -> List[store.BackendImageInfo]:
         """Ensure the backend images are download for each image spec in 'names'"""
@@ -88,6 +90,15 @@ def __needs_to_copy_in_files_before_running(self) -> bool:
         """
         return len(self.config.copy_in_before) > 0
 
+    def __qemu_is_running(self) -> bool:
+        return (self.qemu_runner and self.qemu_runner.is_running()) or False
+
+    def transfer(self, host_path: str, guest_path: str, copy_from: bool) -> None:
+        """ Perform rsync/scp transfer, assumes guest is up """
+        use_rsync = configuration.config_wants_rsync_transfer(self.config)
+        assert self.ssh_config is not None
+        ssh.transfer(host_path, guest_path, self.ssh_config, copy_from, use_rsync)
+
     def __copy_in_files(self) -> None:
         """Copies the given files or directories (located on the host) into the VM"""
         path_mappings = self.config.copy_in_before
@@ -110,19 +121,25 @@ def __copy_in(self, path_mapping: str) -> None:
         if not vm_absolute_path.startswith("/"):
             raise RuntimeError(f"Absolute path for guest required: {vm_absolute_path}")
 
-        assert isinstance(self.primary_image, store.FrontendImageInfo)
-        assert self.primary_image.backend is not None
-        logging.info(
-            f"Copying from '{host_path}' to '{self.primary_image.backend.identifier}:{vm_absolute_path}'"
-        )
+        if not self.__qemu_is_running():
+            assert isinstance(self.primary_image, store.FrontendImageInfo)
+            assert self.primary_image.backend is not None
+            logging.info(
+                f"Copying from '{host_path}' to '{self.primary_image.backend.identifier}:{vm_absolute_path}'"
+            )
 
-        with editor.ImageEditor(
-            self.primary_image.path,
-            self.config.ssh_timeout,
-            self.config.qmp_timeout,
-            self.config.rsync,
-        ) as edit:
-            edit.copy_in(host_path, vm_absolute_path)
+            with editor.ImageEditor(
+                self.primary_image.path,
+                self.config.ssh_timeout,
+                self.config.qmp_timeout,
+                self.config.rsync,
+            ) as edit:
+                edit.copy_in(host_path, vm_absolute_path)
+        else:
+            logging.info(
+                f"Copying from '{host_path}' to '(EXISTING QEMU):{vm_absolute_path}'"
+            )
+            self.transfer(host_path, vm_absolute_path, copy_from=False)
 
     def __needs_to_copy_out_files_after_running(self) -> bool:
         """Checks if at least one directory on the VM needs to be copied out
@@ -152,19 +169,25 @@ def __copy_out(self, path_mapping: str) -> None:
         if not vm_absolute_path.startswith("/"):
             raise RuntimeError(f"Absolute path for guest required: {vm_absolute_path}")
 
-        assert isinstance(self.primary_image, store.FrontendImageInfo)
-        assert self.primary_image.backend is not None
-        logging.info(
-            f"Copying from '{self.primary_image.backend.identifier}:{vm_absolute_path}' to '{host_path}'"
-        )
+        if not self.__qemu_is_running():
+            assert isinstance(self.primary_image, store.FrontendImageInfo)
+            assert self.primary_image.backend is not None
+            logging.info(
+                f"Copying from '{self.primary_image.backend.identifier}:{vm_absolute_path}' to '{host_path}'"
+            )
 
-        with editor.ImageEditor(
-            self.primary_image.path,
-            self.config.ssh_timeout,
-            self.config.qmp_timeout,
-            self.config.rsync,
-        ) as edit:
-            edit.copy_out(vm_absolute_path, host_path)
+            with editor.ImageEditor(
+                self.primary_image.path,
+                self.config.ssh_timeout,
+                self.config.qmp_timeout,
+                self.config.rsync,
+            ) as edit:
+                edit.copy_out(vm_absolute_path, host_path)
+        else:
+            logging.info(
+                f"Copying from '(EXISTING QEMU):{vm_absolute_path}' to '{host_path}'"
+            )
+            self.transfer(vm_absolute_path, host_path, copy_from=True)
 
     def __qemu_added_args(self) -> List[str]:
         new_args = ["-name", self.name]
@@ -226,6 +249,12 @@ def __prepare_ssh(self) -> None:
             extra_options=self.config.ssh_option,
         )
 
+    def __ensure_ssh(self) -> None:
+        assert self.ssh_config is not None
+        client = ssh.SshClient(config=self.ssh_config, command="exit 0")
+        conn = client.connect_stdout(timeout=self.config.ssh_timeout)
+        conn.wait()
+
     def __connect_ssh(self) -> int:
         assert self.ssh_config is not None
         client = ssh.SshClient(config=self.ssh_config, command=self.config.ssh_command)
@@ -281,8 +310,9 @@ def __qemu_sigchld_handler(self, sig: int, _frame: Any) -> None:
     def __post_run(self, returncode: int) -> None:
         self.state = TransientVmState.FINISHED
 
-        if self.__needs_to_copy_out_files_after_running():
+        if self.__needs_to_copy_out_files_after_running() and not self.copy_out_done:
             self.__copy_out_files()
+            self.copy_out_done = True
 
         # If the config name is None, this is a temporary VM,
         # so remove any generated frontend images. However, if the
@@ -342,6 +372,13 @@ def run(self) -> None:
 
     def __do_run(self) -> None:
         self.state = TransientVmState.RUNNING
+        self.copy_out_done = False
+
+        # direct copy-in can only be done with SSH console (for the "before" part and only when requested with --direct-copy)
+        will_direct_copy_in = (
+            configuration.config_requires_ssh_console(self.config)
+            and self.config.direct_copy
+        )
 
         if not self.__is_stateless():
             assert self.vmstate is not None
@@ -357,7 +394,7 @@ def __do_run(self) -> None:
                 self.config.extra_image
             )
 
-        if self.__needs_to_copy_in_files_before_running():
+        if self.__needs_to_copy_in_files_before_running() and not will_direct_copy_in:
             self.__copy_in_files()
 
         print("Finished preparation. Starting virtual machine")
@@ -423,6 +460,10 @@ def __do_run(self) -> None:
         self.__prepare_proc_data()
 
         if configuration.config_requires_ssh_console(self.config):
+            if self.__needs_to_copy_in_files_before_running() and will_direct_copy_in:
+                self.__ensure_ssh()
+                self.__copy_in_files()
+
             # Note that we always return the SSH exit code, even if the guest failed to
             # shut down. This ensures the shutdown_timeout=0 case is handled as expected.
             # (i.e., it returns the SSH code instead of a QEMU error)
@@ -433,6 +474,17 @@ def __do_run(self) -> None:
             # SIGCHLD exit.
             self.qemu_should_die = True
 
+            if self.__needs_to_copy_out_files_after_running() and self.config.direct_copy:
+                # If the VM was shutdown or is otherwise inaccessible,
+                # the copy-out operation will also be attempted in __post_run.
+                try:
+                    self.__copy_out_files()
+                    self.copy_out_done = True
+                except utils.TransientProcessError as e:
+                    logging.error(
+                        "copy_out during existing QEMU session failed: {}".format(e)
+                    )
+
             try:
                 # Wait a bit for the guest to finish the shutdown and QEMU to exit
                 self.qemu_runner.shutdown(timeout=self.config.shutdown_timeout)