Re: xbd and large sectors

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

On Sat, Jul 22, 2023 at 07:21:50PM +0200, Christian Kujau wrote:
> On Fri, 21 Jul 2023, Manuel Bouyer wrote:
> > In this case you can just use
> > ftp://asim.lip6.fr/outgoing/bouyer/rel/HEAD/amd64/
> > or wait for a recent-enough build to show up on
> > http://nyftp.netbsd.org/pub/NetBSD-daily/HEAD/
> 
> Thanks, that's helpful of course. So, my options were either to wait until 
> the builds show up, or checkout the latest trunk and build it myself. So, 
> I did both.
> 
> After a few tries (and adding more diskspace for the build), a strange 
> build error[0] and some hours later, a shiny new[1] install kernel was 
> generated, yay.
> 
> Booting this in PV mode, these weird disk errors were back, but at least 
> the disk were now recognized with the correct size (8G):
> 
>  [   1.1300569] xbd0: 8192 MB, 512 bytes/sect x 16777216 sectors
>  [   1.1300569] xbd0: backend features 0xc<PERSISTENT,INDIRECT>
>  [   1.1300569] xbd0d: error reading fsbn 0 (xbd0 bn 0; cn 0 tn 0 sn 0)
>  [   1.1300569] xbd0: dos partition I/O error
>  [   1.1400549] xbd0d: error reading fsbn 0 of 0-31 (xbd0 bn 0; cn 0 tn 0 sn 0)
>  [   1.1400549] xbd0d: error reading fsbn 1 (xbd0 bn 1; cn 0 tn 0 sn 1)
>  [   1.1700597] xbd1: 16384 MB, 512 bytes/sect x 33554432 sectors
>  [   1.1700597] xbd1: backend features 0xc<PERSISTENT,INDIRECT>
>  [   1.1700597] xbd1d: error reading fsbn 0 (xbd1 bn 0; cn 0 tn 0 sn 0)
>  [   1.1700597] xbd1: dos partition I/O error
> 
> Hope! But, the *partition* step in sysinst failed again:
> 
>     Status: Command failed
>     Command: disklabel -w -r -f /tmp/disklabel.6598 xbd0 'xbd0' 'default label'
>     Hit enter to continue
>     disklabel: ioctl DIOCWDINFO: Input/output error

OK this is probaly because the domU tries an I/O smaller than 4k, or
not aligned to 4k. The xbd driver should really report 4k block size and not
512.

Please try the attached patch (or use binaries at
ftp://asim.lip6.fr/outgoing/bouyer/rel/HEAD/amd64 which were built with the
patch).
I tested that it still works with a 512-bytes backend but I have to way to
test 4k. 

The patch assumes that b_rawblkno is in DEV_BSIZE units, and that the backend
wants sector_number in XEN_BSIZE units (which happens to be the same as
DEV_BSIZE), but I may have misread the linux sources.

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--

Index: sys/arch/xen/xen/xbd_xenbus.c
===================================================================
RCS file: /cvsroot/src/sys/arch/xen/xen/xbd_xenbus.c,v
retrieving revision 1.133
diff -u -p -u -r1.133 xbd_xenbus.c
--- sys/arch/xen/xen/xbd_xenbus.c	21 Jul 2023 11:28:50 -0000	1.133
+++ sys/arch/xen/xen/xbd_xenbus.c	24 Jul 2023 10:51:59 -0000
@@ -169,7 +169,7 @@ struct xbd_xenbus_softc {
 #define BLKIF_SHUTDOWN_REMOTE 1 /* backend-initiated shutdown in progress */
 #define BLKIF_SHUTDOWN_LOCAL  2 /* locally-initiated shutdown in progress */
 
-	uint64_t sc_sectors; /* number of XEN_BSIZE sectors for this device */
+	uint64_t sc_sectors; /* number of sc_secsize sectors for this device */
 	u_long sc_secsize; /* sector size */
 	uint64_t sc_xbdsize; /* size of disk in DEV_BSIZE */
 	u_long sc_info; /* VDISK_* */
@@ -674,12 +674,12 @@ xbd_backend_changed(void *arg, XenbusSta
 		xbd_connect(sc);
 		sc->sc_shutdown = BLKIF_SHUTDOWN_RUN;
 		sc->sc_xbdsize =
-		    sc->sc_sectors * (uint64_t)XEN_BSIZE / DEV_BSIZE;
+		    sc->sc_sectors * (uint64_t)sc->sc_secsize / DEV_BSIZE;
 		dg = &sc->sc_dksc.sc_dkdev.dk_geom;
 		memset(dg, 0, sizeof(*dg));
 
-		dg->dg_secperunit = sc->sc_xbdsize;
-		dg->dg_secsize = DEV_BSIZE;
+		dg->dg_secperunit = sc->sc_sectors;
+		dg->dg_secsize = sc->sc_secsize;
 		dg->dg_ntracks = 1;
 		// XXX: Ok to hard-code DEV_BSIZE?
 		dg->dg_nsectors = 1024 * (1024 / dg->dg_secsize);
@@ -739,14 +739,6 @@ xbd_connect(struct xbd_xenbus_softc *sc)
 		panic("%s: can't read number from %s/virtual-device\n",
 		    device_xname(sc->sc_dksc.sc_dev),
 		    sc->sc_xbusd->xbusd_otherend);
-	err = xenbus_read_ull(NULL,
-	    sc->sc_xbusd->xbusd_otherend, "sectors", &sectors, 10);
-	if (err)
-		panic("%s: can't read number from %s/sectors\n",
-		    device_xname(sc->sc_dksc.sc_dev),
-		    sc->sc_xbusd->xbusd_otherend);
-	sc->sc_sectors = sectors;
-
 	err = xenbus_read_ul(NULL,
 	    sc->sc_xbusd->xbusd_otherend, "info", &sc->sc_info, 10);
 	if (err)
@@ -760,6 +752,14 @@ xbd_connect(struct xbd_xenbus_softc *sc)
 		    device_xname(sc->sc_dksc.sc_dev),
 		    sc->sc_xbusd->xbusd_otherend);
 
+	err = xenbus_read_ull(NULL,
+	    sc->sc_xbusd->xbusd_otherend, "sectors", &sectors, 10);
+	if (err)
+		panic("%s: can't read number from %s/sectors\n",
+		    device_xname(sc->sc_dksc.sc_dev),
+		    sc->sc_xbusd->xbusd_otherend);
+	sc->sc_sectors = sectors * (uint64_t)XEN_BSIZE / sc->sc_secsize;
+
 	xenbus_switch_state(sc->sc_xbusd, NULL, XenbusStateConnected);
 }
 
@@ -1274,7 +1274,8 @@ xbd_diskstart_submit(struct xbd_xenbus_s
 	req->id = req_id;
 	req->operation =
 	    bp->b_flags & B_READ ? BLKIF_OP_READ : BLKIF_OP_WRITE;
-	req->sector_number = bp->b_rawblkno + (start >> XEN_BSHIFT);
+	req->sector_number = (bp->b_rawblkno * DEV_BSIZE / XEN_BSIZE) +
+	    (start >> XEN_BSHIFT);
 	req->handle = sc->sc_handle;
 
 	size = uimin(bp->b_bcount - start, XBD_MAX_CHUNK);
@@ -1331,7 +1332,7 @@ xbd_diskstart_submit_indirect(struct xbd
 	req->operation = BLKIF_OP_INDIRECT;
 	req->indirect_op =
 	    bp->b_flags & B_READ ? BLKIF_OP_READ : BLKIF_OP_WRITE;
-	req->sector_number = bp->b_rawblkno;
+	req->sector_number = bp->b_rawblkno * DEV_BSIZE / XEN_BSIZE;
 	req->handle = sc->sc_handle;
 
 	xbdreq->req_indirect = SLIST_FIRST(&sc->sc_indirect_head);