| Message ID | 20230727115844.8480-1-faithilikerun@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] block/file-posix: fix g_file_get_contents return path | expand |
On Thu, Jul 27, 2023 at 07:58:44PM +0800, Sam Li wrote: > The g_file_get_contents() function returns a g_boolean. If it fails, the > returned value will be 0 instead of -1. Solve the issue by skipping > assigning ret value. > > This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed > by an NVMe partition e.g. /dev/nvme0n1p1 on s390x. > > Signed-off-by: Sam Li <faithilikerun@gmail.com> > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/file-posix.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> We should put this in 8.1 rc2 IMHO With regards, Daniel
On Thu, Jul 27, 2023 at 07:58:44PM +0800, Sam Li wrote: > The g_file_get_contents() function returns a g_boolean. If it fails, the > returned value will be 0 instead of -1. Solve the issue by skipping > assigning ret value. > > This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed > by an NVMe partition e.g. /dev/nvme0n1p1 on s390x. > > Signed-off-by: Sam Li <faithilikerun@gmail.com> > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/file-posix.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) Thanks, applied to my block tree: https://gitlab.com/stefanha/qemu/commits/block Stefan
On Thu, Jul 27, 2023 at 01:15:32PM +0100, Daniel P. Berrangé wrote: > On Thu, Jul 27, 2023 at 07:58:44PM +0800, Sam Li wrote: > > The g_file_get_contents() function returns a g_boolean. If it fails, the > > returned value will be 0 instead of -1. Solve the issue by skipping > > assigning ret value. > > > > This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed > > by an NVMe partition e.g. /dev/nvme0n1p1 on s390x. > > > > Signed-off-by: Sam Li <faithilikerun@gmail.com> > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > block/file-posix.c | 6 ++---- > > 1 file changed, 2 insertions(+), 4 deletions(-) > > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> > > We should put this in 8.1 rc2 IMHO Agreed, I will send a block pull request for 8.1. Stefan
diff --git a/block/file-posix.c b/block/file-posix.c index 9e8e3d8ca5..b16e9c21a1 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1232,7 +1232,6 @@ static int hdev_get_max_hw_transfer(int fd, struct stat *st) static int get_sysfs_str_val(struct stat *st, const char *attribute, char **val) { g_autofree char *sysfspath = NULL; - int ret; size_t len; if (!S_ISBLK(st->st_mode)) { @@ -1242,8 +1241,7 @@ static int get_sysfs_str_val(struct stat *st, const char *attribute, sysfspath = g_strdup_printf("/sys/dev/block/%u:%u/queue/%s", major(st->st_rdev), minor(st->st_rdev), attribute); - ret = g_file_get_contents(sysfspath, val, &len, NULL); - if (ret == -1) { + if (!g_file_get_contents(sysfspath, val, &len, NULL)) { return -ENOENT; } @@ -1253,7 +1251,7 @@ static int get_sysfs_str_val(struct stat *st, const char *attribute, if (*(p + len - 1) == '\n') { *(p + len - 1) = '\0'; } - return ret; + return 0; } #endif
The g_file_get_contents() function returns a g_boolean. If it fails, the returned value will be 0 instead of -1. Solve the issue by skipping assigning ret value. This issue was found by Matthew Rosato using virtio-blk-{pci,ccw} backed by an NVMe partition e.g. /dev/nvme0n1p1 on s390x. Signed-off-by: Sam Li <faithilikerun@gmail.com> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/file-posix.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)