Re: [PATCH 4/4] docs: add an introduction to the system docs

[Kashyap Chamarthy <kchamart@redhat.com>]

On Fri, Jan 13, 2023 at 01:39:23PM +0000, Alex Bennée wrote:
> Drop the frankly misleading quickstart section for a more rounded
> introduction section. This new section gives an overview of the
> accelerators and high level introduction to some of the key features
> of the emulator. We also expand on a general form for a QEMU command
> line with a hopefully not too scary worked example of what this looks
> like.

Thank you for this improvement!

The rendered version you shared on IRC looks quite good already.

https://qemu-stsquad.readthedocs.io/en/docs-next/system/introduction.html

The only main comment I have is to use -blockdev (instead of '-drive')
for the examples of overriding default firmware further below.  Two
reasons: consistency and IIUC, '-drive' is slated for deprecation in the
future.

Besides that, just a couple of small nits below, feel free to pick and
choose what you prefer :-)

> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---

[...]

> +Introduction
> +============
> +
> +Virtualisation Accelerators
> +---------------------------
> +
> +QEMU's system emulation provides a virtual model of a machine (CPU,
> +memory and emulated devices) to run a guest OS. It supports a number
> +of hypervisors (known as accelerators) as well as a dynamic JIT known
> +as the Tiny Code Generator (TCG) capable of emulating many CPUs.

Nit: might want to expand JIT as well (although if someone is reading
this page, they'd already know what it is).  So up to you :-)

[...]

> +There is a full featured block layer allows for construction of

Nit: s/allows/that allows/

> +complex storage topology which can be stacked across multiple layers
> +supporting redirection, networking, snapshots and migration support.

Speaking of which, consider hyper-linking this doc:
https://qemu.readthedocs.io/en/latest/interop/live-block-operations.html

[...]

> +For the common accelerators QEMU supported debugging with its
> +:ref:`gdbstub<GDB usage>` which allows users to connect GDB and debug
> +system software images

Readability tweak for the first part of the sentence:

    For the common accelerators, QEMU supports debugging with its [...]

> +Running
> +-------

[...]

> +While the project doesn't want to discourage users from using the
> +command line to launch VMs we do want to highlight there are a number

Nit: small tweak:

    [...] to launch VMs, we do want to highlight that there are [...]


[...]

> +We then tell QEMU to multiplex the :ref:`QEMU monitor` with the serial
> +port output (we can switch between the two using :ref:`keys in the
> +character backend multiplexer`). As there is no default graphical
> +device we disable the display as we can work entirely in the terminal.
> +
> +.. code::
> +
> + -serial mon:stdio \
> + -display none \

Nice that you mention "-serial mon:stdio" which doesn't terminate QEMU
on Ctrl-c (while "-serial stdio" does :-)).

> +
> +Finally we override the default firmware to ensure we have have some
> +storage for EFI to persist its configuration. That firmware is
> +responsible for finding the disk, booting grub and eventually running
> +our system.
> +
> +.. code::
> +
> + -drive if=pflash,file=(pwd)/pc-bios/edk2-aarch64-code.fd,format=raw,readonly=on \
> + -drive if=pflash,file=$HOME/images/qemu-arm64-efivars,format=raw

Here, -blockdev.  (I don't have a replacement invocation off-hand,
afraid.)

Regardless of whether these are addressed, this is a strict improvement:

Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com>    


[...]

-- 
/kashyap