[ENBD] nbd 2.4.27: partition >4GB problem

[Peter T. Breuer]

"A month of sundays ago Tad Kollar wrote:"
> I just got into ENBD the other day... started with nbd 2.4.27 and kernel 
> 2.4.18. It worked fine up until I tried to use a partition larger than 4GB. 
> Above that point the client would always see the same erroneous size[1] (2TB) 
> for the partition, though the server reported[2] sending the correct size 
> (~26GB in this case).

Actually, I guess that's more a feature than a real big bug :-). But
yes, it needs to be fixed. It may have something to do with the support
for removable devices.

OK .. at the server side (nbd-server.c), we do this:


           u64 size_host = 0;
    ...
           size_host = htonll(self->size);
           if (writenet(self, "size ", 5) < 0) {
              PERR("Introduction failed: %m\n");
              err = -5;
              goto fail;
           }
           if (writenet(self, (char*)&size_host, sizeof(size_host)) < 0) {
              PERR("Introduction failed: %m\n");
              err = -5;
              goto fail;
           }
           MSG("server (%d) sent size %Lu ok\n", self->i, self->size);

so we write "size " plus a 64bit network order datum (crazy, but true;
I am going to have to revise this for endianness someday).

On the client side (nbd-client.c), we get ...


            u64 size64;
            ....
             size:
                memcpy(&size64, buf, sizeof(size64));
                size64 = ntohll (size64);

                // PTB set object parm
                self->size = size64;

                DEBUG ("client (%d) got size = %Lu\n", self->i, size64);
                goto next;


so can you show me the client side log output about what it sees
vis-a-vis size data? You might want to change that DEBUG to a MSG.

Did I really make all that clientside information into DEBUG? Maybe
I should change it all to MSG.

The client appears to do the right thing initially with the size info:

       err = setkernel(self,size64,0,self->blksize,
                            ^^^^^^^
                   self->flags&NBD_CLIENT_READONLY,
                   self->pulse_intvl,
                   self->flags&NBD_CLIENT_SHOW_ERRS,
                   self->flags&NBD_CLIENT_MD5SUM,
                   self->flags&NBD_CLIENT_BUFFERWR,
                   self->pid);
       ...
       static int
       setkernel(struct nbd_client* self, u64 size64, int sigbuf[],
                                          ^^^^^^^^^^^
            unsigned blksize, int ro, long pulse_intvl, int show_errs,
            int md5sum, int buffer_writes, int svpd) {
       ...
              if (size64 > 0) {
                if (setdevicesize(self, size64) < 0) {
                  PERR ("Failed set size %Lu on fd %d: %m\n",size64,dev);
                  return -1;
                }
                DEBUG ("client (%d) set size %Lu\n", self->i, self->size);
              }
      ...
      static unsigned
      setdevicesize(struct nbd_client * self, u64 size) {
                                              ^^^^^^^^^
      ...

Oh!!!!

      ...
      
          unsigned sectors = size >> 9;

          // PTB HACK HACK HACK .. why not allow more sectors :-)
          if (size > (u64)(unsigned)(-1)) {
            sectors = (unsigned)(-1);
          }

          DEBUG("client (%d) device not yet set to size\n", self->i);
          err = self->ioctl(self, NBD_SET_SECTORS, (char*)sectors);

Uh. I don't know what to say. It sets the number of sectors to some
number <= 2^55, and then truncates that to 2^32 at most. So the biggest
device it can hope to deal with is 2^41 bytes (2TB). But worse, if
we pass a size that is >= 2^32, then for some reason known not
even to myself, it shifts the sector number up to the maximum possible,
before telling the kernel that we have a whopping big device.

I wonder why? Could I have been testing the behaviour with removable
devices and left that in?

Anyway, remove the 4 lines beginning HACK HACK HACK, and tell me how it
goes.

I wonder what that breaks?


Peter