What is the correct way to handle the VirtIO config space in vhost-user?

["Alex Bennée" <alex.bennee@linaro.org>]

[Apologies to CC list for repost due to fat fingering the mailing list address]

Hi Michael,

I was updating my vhost-user-rpmb implementation when I realised it
wasn't handling config space access properly. However when I went to
implement it I realised there seems to be two ways of doing this. For
example in vhost-user-gpu we have:

  static void
  vhost_user_gpu_get_config(VirtIODevice *vdev, uint8_t *config_data)
  {
      VhostUserGPU *g = VHOST_USER_GPU(vdev);
      VirtIOGPUBase *b = VIRTIO_GPU_BASE(vdev);
      struct virtio_gpu_config *vgconfig =
          (struct virtio_gpu_config *)config_data;
      Error *local_err = NULL;
      int ret;

      memset(config_data, 0, sizeof(struct virtio_gpu_config));

      ret = vhost_dev_get_config(&g->vhost->dev,
                                 config_data, sizeof(struct virtio_gpu_config),
                                 &local_err);
      if (ret) {
          error_report_err(local_err);
          return;
      }

      /* those fields are managed by qemu */
      vgconfig->num_scanouts = b->virtio_config.num_scanouts;
      vgconfig->events_read = b->virtio_config.events_read;
      vgconfig->events_clear = b->virtio_config.events_clear;
  }

which is setup with .get_config and .set_config functions that poke the
appropriate vhost communication. However to do this needs an instance
init to create a vhost just so it can jump the g->vhost->dev indirection:

  static void
  vhost_user_gpu_instance_init(Object *obj)
  {
      VhostUserGPU *g = VHOST_USER_GPU(obj);

      g->vhost = VHOST_USER_BACKEND(object_new(TYPE_VHOST_USER_BACKEND));
      object_property_add_alias(obj, "chardev",
                                OBJECT(g->vhost), "chardev");
  }

(aside: this continues my QOM confusion about when things should be in a
class or instance init, up until this point I hadn't needed it in my
stub).

However when grepping around I found some vhost-user devices do it
differently, for example vhost-user-blk has:

  static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
  {
      int ret;
      struct virtio_blk_config blkcfg;
      VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
      Error *local_err = NULL;

      ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
                                 sizeof(struct virtio_blk_config),
                                 &local_err);
      if (ret < 0) {
          error_report_err(local_err);
          return ret;
      }

      /* valid for resize only */
      if (blkcfg.capacity != s->blkcfg.capacity) {
          s->blkcfg.capacity = blkcfg.capacity;
          memcpy(dev->vdev->config, &s->blkcfg, sizeof(struct virtio_blk_config));
          virtio_notify_config(dev->vdev);
      }

      return 0;
  }

Although this seems to miss the ability to "set" a config - although
that seems confusing anyway, surely the guest only ever reads the config
space?

So my question is which approach is the correct one? Is one a legacy
approach or is it "depends on what you are doing"?

Ultimately I guess this points to the need for a bit more API
documentation to make it clear when certain methods should be used.

-- 
Alex Bennée