| Message ID | f34ee80866e6f591bcb98401dee27682f5543fca.1629190206.git.mprivozn@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Two chardev with fdset fixes | expand |
On 8/17/21 10:56 AM, Michal Privoznik wrote: > If a chardev has a logfile the file is opened using > qemu_open_old() which does the job, but since @errp is not > propagated into qemu_open_internal() we lose much more accurate > error and just report "Unable to open logfile $errno". When > using plain files, it's probably okay as nothing complex is > happening behind the curtains. But the problem becomes more > prominent when passing an "/dev/fdset/XXX" path since much more > needs to be done. > > The fix is to use qemu_create() which passes @errp further down. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > --- > chardev/char.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
On Tue, Aug 17, 2021 at 12:56 PM Michal Privoznik <mprivozn@redhat.com> wrote: > If a chardev has a logfile the file is opened using > qemu_open_old() which does the job, but since @errp is not > propagated into qemu_open_internal() we lose much more accurate > error and just report "Unable to open logfile $errno". When > using plain files, it's probably okay as nothing complex is > happening behind the curtains. But the problem becomes more > prominent when passing an "/dev/fdset/XXX" path since much more > needs to be done. > > The fix is to use qemu_create() which passes @errp further down. > > Signed-off-by: Michal Privoznik <mprivozn@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- > chardev/char.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/chardev/char.c b/chardev/char.c > index 4595a8d430..0169d8dde4 100644 > --- a/chardev/char.c > +++ b/chardev/char.c > @@ -241,18 +241,15 @@ static void qemu_char_open(Chardev *chr, > ChardevBackend *backend, > ChardevCommon *common = backend ? backend->u.null.data : NULL; > > if (common && common->has_logfile) { > - int flags = O_WRONLY | O_CREAT; > + int flags = O_WRONLY; > if (common->has_logappend && > common->logappend) { > flags |= O_APPEND; > } else { > flags |= O_TRUNC; > } > - chr->logfd = qemu_open_old(common->logfile, flags, 0666); > + chr->logfd = qemu_create(common->logfile, flags, 0666, errp); > if (chr->logfd < 0) { > - error_setg_errno(errp, errno, > - "Unable to open logfile %s", > - common->logfile); > return; > } > } > -- > 2.31.1 > >
On 8/17/21 11:54 AM, Marc-André Lureau wrote: > On Tue, Aug 17, 2021 at 12:56 PM Michal Privoznik <mprivozn@redhat.com> > wrote: > >> If a chardev has a logfile the file is opened using >> qemu_open_old() which does the job, but since @errp is not >> propagated into qemu_open_internal() we lose much more accurate >> error and just report "Unable to open logfile $errno". When >> using plain files, it's probably okay as nothing complex is >> happening behind the curtains. But the problem becomes more >> prominent when passing an "/dev/fdset/XXX" path since much more >> needs to be done. >> >> The fix is to use qemu_create() which passes @errp further down. >> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com> >> > > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Thanks, can you add it to your pull queue please? Michal
diff --git a/chardev/char.c b/chardev/char.c index 4595a8d430..0169d8dde4 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -241,18 +241,15 @@ static void qemu_char_open(Chardev *chr, ChardevBackend *backend, ChardevCommon *common = backend ? backend->u.null.data : NULL; if (common && common->has_logfile) { - int flags = O_WRONLY | O_CREAT; + int flags = O_WRONLY; if (common->has_logappend && common->logappend) { flags |= O_APPEND; } else { flags |= O_TRUNC; } - chr->logfd = qemu_open_old(common->logfile, flags, 0666); + chr->logfd = qemu_create(common->logfile, flags, 0666, errp); if (chr->logfd < 0) { - error_setg_errno(errp, errno, - "Unable to open logfile %s", - common->logfile); return; } }
If a chardev has a logfile the file is opened using qemu_open_old() which does the job, but since @errp is not propagated into qemu_open_internal() we lose much more accurate error and just report "Unable to open logfile $errno". When using plain files, it's probably okay as nothing complex is happening behind the curtains. But the problem becomes more prominent when passing an "/dev/fdset/XXX" path since much more needs to be done. The fix is to use qemu_create() which passes @errp further down. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- chardev/char.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-)