Re: [PATCH v1 4/8] admin: Add device migration admin commands

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Oct 08, 2023 at 02:25:51PM +0300, Parav Pandit wrote:

...

> +\paragraph{Device Context Size Get Command}
> +\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Device Migration / Device Context Size Get Command}
> +
> +This command returns the remaining estimated device context size. The 
> +driver can query the remaining estimated device context size
> +for the current mode or for the \field{Freeze} mode. While
> +reading the device context using VIRTIO_ADMIN_CMD_DEV_CTX_READ command, the
> +actual device context size may differ than what is being returned by
> +this command. After reading the device context using command
> +VIRTIO_ADMIN_CMD_DEV_CTX_READ, the remaining estimated context size
> +usually reduces by amount of device context read by the driver using
> +VIRTIO_ADMIN_CMD_DEV_CTX_READ command. If the device context is updated
> +rapidly the remaining estimated context size may also increase even after
> +reading the device context using VIRTIO_ADMIN_CMD_DEV_CTX_READ command.
> +
> +For the command VIRTIO_ADMIN_CMD_DEV_CTX_SIZE_GET, \field{opcode} is set to 0x9.
> +The \field{group_member_id} refers to the member device to be accessed.
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_dev_ctx_size_get_data {
> +        u8 freeze_mode;
> +};
> +\end{lstlisting}
> +
> +The \field{command_specific_data} is in the format
> +\field{struct virtio_admin_cmd_dev_ctx_size_get_data}.
> +When \field{freeze_mode} is set to 1, the device returns the estimated
> +device context size when the device will be in \field{Freeze} mode.
> +As the device context is read from the device, the remaining estimated
> +context size may decrease. For example, member device mode is
> +\field{Stop}, the device has estimated total device context size
> +as 12KB; the device would return 12KB for the first
> +VIRTIO_ADMIN_CMD_DEV_CTX_SIZE_GET command, once the driver has
> +already read 8KB of device context data using
> +VIRTIO_ADMIN_CMD_DEV_CTX_READ command, and the remaining data is
> +4KB, hence the device returns 4KB in the subsequent
> +VIRTIO_ADMIN_CMD_DEV_CTX_SIZE_GET command.
> +
> +\begin{lstlisting}
> +struct virtio_admin_cmd_dev_ctx_size_get_result {
> +        le64 size;
> +};
> +\end{lstlisting}

So we have a 64 bit size? How are we going to return so much?


> +
> +When the command completes successfully, \field{command_specific_result} is in
> +the format \field{struct virtio_admin_cmd_dev_ctx_size_get_result}.
> +
> +Once the device context is fully read, this command returns zero for
> +\field{size} until the new device context is generated.
> +
> +\paragraph{Device Context Read Command}
> +\label{par:Basic Facilities of a Virtio Device / Device groups / Group administration commands / Device Migration / Device Context Read Command}
> +
> +This command reads the current device context.
> +For the command VIRTIO_ADMIN_CMD_DEV_CTX_READ, \field{opcode} is set to 0xa.
> +The \field{group_member_id} refers to the member device to be accessed.
> +
> +This command has no command specific data.

So I am not sure this is wise. Multi-year experience with QEMU taught us
that we are likely to make mistakes when defining migration format -
forget some fields, validate them incorrectly, and so on.
Making a somewhat safe assumption that we'll make mistakes
in the spec, too, I'd like to see some kind of idea of how
we'll support compatibility and/or graceful failure if/when we do.


> +\begin{lstlisting}
> +struct virtio_admin_cmd_dev_ctx_rd_len {
> +        le32 context_len;
> +};
> +
> +struct virtio_admin_cmd_dev_ctx_rd_result {
> +        u8 data[];
> +};
> +\end{lstlisting}

so callers needs to pin whatever device tells it to?

admin commands support truncation intentionally.

it is not clear, to me, that it's ok to have device just
save as much state as it wants to.

> +
> +When the command completes successfully, \field{command_specific_result}
> +is in the format \field{struct virtio_admin_cmd_dev_ctx_rd_result}
> +returned by the device containing the device context data and
> +\field{command_specific_output} is in format of
> +\field{struct virtio_admin_cmd_dev_ctx_rd_len} containing length of
> +context data returned by the device in the command response. When the length
> +returned is zero or when the returned context data is less the data requested by
> +the driver, the device do not have any device context data left that the device
> +can report, at this point the device context stream ends.
> +
> +The driver can read the whole device context data using one or multiple
> +commands. When the device context does not fit in the
> +\field{command_specific_result}, driver reads the subsequent remaining
> +bytes using one or more subsequent commands.

how?

-- 
MST