Hi
On Tue, Oct 14, 2025 at 1:21 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
>
> On 14.10.25 10:30, Marc-André Lureau wrote:
> > On Mon, Oct 13, 2025 at 5:41 PM Vladimir Sementsov-Ogievskiy
> > <[email protected]> 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 <[email protected]>
> >
> > Reviewed-by: Marc-André Lureau <[email protected]>
> >
> >> ---
> >> chardev/char.c | 49 +++++++++++++++++++++++++++++++------------------
> >> 1 file changed, 31 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/chardev/char.c b/chardev/char.c
> >> index a43b7e5481..d5a2533e8e 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 */
> >
> > maybe keep that comment?
>
> I a bit don't follow it, honestly.. What it mean? That we should
> handle members of common structure here?
The ChardevBackend type is a union, and all the members inherit from
ChardevCommon. Ideally, we wouldn't cast this way, but that would
require some changes in code generator...
>
> Not a problem to put it back to chardev_init_logfd().. But maybe, it
> then should be renamed to chardev_init_common()
>
> >
> >
> >> - 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,28 @@ void qemu_chr_set_feature(Chardev *chr,
> >> return set_bit(feature, chr->features);
> >> }
> >>
> >> +static bool chardev_init_logfd(Chardev *chr, ChardevBackend *backend,
> >> + Error **errp)
> >> +{
> >> + 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 +1026,14 @@ static Chardev *chardev_new(const char *id, const
> >> char *typename,
> >> chr->label = g_strdup(id);
> >> chr->gcontext = gcontext;
> >>
> >> + if (!chardev_init_logfd(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 +1044,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,
> >> --
> >> 2.48.1
> >>
> >>
> >
> >
> > --
> > Marc-André Lureau
>
>
> --
> Best regards,
> Vladimir
--
Marc-André Lureau
