Re: [PATCH v2 1/2] virtio: document vdc->get_features() callback

[Cornelia Huck]

On Wed, Aug 03 2022, Stefan Hajnoczi <stefanha@redhat.com> wrote:

> Suggested-by: Cornelia Huck <cohuck@redhat.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  include/hw/virtio/virtio.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index db1c0ddf6b..8d27fe1824 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -120,9 +120,29 @@ struct VirtioDeviceClass {
>      /* This is what a VirtioDevice must implement */
>      DeviceRealize realize;
>      DeviceUnrealize unrealize;
> +
> +    /**
> +     * get_features:
> +     * @vdev: the VirtIODevice
> +     * @requested_features: existing device feature bits from
> +     *                      vdev->host_features
> +     * @errp: pointer to error object
> +     *
> +     * Get the device feature bits.
> +     *
> +     * The ->get_features() function typically sets always-on device feature
> +     * bits as well as conditional feature bits that require some logic to
> +     * compute.
> +     *
> +     * Device feature bits can also be set in vdev->host_features before this
> +     * function is called using DEFINE_PROP_BIT64() qdev properties.
> +     *
> +     * Returns: the final device feature bits to store in 
> vdev->host_features.
> +     */

Not sure if we want to go full function doc for features, as none of the
other callbacks have it...

I thought about something like

"Called with vdev->host_features in requested_features. Returns device
feature bits to be stored in vdev->host_features after factoring in
device-specific feature bits."

The important part IMHO is that requested_features contains
vdev->host_features, so no need to merge them in.

>      uint64_t (*get_features)(VirtIODevice *vdev,
>                               uint64_t requested_features,
>                               Error **errp);
> +
>      uint64_t (*bad_features)(VirtIODevice *vdev);
>      void (*set_features)(VirtIODevice *vdev, uint64_t val);
>      int (*validate_features)(VirtIODevice *vdev);