[Qemu-devel] [PATCH] nbd/trivial: fix type cast for ioctl

[Qemu-devel] [PATCH] nbd/trivial: fix type cast for ioctl

[Bogdan Purcareata]

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;
-- 
2.1.4

Re: [Qemu-devel] [PATCH] nbd/trivial: fix type cast for ioctl

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH] nbd/trivial: fix type cast for ioctl

[Michael Tokarev]

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

Re: [Qemu-devel] [PATCH] nbd/trivial: fix type cast for ioctl

[Purcareata Bogdan]

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
>