On 04.11.25 13:17, Vladimir Sementsov-Ogievskiy wrote:
> We are going to share new chardev_init_logfd() with further
> alternative initialization interface. Let qemu_char_open() be
> a wrapper for .open(), and its artifacts (handle be_opened if
> was not set to false by backend, and filename).
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> chardev/char.c | 50 ++++++++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 18 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 3e432195a5..216c95c053 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -250,22 +250,6 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend,
> bool *be_opened, Error **errp)
> {
> ChardevClass *cc = CHARDEV_GET_CLASS(chr);
> - /* Any ChardevCommon member would work */
> - ChardevCommon *common = backend ? backend->u.null.data : NULL;
> -
> - if (common && common->logfile) {
> - int flags = O_WRONLY;
> - if (common->has_logappend &&
> - common->logappend) {
> - flags |= O_APPEND;
> - } else {
> - flags |= O_TRUNC;
> - }
> - chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> - if (chr->logfd < 0) {
> - return;
> - }
> - }
>
> if (cc->open) {
> cc->open(chr, backend, be_opened, errp);
> @@ -1000,6 +984,29 @@ void qemu_chr_set_feature(Chardev *chr,
> return set_bit(feature, chr->features);
> }
>
> +static bool chardev_init_common(Chardev *chr, ChardevBackend *backend,
> + Error **errp)
> +{
> + /* Any ChardevCommon member would work */
> + ChardevCommon *common = backend ? backend->u.null.data : NULL;
> +
> + if (common && common->logfile) {
> + int flags = O_WRONLY;
> + if (common->has_logappend &&
> + common->logappend) {
> + flags |= O_APPEND;
> + } else {
> + flags |= O_TRUNC;
> + }
> + chr->logfd = qemu_create(common->logfile, flags, 0666, errp);
> + if (chr->logfd < 0) {
> + return false;
> + }
> + }
> +
> + return true;
> +}
> +
> static Chardev *chardev_new(const char *id, const char *typename,
> ChardevBackend *backend,
> GMainContext *gcontext,
> @@ -1020,11 +1027,14 @@ static Chardev *chardev_new(const char *id, const char *typename,
> chr->label = g_strdup(id);
> chr->gcontext = gcontext;
>
> + if (!chardev_init_common(chr, backend, errp)) {
> + goto fail;
> + }
> +
> qemu_char_open(chr, backend, &be_opened, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> - object_unref(obj);
> - return NULL;
> + goto fail;
> }
>
> if (!chr->filename) {
> @@ -1035,6 +1045,10 @@ static Chardev *chardev_new(const char *id, const char *typename,
> }
>
> return chr;
> +
> +fail:
> + object_unref(obj);
> + return NULL;
> }
>
> Chardev *qemu_chardev_new(const char *id, const char *typename,
This commit means that we'll not migrate the logfd.. But why not? I have not current plans
to support migration of serial port devices, but seems good to save a possibility for it
now.
At least, if not support now, I should explicitly error-out on trying to migreate chardev
with logfd, instead of silently recreate the file on target (which will actually break
source, which continue writing to it)
--
Best regards,
Vladimir