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

Vladimir Sementsov-Ogievskiy posted 25 patches 4 weeks, 1 day ago
[PATCH v2 18/25] chardev: introduce backend-transfer vmstate for chardev
Posted by Vladimir Sementsov-Ogievskiy 4 weeks, 1 day ago
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()
+    },
+};
diff --git a/chardev/meson.build b/chardev/meson.build
index 56ee39ac0b..239c8cd072 100644
--- a/chardev/meson.build
+++ b/chardev/meson.build
@@ -31,6 +31,7 @@ chardev_ss = chardev_ss.apply({})
 
 system_ss.add(files(
     'char-hmp-cmds.c',
+    'char-backend-transfer.c',
     'msmouse.c',
     'wctablet.c',
     'testdev.c'))
diff --git a/include/chardev/char-backend-transfer.h b/include/chardev/char-backend-transfer.h
new file mode 100644
index 0000000000..2c3da5f836
--- /dev/null
+++ b/include/chardev/char-backend-transfer.h
@@ -0,0 +1,17 @@
+/*
+ * Event notifier migration support
+ * Copyright (c) Yandex Technologies LLC, 2025
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef CHAR_BACKEND_TRANSFER_H
+#define CHAR_BACKEND_TRANSFER_H
+
+#include "qemu/typedefs.h"
+
+extern const VMStateDescription vmstate_backend_transfer_char;
+#define VMSTATE_BACKEND_TRANSFER_CHARDEV(_field, _state)                       \
+    VMSTATE_STRUCT(_field, _state, 0, vmstate_backend_transfer_char,           \
+                   CharBackend)
+
+#endif
-- 
2.48.1
Re: [PATCH v2 18/25] chardev: introduce backend-transfer vmstate for chardev
Posted by Daniel P. Berrangé 4 weeks, 1 day ago
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 :|
Re: [PATCH v2 18/25] chardev: introduce backend-transfer vmstate for chardev
Posted by Vladimir Sementsov-Ogievskiy 4 weeks, 1 day ago
On 16.10.25 14:55, Daniel P. Berrangé wrote:
> 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.
> 

Ow, that's right, me ashamed. I implemented it in a wrong layer.
Will rework, thanks!

-- 
Best regards,
Vladimir