qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] vhost-user-fs: add capability to allow migration


From: Anton Kuchin
Subject: Re: [PATCH] vhost-user-fs: add capability to allow migration
Date: Fri, 10 Feb 2023 16:09:23 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1

On 02/02/2023 11:59, Juan Quintela wrote:
Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
On 01/02/2023 16:26, Juan Quintela wrote:
Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
On 19/01/2023 18:02, Stefan Hajnoczi wrote:
On Thu, 19 Jan 2023 at 10:29, Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
On 19/01/2023 16:30, Stefan Hajnoczi wrote:
On Thu, 19 Jan 2023 at 07:43, Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
On 18/01/2023 17:52, Stefan Hajnoczi wrote:
On Sun, 15 Jan 2023 at 12:21, Anton Kuchin <antonkuchin@yandex-team.ru> wrote:
Once told that, I think that you are making your live harder in the
future when you add the other migratable devices.

I am assuming here that your "underlying device" is:

enum VhostUserFSType {
      VHOST_USER_NO_MIGRATABLE = 0,
      // The one we are doing here
      VHOST_USER_EXTERNAL_MIGRATABLE,
      // The one you describe for the future
      VHOST_USER_INTERNAL_MIGRATABLE,
};

struct VHostUserFS {
      /*< private >*/
      VirtIODevice parent;
      VHostUserFSConf conf;
      struct vhost_virtqueue *vhost_vqs;
      struct vhost_dev vhost_dev;
      VhostUserState vhost_user;
      VirtQueue **req_vqs;
      VirtQueue *hiprio_vq;
      int32_t bootindex;
      enum migration_type;
      /*< public >*/
};


static int vhost_user_fs_pre_save(void *opaque)
{
      VHostUserFS *s = opaque;

      if (s->migration_type == VHOST_USER_NO_MIGRATABLE)) {
          error_report("Migration of vhost-user-fs devices requires internal FUSE 
"
                       "state of backend to be preserved. If orchestrator can "
                       "guarantee this (e.g. dst connects to the same backend "
                       "instance or backend state is migrated) set 'vhost-user-fs' 
"
                       "migration capability to true to enable migration.");
          return -1;
      }
      if (s->migration_type == VHOST_USER_EXTERNAL_MIGRATABLE) {
          return 0;
      }
      if (s->migration_type == VHOST_USER_INTERNAL_MIGRATABLE) {
          error_report("still not implemented");
          return -1;
      }
      assert("we don't reach here");
}

Your initial vmstateDescription

static const VMStateDescription vuf_vmstate = {
      .name = "vhost-user-fs",
      .unmigratable = 1,
      .minimum_version_id = 0,
      .version_id = 0,
      .fields = (VMStateField[]) {
          VMSTATE_INT8(migration_type, struct VHostUserFS),
          VMSTATE_VIRTIO_DEVICE,
          VMSTATE_END_OF_LIST()
      },
      .pre_save = vhost_user_fs_pre_save,
};

And later you change it to something like:

static bool vhost_fs_user_internal_state_needed(void *opaque)
{
      VHostUserFS *s = opaque;

      return s->migration_type == VMOST_USER_INTERNAL_MIGRATABLE;
}

static const VMStateDescription vmstate_vhost_user_fs_internal_sub = {
      .name = "vhost-user-fs/internal",
      .version_id = 1,
      .minimum_version_id = 1,
      .needed = &vhost_fs_user_internal_state_needed,
      .fields = (VMStateField[]) {
          .... // Whatever
          VMSTATE_END_OF_LIST()
      }
};

static const VMStateDescription vuf_vmstate = {
      .name = "vhost-user-fs",
      .minimum_version_id = 0,
      .version_id = 0,
      .fields = (VMStateField[]) {
          VMSTATE_INT8(migration_type, struct VHostUserFS),
          VMSTATE_VIRTIO_DEVICE,
          VMSTATE_END_OF_LIST()
      },
      .pre_save = vhost_user_fs_pre_save,
      .subsections = (const VMStateDescription*[]) {
          &vmstate_vhost_user_fs_internal_sub,
          NULL
      }
};

And you are done.

I will propose to use a property to set migration_type, but I didn't
want to write the code right now.

I have a little problem with implementation here and need an advice:

Can we make this property adjustable at runtime after device was realized?
There is a statement in qemu wiki [1] that makes me think this is possible
but I couldn't find any code for it or example in other devices.
> "Properties are, by default, locked while the device is realized. Exceptions > can be made by the devices themselves in order to implement a way for a user
> to interact with a device while it is realized."

Or is your idea just to set this property once at construction and keep it
constant for device lifetime?

[1] https://wiki.qemu.org/Features/QOM


I think that this proposal will make Stephan happy, and it is just
adding and extra uint8_t that is helpul to implement everything.
That is exactly the approach I'm trying to implement it right now. Single
flag can be convenient for orchestrator but makes it too hard to account in
all cases for all devices on qemu side without breaking future
compatibility.
So I'm rewriting it with properties.
Nice.  That would be my proposal.  Just a bit complicated for a proof of 
concetp.

BTW do you think each vhost-user device should have its own enum of
migration
types or maybe we could make them common for all device types?
I will put it for vhost-user, because as far as I know nobody else is
asking for this functionality.

I mean do we need it for all vhost-user devices or only for vhost-user-fs
that I'm implementing now?


The most similar device that I can think of right now is vfio devices.
But they are implemeting callbacks to save hardware device state, and
they go device by device, i.e. there is nothing general there.

Later, Juan.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]