Re: [PATCH v2 18/25] chardev: introduce backend-transfer vmstate for chardev

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Oct 16, 2025 at 02:40:55PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> We'll need to transfer the chardev attached to vhost-user-blk, to
> support backend-transfer migration for vhost-user-blk. So, prepare
> chardev vmsd now.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> ---
>  chardev/char-backend-transfer.c         | 52 +++++++++++++++++++++++++
>  chardev/meson.build                     |  1 +
>  include/chardev/char-backend-transfer.h | 17 ++++++++
>  3 files changed, 70 insertions(+)
>  create mode 100644 chardev/char-backend-transfer.c
>  create mode 100644 include/chardev/char-backend-transfer.h
> 
> diff --git a/chardev/char-backend-transfer.c b/chardev/char-backend-transfer.c
> new file mode 100644
> index 0000000000..f1a399c7fa
> --- /dev/null
> +++ b/chardev/char-backend-transfer.c
> @@ -0,0 +1,52 @@
> +/*
> + * Event notifier migration support
> + * Copyright (c) Yandex Technologies LLC, 2025
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "chardev/char-fe.h"
> +#include "migration/vmstate.h"
> +
> +typedef struct CharBackendTransferTmp {
> +    CharBackend *parent;
> +    int fd;
> +} CharBackendTransferTmp;
> +
> +static int char_backend_transfer_pre_save(void *opaque)
> +{
> +    CharBackendTransferTmp *tmp = opaque;
> +
> +    tmp->fd = qemu_chr_get_client(tmp->parent->chr);
> +    if (tmp->fd < 0) {
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static int char_backend_transfer_post_load(void *opaque, int version_id)
> +{
> +    CharBackendTransferTmp *tmp = opaque;
> +
> +    return qemu_chr_add_client(tmp->parent->chr, tmp->fd);
> +}
> +
> +const VMStateDescription vmstate_backend_transfer_char_tmp = {
> +    .name = "backend-transfer-char-tmp",
> +    .pre_save = char_backend_transfer_pre_save,
> +    .post_load = char_backend_transfer_post_load,
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_FD(fd, CharBackendTransferTmp),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};
> +
> +const VMStateDescription vmstate_backend_transfer_char = {
> +    .name = "backend-transfer-char",
> +    .fields = (const VMStateField[]) {
> +        VMSTATE_WITH_TMP(CharBackend, CharBackendTransferTmp,
> +                         vmstate_backend_transfer_char_tmp),
> +        VMSTATE_END_OF_LIST()
> +    },
> +};

On the one hand this code is positioned as if it were a generic
mechanism for transferring chardev vmstate, but on the oother
hand it is hardcoded to only work with the socket chardev and
is only able to transfer an FD, no other state.

Socket chardevs can involve telnet, tls or websocket protocol
layering all of which have considerable state that is being
ignored by this.

IMHO each chardev backend needs to be directly responsible
for handling vmstate, and it needs to be able to raise an
error if there is state which cannot be transferred. This
would avoid creatin of the undesirable qemu_chr_get_client
method as public API.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|