| Message ID | 1428058914-32050-1-git-send-email-bogdan.purcareata@freescale.com |
|---|---|
| State | New |
| Headers | show |
On 03/04/2015 13:01, Bogdan Purcareata wrote: > This fixes ioctl behavior on powerpc e6500 platforms with 64bit kernel and 32bit > userspace. The current type cast has no effect there and the value passed to the > kernel is still 0. Probably an issue related to the compiler, since I'm assuming > the same configuration works on a similar setup on x86. > > Also ensure consistency with previous type cast in TRACE message. > > Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com> > --- > nbd.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/nbd.c b/nbd.c > index 91b7d56..da81a5b 100644 > --- a/nbd.c > +++ b/nbd.c > @@ -681,7 +681,7 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size) > > TRACE("Setting size to %zd block(s)", (size_t)(size / BDRV_SECTOR_SIZE)); > > - if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / (size_t)BDRV_SECTOR_SIZE) < 0) { > + if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) { > int serrno = errno; > LOG("Failed setting size (in blocks)"); > return -serrno; > Thanks, queued for 2.4. Cc: qemu-stable@nongnu.org
03.04.2015 17:15, Paolo Bonzini wrote: > On 03/04/2015 13:01, Bogdan Purcareata wrote: ... >> - if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / (size_t)BDRV_SECTOR_SIZE) < 0) { >> + if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) { >> int serrno = errno; Hmm.. I don't think this is right at all. Now we compare size_t with zero, the result is always false, and set error if ioctl return anything other than 0, including any positive value. Compare: if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE)) < 0) I think the latter is the right version. So much for trivial... ;) Thanks, /mjt
On 03.04.2015 17:44, Michael Tokarev wrote: > 03.04.2015 17:15, Paolo Bonzini wrote: >> On 03/04/2015 13:01, Bogdan Purcareata wrote: > ... >>> - if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / (size_t)BDRV_SECTOR_SIZE) < 0) { >>> + if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) { >>> int serrno = errno; > > Hmm.. I don't think this is right at all. Now we compare size_t with zero, > the result is always false, and set error if ioctl return anything other than 0, > including any positive value. > > Compare: > > if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) > if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE)) < 0) > > I think the latter is the right version. Yes, the latter is indeed the right version. Great catch! > So much for trivial... ;) Me and my slippery fingers. Sorry for the mishap. Bogdan P. > Thanks, > > /mjt >
diff --git a/nbd.c b/nbd.c index 91b7d56..da81a5b 100644 --- a/nbd.c +++ b/nbd.c @@ -681,7 +681,7 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size) TRACE("Setting size to %zd block(s)", (size_t)(size / BDRV_SECTOR_SIZE)); - if (ioctl(fd, NBD_SET_SIZE_BLOCKS, size / (size_t)BDRV_SECTOR_SIZE) < 0) { + if (ioctl(fd, NBD_SET_SIZE_BLOCKS, (size_t)(size / BDRV_SECTOR_SIZE) < 0)) { int serrno = errno; LOG("Failed setting size (in blocks)"); return -serrno;
This fixes ioctl behavior on powerpc e6500 platforms with 64bit kernel and 32bit userspace. The current type cast has no effect there and the value passed to the kernel is still 0. Probably an issue related to the compiler, since I'm assuming the same configuration works on a similar setup on x86. Also ensure consistency with previous type cast in TRACE message. Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com> --- nbd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)