* [PATCH hci-8.0 v2 1/3] chardev: implement cpr_claim_fds() in the base class #VSTOR-148304
2026-10-05 9:26 [PATCH hci-8.0 v2 0/3] chardev: fix qemu-update destroying VMs with a console log #VSTOR-148304 Denis V. Lunev
@ 2026-10-05 9:26 ` Denis V. Lunev
2026-10-05 9:28 ` no-reply
2026-10-05 9:26 ` [PATCH hci-8.0 v2 2/3] chardev: preserve the logfile fd across cpr-exec #VSTOR-148304 Denis V. Lunev
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Denis V. Lunev @ 2026-10-05 9:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev, den
From: Denis V. Lunev <den@openvz.org>
Give TYPE_CHARDEV an empty cpr_claim_fds() and make the file, pty and
socket backends chain to their parent's. Every chardev class then has
the hook, so qmp_chardev_change() calls it without a NULL check, and
state owned by the base class can be re-claimed there as well.
No functional change.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
chardev/char-file.c | 3 +++
chardev/char-pty.c | 3 +++
chardev/char-socket.c | 3 +++
chardev/char.c | 13 +++++++------
include/chardev/char.h | 5 ++++-
5 files changed, 20 insertions(+), 7 deletions(-)
diff --git a/chardev/char-file.c b/chardev/char-file.c
index a90f4a77e13..89011f52e3e 100644
--- a/chardev/char-file.c
+++ b/chardev/char-file.c
@@ -124,7 +124,10 @@ out_close:
static void char_file_cpr_claim_fds(Chardev *chr)
{
FDChardev *s = FD_CHARDEV(chr);
+ ChardevClass *pc = CHARDEV_CLASS(
+ object_class_get_parent(object_class_by_name(TYPE_CHARDEV_FILE)));
+ pc->cpr_claim_fds(chr);
if (s->ioc_out) {
cpr_delete_fd(chr->label, 0);
cpr_save_fd(chr->label, 0, QIO_CHANNEL_FILE(s->ioc_out)->fd);
diff --git a/chardev/char-pty.c b/chardev/char-pty.c
index a0090e4c3d1..8058da92bb4 100644
--- a/chardev/char-pty.c
+++ b/chardev/char-pty.c
@@ -403,7 +403,10 @@ static void char_pty_open(Chardev *chr,
static void char_pty_cpr_claim_fds(Chardev *chr)
{
PtyChardev *s = PTY_CHARDEV(chr);
+ ChardevClass *pc = CHARDEV_CLASS(
+ object_class_get_parent(object_class_by_name(TYPE_CHARDEV_PTY)));
+ pc->cpr_claim_fds(chr);
if (s->ioc) {
cpr_delete_fd(chr->label, 0);
cpr_save_fd(chr->label, 0, QIO_CHANNEL_FILE(s->ioc)->fd);
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index d20aea764f9..c73a3dd0f7a 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1681,7 +1681,10 @@ char_socket_get_connected(Object *obj, Error **errp)
static void char_socket_cpr_claim_fds(Chardev *chr)
{
SocketChardev *s = SOCKET_CHARDEV(chr);
+ ChardevClass *pc = CHARDEV_CLASS(
+ object_class_get_parent(object_class_by_name(TYPE_CHARDEV_SOCKET)));
+ pc->cpr_claim_fds(chr);
if (s->sioc) {
cpr_delete_fd(chr->label, CPR_CONNSOCK_ID);
cpr_save_fd(chr->label, CPR_CONNSOCK_ID, s->sioc->fd);
diff --git a/chardev/char.c b/chardev/char.c
index 87f40d0137e..b64767f574e 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -295,12 +295,17 @@ static int null_chr_write(Chardev *chr, const uint8_t *buf, int len)
return len;
}
+static void char_cpr_claim_fds(Chardev *chr)
+{
+}
+
static void char_class_init(ObjectClass *oc, const void *data)
{
ChardevClass *cc = CHARDEV_CLASS(oc);
cc->chr_write = null_chr_write;
cc->chr_be_event = chr_be_event;
+ cc->cpr_claim_fds = char_cpr_claim_fds;
}
static void char_finalize(Object *obj)
@@ -1164,9 +1169,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
backend, chr->gcontext, handover_yank_instance, errp);
if (!chr_new) {
/* the failed replacement's open() may have displaced our cpr fds */
- if (cc->cpr_claim_fds) {
- cc->cpr_claim_fds(chr);
- }
+ cc->cpr_claim_fds(chr);
return NULL;
}
@@ -1186,9 +1189,7 @@ ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
qemu_chr_be_event(chr, CHR_EVENT_OPENED);
}
/* chr_new's open() may have displaced our cpr fds; re-claim them */
- if (cc->cpr_claim_fds) {
- cc->cpr_claim_fds(chr);
- }
+ cc->cpr_claim_fds(chr);
object_unref(OBJECT(chr_new));
return NULL;
}
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 431d07ad3c6..968766fd5d5 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -305,7 +305,10 @@ struct ChardevClass {
/* notify the backend of frontend open state */
void (*chr_set_fe_open)(Chardev *chr, int fe_open);
- /* (re-)register the backend's CPR fds, e.g. on a failed chardev-change */
+ /*
+ * (re-)register CPR fds, e.g. on a failed chardev-change; an override
+ * must call the parent's hook first
+ */
void (*cpr_claim_fds)(Chardev *chr);
/* handle various events */
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH hci-8.0 v2 2/3] chardev: preserve the logfile fd across cpr-exec #VSTOR-148304
2026-10-05 9:26 [PATCH hci-8.0 v2 0/3] chardev: fix qemu-update destroying VMs with a console log #VSTOR-148304 Denis V. Lunev
2026-10-05 9:26 ` [PATCH hci-8.0 v2 1/3] chardev: implement cpr_claim_fds() in the base class #VSTOR-148304 Denis V. Lunev
@ 2026-10-05 9:26 ` Denis V. Lunev
2026-10-05 9:30 ` no-reply
2026-10-05 9:26 ` [PATCH hci-8.0 v2 3/3] iotests/migrate-cpr-exec: cover chardevs with logfile= #VSTOR-148304 Denis V. Lunev
2026-10-05 9:47 ` [PATCH hci-8.0 v2 0/3] chardev: fix qemu-update destroying VMs with a console log #VSTOR-148304 Andrey Drobyshev
3 siblings, 1 reply; 8+ messages in thread
From: Denis V. Lunev @ 2026-10-05 9:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev, den
From: Denis V. Lunev <den@openvz.org>
A chardev with logfile= opens the log in qemu_char_open() on every
start, including the start of the new QEMU after cpr-exec. The backend
fds are taken from the cpr fd registry there, the log fd is not, so the
new process reopens the path itself.
libvirt normally opens the log through virtlogd and passes a pipe in an
fdset. The qemu-update command line cannot carry that fdset, so it
names the log file path. The exec'd QEMU, running as the qemu user,
then fails to create a log it has no write access to, e.g. a nova
console.log on node-local storage:
qemu-kvm: -chardev pty,id=charserial0,logfile=.../console.log,
logappend=on: Could not create '.../console.log': Permission denied
and the VM is destroyed. Where the open succeeds, the new QEMU writes
the file directly, behind virtlogd's back.
Save the log fd in the cpr registry under the chardev label and take it
from there on cpr-exec incoming, as the backends do. The logfile path
is not opened at all then. If the source did not save it, run without
a log rather than reopen the path. A failed chardev-change re-claims
the log through the base class cpr_claim_fds(), since the replacement's
log has displaced the entry.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
chardev/char.c | 30 +++++++++++++++++++++++++-----
include/migration/cpr.h | 1 +
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/chardev/char.c b/chardev/char.c
index b64767f574e..c037d2e7730 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -40,6 +40,7 @@
#include "qemu/id.h"
#include "qemu/coroutine.h"
#include "qemu/yank.h"
+#include "migration/cpr.h"
#include "chardev-internal.h"
@@ -261,9 +262,21 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
} else {
flags |= O_TRUNC;
}
- chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
- if (chr->logfd < 0) {
- return;
+ if (cpr_exec_is_incoming()) {
+ chr->logfd = cpr_find_fd(chr->label, CPR_CHARDEV_LOG_ID);
+ if (chr->logfd < 0) {
+ warn_report("chardev '%s': log file was not preserved, "
+ "continuing without it", chr->label);
+ }
+ } else {
+ chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
+ if (chr->logfd < 0) {
+ return;
+ }
+ if (!cpr_is_incoming()) {
+ cpr_delete_fd(chr->label, CPR_CHARDEV_LOG_ID);
+ cpr_save_fd(chr->label, CPR_CHARDEV_LOG_ID, chr->logfd);
+ }
}
}
@@ -297,6 +310,10 @@ static int null_chr_write(Chardev *chr, const uint8_t *buf, int len)
static void char_cpr_claim_fds(Chardev *chr)
{
+ if (chr->logfd >= 0) {
+ cpr_delete_fd(chr->label, CPR_CHARDEV_LOG_ID);
+ cpr_save_fd(chr->label, CPR_CHARDEV_LOG_ID, chr->logfd);
+ }
}
static void char_class_init(ObjectClass *oc, const void *data)
@@ -315,11 +332,14 @@ static void char_finalize(Object *obj)
if (chr->be) {
chr->be->chr = NULL;
}
- g_free(chr->filename);
- g_free(chr->label);
if (chr->logfd != -1) {
+ if (cpr_find_fd(chr->label, CPR_CHARDEV_LOG_ID) == chr->logfd) {
+ cpr_delete_fd(chr->label, CPR_CHARDEV_LOG_ID);
+ }
close(chr->logfd);
}
+ g_free(chr->filename);
+ g_free(chr->label);
qemu_mutex_destroy(&chr->chr_write_lock);
}
diff --git a/include/migration/cpr.h b/include/migration/cpr.h
index d74ff1ea602..8fb79eb0a28 100644
--- a/include/migration/cpr.h
+++ b/include/migration/cpr.h
@@ -15,6 +15,7 @@
#define CPR_LISTENSOCK_ID 0
#define CPR_CONNSOCK_ID 1
+#define CPR_CHARDEV_LOG_ID 2
#define QEMU_CPR_FILE_MAGIC 0x51435052
#define QEMU_CPR_FILE_VERSION 0x00000001
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH hci-8.0 v2 3/3] iotests/migrate-cpr-exec: cover chardevs with logfile= #VSTOR-148304
2026-10-05 9:26 [PATCH hci-8.0 v2 0/3] chardev: fix qemu-update destroying VMs with a console log #VSTOR-148304 Denis V. Lunev
2026-10-05 9:26 ` [PATCH hci-8.0 v2 1/3] chardev: implement cpr_claim_fds() in the base class #VSTOR-148304 Denis V. Lunev
2026-10-05 9:26 ` [PATCH hci-8.0 v2 2/3] chardev: preserve the logfile fd across cpr-exec #VSTOR-148304 Denis V. Lunev
@ 2026-10-05 9:26 ` Denis V. Lunev
2026-10-05 9:35 ` no-reply
2026-10-05 9:47 ` [PATCH hci-8.0 v2 0/3] chardev: fix qemu-update destroying VMs with a console log #VSTOR-148304 Andrey Drobyshev
3 siblings, 1 reply; 8+ messages in thread
From: Denis V. Lunev @ 2026-10-05 9:26 UTC (permalink / raw)
To: svt-core; +Cc: andrey.drobyshev, den
From: Denis V. Lunev <den@openvz.org>
Run a console pty with its log in an fdset, a guest agent socket and a
second QMP monitor, each with a log, through cpr-exec with a command
line whose logfile paths cannot be created, as libvirt's is. Cover
chardev-change to a new log for pty and socket, and a failed
chardev-change, whose rollback must keep the old log preserved.
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
tests/qemu-iotests/tests/migrate-cpr-exec | 154 ++++++++++++++++++
tests/qemu-iotests/tests/migrate-cpr-exec.out | 4 +-
2 files changed, 156 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/tests/migrate-cpr-exec b/tests/qemu-iotests/tests/migrate-cpr-exec
index 51e6cbcd659..2b30929561c 100755
--- a/tests/qemu-iotests/tests/migrate-cpr-exec
+++ b/tests/qemu-iotests/tests/migrate-cpr-exec
@@ -288,6 +288,160 @@ class TestCprExec(iotests.QMPTestCase):
'-device', 'isa-serial,chardev=char0')
self._test_cpr(pre_cpr_hook=change_socket)
+ BAD_LOG = os.path.join(iotests.test_dir, 'nonexistent', 'chr.log')
+
+ def _serial_write(self, data: str) -> None:
+ for c in data:
+ self.vm.qtest(f'outb 0x3f8 {ord(c)}')
+
+ def _open_files(self) -> set:
+ fd_dir = f'/proc/{self.vm.get_pid()}/fd'
+ files = set()
+ for fd in os.listdir(fd_dir):
+ try:
+ files.add(os.readlink(os.path.join(fd_dir, fd)))
+ except FileNotFoundError:
+ pass
+ return files
+
+ def _replace_arg(self, old: str, new: str) -> None:
+ args = list(self.vm._qemu_full_args)
+ args[args.index(old)] = new
+ self.vm._qemu_full_args = tuple(args)
+
+ def _read(self, path: str) -> str:
+ with open(path, encoding='utf-8') as f:
+ return f.read()
+
+ def _log_path(self, name: str) -> str:
+ path = os.path.join(iotests.test_dir, name)
+ self.addCleanup(lambda: os.path.exists(path) and os.remove(path))
+ return path
+
+ def test_chardev_logfile(self) -> None:
+ """Log fds survive cpr-exec; the new logfile= paths are never opened"""
+ console_log = self._log_path('console.log')
+ agent_log = self._log_path('agent.log')
+ mon_log = self._log_path('mon.log')
+ agent_sock = iotests.file_path('agent.sock')
+ mon_sock = iotests.file_path('mon.sock')
+
+ console = 'pty,id=charserial0,logfile=/dev/fdset/1,logappend=on'
+ agent = (f'socket,id=charchannel0,path={agent_sock},server=on,'
+ f'wait=off,logfile={agent_log}')
+ mon = (f'socket,id=charmonitor1,path={mon_sock},server=on,'
+ f'wait=off,logfile={mon_log}')
+
+ def pre_cpr():
+ self._serial_write('before\n')
+ for c in (console, agent, mon):
+ self._replace_arg(c, c.replace(c[c.index('logfile='):],
+ f'logfile={self.BAD_LOG}'))
+ args = list(self.vm._qemu_full_args)
+ i = args.index('-add-fd')
+ self.vm._qemu_full_args = tuple(args[:i] + args[i + 2:])
+
+ def post_cpr():
+ self._serial_write('after\n')
+ self.assertEqual(self._read(console_log), 'before\nafter\n')
+ self.assertLessEqual({agent_log, mon_log}, self._open_files())
+ self.assertFalse(os.path.exists(os.path.dirname(self.BAD_LOG)))
+
+ fd = os.open(console_log, os.O_WRONLY | os.O_CREAT | os.O_APPEND)
+ try:
+ self.vm.add_fd(fd, 1, 'console')
+ self.vm.add_args(
+ '-chardev', console,
+ '-device', 'isa-serial,chardev=charserial0',
+ '-chardev', agent, '-device', 'virtio-serial',
+ '-device', 'virtserialport,chardev=charchannel0,'
+ 'name=org.qemu.guest_agent.0',
+ '-chardev', mon, '-mon', 'chardev=charmonitor1,mode=control')
+ self._test_cpr(pre_cpr_hook=pre_cpr, post_cpr_hook=post_cpr)
+ finally:
+ os.close(fd)
+
+ def _test_chardev_change_logfile(self, backend: str, data_new: dict,
+ opts_old: str) -> None:
+ """cpr-exec after chardev-change preserves the new log, not the old"""
+ log_old = self._log_path('chr-old.log')
+ log_new = self._log_path('chr-new.log')
+ arg = f'{backend},id=char0,{opts_old}logfile={log_old}'
+
+ def change():
+ self._serial_write('before\n')
+ res = self.vm.qmp('chardev-change', id='char0',
+ backend={'type': backend,
+ 'data': {**data_new,
+ 'logfile': log_new}})
+ assert 'return' in res, res
+ self._replace_arg(arg, f'{backend},id=char0,{opts_old}'
+ f'logfile={self.BAD_LOG}')
+
+ def post_cpr():
+ self._serial_write('after\n')
+ self.assertEqual(self._read(log_old), 'before\n')
+ self.assertEqual(self._read(log_new), 'after\n')
+ self.assertNotIn(log_old, self._open_files())
+
+ self.vm.add_args('-chardev', arg,
+ '-device', 'isa-serial,chardev=char0')
+ self._test_cpr(pre_cpr_hook=change, post_cpr_hook=post_cpr)
+
+ def test_chardev_change_logfile_pty(self) -> None:
+ self._test_chardev_change_logfile('pty', {}, '')
+
+ def test_chardev_change_logfile_socket(self) -> None:
+ sock1 = iotests.file_path('chr-log-sock1')
+ sock2 = iotests.file_path('chr-log-sock2')
+ self._test_chardev_change_logfile(
+ 'socket',
+ {'addr': {'type': 'unix', 'data': {'path': sock2}},
+ 'server': True, 'wait': False},
+ f'path={sock1},server=on,wait=off,')
+
+ def test_chardev_change_logfile_rollback(self) -> None:
+ """A rolled back chardev-change keeps the old log preserved"""
+ log = self._log_path('chr.log')
+ log_new = self._log_path('chr-new.log')
+ link = self._log_path('pty-rollback.link')
+ open(link, 'w').close()
+ arg = f'pty,id=char0,logfile={log}'
+
+ def failed_change():
+ self._serial_write('before\n')
+ res = self.vm.qmp('chardev-change', id='char0',
+ backend={'type': 'pty',
+ 'data': {'path': link,
+ 'logfile': log_new}})
+ assert 'error' in res, res
+ self._replace_arg(arg, f'pty,id=char0,logfile={self.BAD_LOG}')
+
+ def post_cpr():
+ self._serial_write('after\n')
+ self.assertEqual(self._read(log), 'before\nafter\n')
+
+ self.vm.add_args('-chardev', arg,
+ '-device', 'isa-serial,chardev=char0')
+ self._test_cpr(pre_cpr_hook=failed_change, post_cpr_hook=post_cpr)
+
+ def test_chardev_logfile_not_preserved(self) -> None:
+ """A log the source did not save is dropped, not reopened by path"""
+ log = self._log_path('unsaved.log')
+ arg = 'pty,id=char0'
+
+ def add_log():
+ self._replace_arg(arg, f'{arg},logfile={log}')
+
+ def post_cpr():
+ self.assertFalse(os.path.exists(log))
+
+ self.vm.add_args('-chardev', arg,
+ '-device', 'isa-serial,chardev=char0')
+ self._test_cpr(pre_cpr_hook=add_log, post_cpr_hook=post_cpr)
+ self.assertIn("chardev 'char0': log file was not preserved, "
+ "continuing without it", self.vm.get_log())
+
def test_nbd_attached_hd(self) -> None:
nbd_img = os.path.join(iotests.test_dir, 'nbd_disk')
iotests.qemu_img_create('-f', 'qcow2', nbd_img, '64M')
diff --git a/tests/qemu-iotests/tests/migrate-cpr-exec.out b/tests/qemu-iotests/tests/migrate-cpr-exec.out
index 5ce2f9a2ed9..cfa5c0d0e66 100644
--- a/tests/qemu-iotests/tests/migrate-cpr-exec.out
+++ b/tests/qemu-iotests/tests/migrate-cpr-exec.out
@@ -1,5 +1,5 @@
-..............................
+...................................
----------------------------------------------------------------------
-Ran 30 tests
+Ran 35 tests
OK
--
2.53.0
^ permalink raw reply [flat|nested] 8+ messages in thread