Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

false physical blocksize detected #104

Closed
mraerino opened this issue Apr 1, 2021 · 4 comments · Fixed by #107
Closed

false physical blocksize detected #104

mraerino opened this issue Apr 1, 2021 · 4 comments · Fixed by #107

Comments

@mraerino
Copy link

mraerino commented Apr 1, 2021

I have a cdrom in a VM that is emulated from an iso on the host system.

in the VM, fdisk reports these block sizes:

$ sudo fdisk -l /dev/sr0 
Disk /dev/sr0: 4 MiB, 4194304 bytes, 2048 sectors
Disk model: QEMU DVD-ROM    
Units: sectors of 1 * 2048 = 2048 bytes
Sector size (logical/physical): 2048 bytes / 2048 bytes
I/O size (minimum/optimal): 2048 bytes / 2048 bytes

while diskfs is detecting a different physical block size:

disk, _ := diskfs.OpenWithMode("/dev/sr0", diskfs.ReadOnly)
fs, _ := disk.GetFilesystem(0)
DEBU[0000] checking device: /dev/sr0                    
DEBU[0000] initDisk(): start                            
DEBU[0000] initDisk(): block device                     
DEBU[0000] initDisk(): logical block size 2048, physical block size 4096 
DEBU[0000] trying fat32                                 
DEBU[0000] fat32 failed: blocksize for FAT32 must be either 512 bytes or 0, not 2048 
DEBU[0000] trying iso9660 with physical block size 4096 

what could cause this? is there something i can tune so this can't happen?


related to #103

@deitch
Copy link
Collaborator

deitch commented Apr 1, 2021

The sector sizes are determined here. If you dig in, you can see that it determines the device type (correctly, a block device, when you give it /dev/sr0), and then it calls getSectorSizes (I linked to the Unix one, since, as far as I can tell, you are running on a Unix-like system):

func getSectorSizes(f *os.File) (int64, int64, error) {
	fd := f.Fd()
	logicalSectorSize, err := unix.IoctlGetInt(int(fd), blksszGet)
	if err != nil {
		return 0, 0, fmt.Errorf("Unable to get device logical sector size: %v", err)
	}
	physicalSectorSize, err := unix.IoctlGetInt(int(fd), blkbszGet)
	if err != nil {
		return 0, 0, fmt.Errorf("Unable to get device physical sector size: %v", err)
	}
	return int64(logicalSectorSize), int64(physicalSectorSize), nil
}

I find it interesting that fdisk returned a different value than the ioctl calls, as I think that fdisk does those ioctl calls. It is worth looking into their source code.

The source for fdisk listing (fdisk -l) is here

@deitch
Copy link
Collaborator

deitch commented Apr 1, 2021

Yup, code starts here:

/*
 * Get physical block device size. The BLKPBSZGET is supported since Linux
 * 2.6.32. For old kernels is probably the best to assume that physical sector
 * size is the same as logical sector size.
 *
 * Example:
 *
 * rc = blkdev_get_physector_size(fd, &physec);
 * if (rc || physec == 0) {
 *	rc = blkdev_get_sector_size(fd, &physec);
 *	if (rc)
 *		physec = DEFAULT_SECTOR_SIZE;
 * }
 */
#ifdef BLKPBSZGET
int blkdev_get_physector_size(int fd, int *sector_size)
{
	if (ioctl(fd, BLKPBSZGET, &sector_size) >= 0)
		return 0;
	return -1;
}
#else
int blkdev_get_physector_size(int fd __attribute__((__unused__)), int *sector_size)
{
	*sector_size = DEFAULT_SECTOR_SIZE;
	return 0;
}
#endif

And the definitions here:

/* block device topology ioctls, introduced in 2.6.32 (commit ac481c20) */
# ifndef BLKIOMIN
#  define BLKIOMIN   _IO(0x12,120)
#  define BLKIOOPT   _IO(0x12,121)
#  define BLKALIGNOFF _IO(0x12,122)
#  define BLKPBSZGET _IO(0x12,123)
# endif

I wonder if it is being misset on different platforms, because we have it fixed here:

	blkbszGet                        = 0x80081270

It is possible, especially in a virtualized environment (I saw you are running in qemu).

Hmm, oh hold on. Even with a fixed number, I am seeing it as we set it to the value of BLKBSZGET and not BLKPBSZGET, which are different values.

Let's start with a branch that sets it to the "right" fixed value for now, then let's see about getting it dynamically determined.

@deitch
Copy link
Collaborator

deitch commented Apr 1, 2021

@mraerino can you try against this branch and see what you get? https://github.com/diskfs/go-diskfs/tree/pbsize

@mraerino
Copy link
Author

mraerino commented Apr 2, 2021

works! #107 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants