[ENBD] [gmane.network.aoe.aoetools.general] enbd and 4TB

[Peter T. Breuer]

"Also sprach Florian Frank:"
> >   err = enbd_set_size (lo, longlongval << 9);
> > 
> > and we might be even further on track.
> 
> grep nda /proc/partitions
> 43     0 3906140160 nda

Is that right? I think it counts in KB, so it looks OK-ish. 3.9G KB.

> But if I access the device with mkfs.xfs I get in dmesg:
> 
> ENBD #2475[1]: do_enbd_request overrange request
> Buffer I/O error on device nda, logical block 3662006403

:-). OK, let's look at line 2475 ...

  if (req->sector + req->nr_sectors > lo->sectors) {
          ENBD_FAIL ("overrange request");
  }

However, lo->sectors is 64 bit ...

      u64 sectors;                         /* PTB add - device size (sectors) */

and the other values come straight from the fields of the kernel
request passed to the driver.  The lo->sectors field is set in the same
call to enbd_set_size() that tells the kernel the size, from the
SET_SECTORS64 ioctl:

   enbd_set_size (struct enbd_device *lo, __u64 arg) {

      ...
      lo->size     = enbd_sizes[nbd << ENBD_SHIFT] = arg >> 10; // KB
      lo->sectors  = lo->size << 1; // 512B
      ...

Uh oh. I think the "size" field is 32 bit:


      unsigned size;                       /* PTB add - device size in blks */


That is, uh, kinda annoying. Still, it should make it all the way to 4TB.
Nevertheless, would you mind changing it to

     u64 size;                          /* PTB add - device size in blks */

(enbd.h, struct enbd_device)

And the toubles aren't over, because the enbd_sizes[] array is 32 bit,
and I think it HAS to be, to fit in the kernels block device arrays.

   static int enbd_sizes[MAX_NBD * ENBD_MAXCONN];

But I don't see where it's registered. Maybe it no longer is. Good. One
can set that to


   static _u64 enbd_sizes[MAX_NBD * ENBD_MAXCONN];

too.

There's a problem setting the fake disk geometry now ... this will
simply never fly! Oh well. In the HDIO_GETGEO ioctl, change

    int sectors = enbd_sizes[nbd << ENBD_SHIFT] << 1;

to

    __u64 sectors = enbd_sizes[nbd << ENBD_SHIFT] << 1;


> Buffer I/O error on device nda, logical block 3662006404
> Buffer I/O error on device nda, logical block 3662006405

If you could arrange for some more printout in the  "overrange .."
message, that would be nice. Something like


    ENBD_ERROR( "overrange request at %Ld-%Ld sectors\n",
          req->sector, req->sector + req->nr_sectors - 1);
    goto error_out;


Hic. There will be a LOT of noise. The original was protected by 
a trigger that suppressed the message if it was repeated next time too.
The implication is that there was some kind of success in the sequence
too, or you would have seen more noise.


> Buffer I/O error on device nda, logical block 3662006406
> Buffer I/O error on device nda, logical block 3662006407
> Buffer I/O error on device nda, logical block 3906140163
> Buffer I/O error on device nda, logical block 3906140164
> Buffer I/O error on device nda, logical block 3906140165
> Buffer I/O error on device nda, logical block 3906140166
> Buffer I/O error on device nda, logical block 3906140167

You know - this isn't necessarily bad. I seem to recall that mke2fs
tries to find the size of the device by binary search. It would perhaps
be useful to add some more info in the "overrange request" message.

Peter