Re: [QEMU HCI-8.0 PATCH v2 5/5] vhost-blk: preserve the uevent socket across cpr-exec

[Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>]

> qemu-update uses cpr-exec migration: QEMU re-execs itself in place.
> A plain monitor fdset descriptor like our uevent socket does not
> survive that (unlike the cpr_save_fd()'d tap/vhost fds), so without
> help the resized-device notifications would silently stop until the
> next full VM start.
> 
> Preserve it the same way the net backends do: on cold boot
> cpr_save_fd() the socket under the device's canonical path, and on
> the re-exec'd (incoming) side adopt it with cpr_find_fd() instead of
> reopening a command line fd that no longer resolves. The management
> layer passes a placeholder for the "ueventfd" property in that case.
> Drop the saved descriptor on teardown so it does not outlive the
> device.
> 
> https://virtuozzo.atlassian.net/browse/VSTOR-143437
> Signed-off-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
>
> diff --git a/hw/block/vhost-blk.c b/hw/block/vhost-blk.c
> index 7cb842a859e..eca12356ae5 100644
> --- a/hw/block/vhost-blk.c
> +++ b/hw/block/vhost-blk.c
> @@ -28,6 +28,7 @@
>  #include <sys/ioctl.h>
>  #include <linux/fs.h>
>  #include <linux/netlink.h>
> +#include "migration/cpr.h"
>  #include "system/runstate.h"
>  
>  static int vhost_blk_start(VirtIODevice *vdev)
> @@ -364,20 +365,36 @@ static bool vhost_blk_uevent_check(int fd, const char *src, Error **errp)
>  
>  static bool vhost_blk_uevent_attach(VHostBlk *s, Error **errp)
>  {
> +    g_autofree char *cpr_name = NULL;
> +
>      if (!s->conf.ueventfd) {
>          return true;
>      }
>  
> -    s->uevent_fd = qemu_open(s->conf.ueventfd, O_RDWR, errp);
> -    if (s->uevent_fd < 0) {
> -        error_prepend(errp, "vhost-blk: unable to open uevent socket: ");
> -        return false;
> -    }
> +    cpr_name = object_get_canonical_path(OBJECT(s));

That name is gonna be "/machine/peripheral/...".  Let's do CPR
consistently with other devices.  E.g. see how it's done in
vhost_vsock_device_realize():

  DeviceState *proxy = qdev_get_parent_bus(DEVICE(vsock))->parent;
  ...
  /* Add migration blockers if proxy->id isn't present */

For vhost-blk proxy->id is likely gonna be "virtio-disk0".

Also, in this case, since it's not a vhostfd or backend FD, but an
ueventfd, I'd prefer adding it as a suffix, as it's done for other
devices.  So CPR key should end up looking like "virtio-disk0_ueventfd".

>  
> -    if (!vhost_blk_uevent_check(s->uevent_fd, s->conf.ueventfd, errp)) {
> -        qemu_close(s->uevent_fd);
> -        s->uevent_fd = -1;
> -        return false;
> +    if (cpr_is_incoming()) {
> +        s->uevent_fd = cpr_find_fd(cpr_name, 0);
> +        if (s->uevent_fd < 0) {
> +            error_setg(errp,
> +                       "vhost-blk: no preserved uevent socket to restore");
> +            return false;
> +        }
> +    } else {
> +        s->uevent_fd = qemu_open(s->conf.ueventfd, O_RDWR, errp);
> +        if (s->uevent_fd < 0) {
> +            error_prepend(errp, "vhost-blk: unable to open uevent socket: ");
> +            return false;
> +        }
> +
> +        if (!vhost_blk_uevent_check(s->uevent_fd, s->conf.ueventfd, errp)) {

Don't we want the same validation for cpr_is_incoming() case?

Andrey

-- 
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>