Merge pull request #9656 from pmores/document-qemu-rs-conventions

runtime-rs: document architecture & implementation conventions in qem…

src/runtime-rs/crates/hypervisor/src/qemu/cmdline_generator.rs

@@ -70,6 +70,81 @@ impl Display for VirtioBusType {
     }
 }
 
+// Conventions used in qemu command line generation
+// ================================================
+//
+// While acknowledging govmm inspiration, this implementation differs in
+// several important aspects.
+//
+// 1:1 correspondence between qemu switches and cmdline generator structs
+// ----------------------------------------------------------------------
+//
+// There should mostly be a 1:1 mapping between qemu command line switches and
+// objects (structs) that represent them.  That is, each struct's
+// ToQemuParams::qemu_params() is expected to generate a single
+// `-whatever <params...>`.  Notably, a lot of devices are assembled from a
+// frontend (`-device`) and a backend (`-object`), each of which should be
+// represented and generated by a separate object (struct).  If setting up
+// a qemu construct (device or other) requires assembly from multiple such
+// parts, this is expected to be done in a `QemuCmdLine::add_*()` function.
+//
+// As an example, a network device might consist of `-netdev tap` and
+// `-device virtio-net-pci`.  The former is represented by struct Netdev,
+// the latter by struct DeviceVirtioNet, both of which are instantiated &
+// set up in QemuCmdLine::add_network_device().
+//
+// There is a couple of exceptions to this convention.  struct Kernel might
+// be required to generate multiple qemu cmdline switches to perform full
+// kernel set-up, struct Knobs (directly lifted from govmm) does that too
+// due to its purpose of being a catch-all grabbag.  Both are singletons
+// dictated mostly by nature of qemu's cmdline syntax.  Going forward, most
+// if not all future additions to qemu cmdline generation are expected to
+// follow this convention.
+//
+// This convention is a departure from govmm style which tends to lump all
+// information necessary to set up a piece of VM equipment into a single
+// object.  This might make sense at the first sight, after all a network
+// device needs both its frontend and backend so why not represent it with
+// a single struct.  However, as can be observed in govmm, the resulting
+// structs tend to be big and hard to read since the frontend and backend
+// params are distinct so the big struct is actually two separate structs
+// really.  If a need ever arises to have explicit representation of a whole
+// device it would still seem preferable to assemble it from the explicit
+// individual representations of frontend and backend.
+//
+// Naming conventions
+// ------------------
+//
+// The idea is to name entities in this qemu cmdline generator as close
+// as possible to the qemu cmdline switches and params that they represent.
+// This is to reduce the intellectual overhead of having to map between names
+// on qemu command line and in this source code.
+//
+// Most of the cmdline switch generating structs in this file are expected
+// to generate a single qemu cmdline switch as detailed above.  It follows
+// that most member fields of these objects will represent additional params
+// of the qemu switches and their names should ideally be as close to the
+// corresponding qemu switch params as possible, within reason and Rust
+// syntax limitations (e.g. dashes in qemu param names obviously need to be
+// converted to underscores for code to even compile).  As an example,
+// struct MemoryBackendFile members' names are identical to qemu's
+// `-object memory-backend-file,...` params, with the exception of `mem-path`
+// which is called `mem_path` to comply with Rust syntax.
+//
+// The struct names should follow the same logic, their names should ideally
+// be as close as possible to the qemu switch they represent (e.g. `-smp` is
+// generated by struct Smp), or to common parlance if that's not descriptive
+// enough.  This applies above all to devices which are commonly set up by
+// `-device` & `-object`.  In those cases, e.g. `-chardev socket` might be
+// referred to as "chardev socket" in common speech so the corresponding struct
+// is called ChardevSocket, or `-device vhost-user-fs-pci` will likely be
+// pronounced as "device vhost user fs" so its struct is called
+// DeviceVhostUserFs, too.  Admittedly, this approach can be rather squishy and
+// a matter of taste ultimately, but it should still be useful to keep in mind
+// that the idea is to keep the names in this source code as close as possible
+// to what folks working with qemu in kata are likely to use and be familiar
+// with already.
+
 #[derive(Debug)]
 struct Kernel {
     // PathBuf would seem more appropriae but since we get the kernel path
@@ -1311,6 +1386,15 @@ pub struct QemuCmdLine<'a> {
     id: String,
     config: &'a HypervisorConfig,
 
+    // In principle, all objects implementing ToQemuParams could be just stored
+    // in the `devices` container.  However, there are several special cases
+    // that might need to be set up in several steps after having been
+    // initially constructed (from HypervisorConfig, mostly).  For instance,
+    // adding an NVDIMM needs to modify Machine and query Memory, adding a
+    // virtiofs share modifies both Memory and Machine, adding a serial console
+    // modifies Kernel etc.  For convenience accessing them, we store these
+    // singletons in named QemuCmdLine member.  The rest should go to the
+    // anonymous `devices` container.
     kernel: Kernel,
     memory: Memory,
     smp: Smp,