| Message ID | 20240715082155.28771-8-zhao1.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | trivial: Replace some qemu_open_old() with qemu_open() | expand |
On 15/7/24 10:21, Zhao Liu wrote: > For qemu_open_old(), osdep.h said: > >> Don't introduce new usage of this function, prefer the following >> qemu_open/qemu_create that take an "Error **errp". > > So replace qemu_open_old() with qemu_open(). And considering > rng_random_opened() will lose its obvious error handling case after > removing error_setg_file_open(), add comment to remind here. > > Cc: Laurent Vivier <lvivier@redhat.com> > Cc: Amit Shah <amit@kernel.org> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > backends/rng-random.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/backends/rng-random.c b/backends/rng-random.c > index 80eb5be138ce..3cdb982533b5 100644 > --- a/backends/rng-random.c > +++ b/backends/rng-random.c > @@ -75,10 +75,11 @@ static void rng_random_opened(RngBackend *b, Error **errp) > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > "filename", "a valid filename"); > } else { > - s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK); > - if (s->fd == -1) { > - error_setg_file_open(errp, errno, s->filename); > - } > + /* > + * Once the open fails, the error message is integrated into > + * the *errp object by qemu_open(). > + */ IMHO this comment is superfluous, I'd drop it, otherwise: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > + s->fd = qemu_open(s->filename, O_RDONLY | O_NONBLOCK, errp); > } > } >
On Mon, Jul 15, 2024 at 11:46:54AM +0200, Philippe Mathieu-Daudé wrote: > Date: Mon, 15 Jul 2024 11:46:54 +0200 > From: Philippe Mathieu-Daudé <philmd@linaro.org> > Subject: Re: [PATCH 7/7] backends/rng-random: Get rid of qemu_open_old() > > On 15/7/24 10:21, Zhao Liu wrote: > > For qemu_open_old(), osdep.h said: > > > > > Don't introduce new usage of this function, prefer the following > > > qemu_open/qemu_create that take an "Error **errp". > > > > So replace qemu_open_old() with qemu_open(). And considering > > rng_random_opened() will lose its obvious error handling case after > > removing error_setg_file_open(), add comment to remind here. > > > > Cc: Laurent Vivier <lvivier@redhat.com> > > Cc: Amit Shah <amit@kernel.org> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > > --- > > backends/rng-random.c | 9 +++++---- > > 1 file changed, 5 insertions(+), 4 deletions(-) > > > > diff --git a/backends/rng-random.c b/backends/rng-random.c > > index 80eb5be138ce..3cdb982533b5 100644 > > --- a/backends/rng-random.c > > +++ b/backends/rng-random.c > > @@ -75,10 +75,11 @@ static void rng_random_opened(RngBackend *b, Error **errp) > > error_setg(errp, QERR_INVALID_PARAMETER_VALUE, > > "filename", "a valid filename"); > > } else { > > - s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK); > > - if (s->fd == -1) { > > - error_setg_file_open(errp, errno, s->filename); > > - } > > + /* > > + * Once the open fails, the error message is integrated into > > + * the *errp object by qemu_open(). > > + */ > > IMHO this comment is superfluous, I'd drop it, otherwise: > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Thanks Philippe! I'll wait patch 1's comment, after that I can post a new version with the change you mentioned.
15.07.2024 13:10, Zhao Liu wrote: ... > Thanks Philippe! I'll wait patch 1's comment, after that I can post a > new version with the change you mentioned. I don't think either of this is necessary. Patch 1 LGTM, and I'll drop the comment while applying. /mjt
On Mon, Jul 15, 2024 at 02:07:00PM +0300, Michael Tokarev wrote: > Date: Mon, 15 Jul 2024 14:07:00 +0300 > From: Michael Tokarev <mjt@tls.msk.ru> > Subject: Re: [PATCH 7/7] backends/rng-random: Get rid of qemu_open_old() > > 15.07.2024 13:10, Zhao Liu wrote: > ... > > Thanks Philippe! I'll wait patch 1's comment, after that I can post a > > new version with the change you mentioned. > > I don't think either of this is necessary. Patch 1 LGTM, and I'll > drop the comment while applying. > Many thanks!
diff --git a/backends/rng-random.c b/backends/rng-random.c index 80eb5be138ce..3cdb982533b5 100644 --- a/backends/rng-random.c +++ b/backends/rng-random.c @@ -75,10 +75,11 @@ static void rng_random_opened(RngBackend *b, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "filename", "a valid filename"); } else { - s->fd = qemu_open_old(s->filename, O_RDONLY | O_NONBLOCK); - if (s->fd == -1) { - error_setg_file_open(errp, errno, s->filename); - } + /* + * Once the open fails, the error message is integrated into + * the *errp object by qemu_open(). + */ + s->fd = qemu_open(s->filename, O_RDONLY | O_NONBLOCK, errp); } }