Am 06.02.2015 um 18:37 hat Denis V. Lunev geschrieben: > Signed-off-by: Denis V. Lunev <d...@openvz.org> > CC: Paolo Bonzini <pbonz...@redhat.com> > CC: Kevin Wolf <kw...@redhat.com> > --- > block.c | 10 +++++----- > block/raw-posix.c | 16 ++++++++-------- > 2 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/block.c b/block.c > index d45e4dd..e98d651 100644 > --- a/block.c > +++ b/block.c > @@ -225,8 +225,8 @@ static void bdrv_io_limits_intercept(BlockDriverState *bs, > size_t bdrv_opt_mem_align(BlockDriverState *bs) > { > if (!bs || !bs->drv) { > - /* 4k should be on the safe side */ > - return 4096; > + /* page size should be on the safe side */ > + return getpagesize();
Can we make this MAX(4096, getpagesize())? The 4k aren't chosen because of buffer alignment in RAM, but because of a disk sector size of 4k is the highest common value. > } > > return bs->bl.opt_mem_alignment; > @@ -543,7 +543,7 @@ void bdrv_refresh_limits(BlockDriverState *bs, Error > **errp) > bs->bl.max_transfer_length = bs->file->bl.max_transfer_length; > bs->bl.opt_mem_alignment = bs->file->bl.opt_mem_alignment; > } else { > - bs->bl.opt_mem_alignment = 512; > + bs->bl.opt_mem_alignment = BDRV_SECTOR_SIZE; I wouldn't make this conversion. The value happens to be the same, but BDRV_SECTOR_SIZE is just the arbitrary unit that the block layer uses internally for things like sector_num/nb_sectors. The 512 in this place, however, is the minimum alignment that hardware could require, and logically independent of BDRV_SECTOR_SIZE. Similar considerations apply to the other conversions of 512 in this patch. They would all be better left as literal 512 (unless you want to introduce new constants for their specific purpose). > } > > if (bs->backing_hd) { > @@ -965,8 +965,8 @@ static int bdrv_open_common(BlockDriverState *bs, > BlockDriverState *file, > } > > bs->open_flags = flags; > - bs->guest_block_size = 512; > - bs->request_alignment = 512; > + bs->guest_block_size = BDRV_SECTOR_SIZE; > + bs->request_alignment = BDRV_SECTOR_SIZE; > bs->zero_beyond_eof = true; > open_flags = bdrv_open_flags(bs, flags); > bs->read_only = !(open_flags & BDRV_O_RDWR); > diff --git a/block/raw-posix.c b/block/raw-posix.c > index 853ffa6..9848f83 100644 > --- a/block/raw-posix.c > +++ b/block/raw-posix.c > @@ -122,7 +122,6 @@ > reopen it to see if the disk has been changed */ > #define FD_OPEN_TIMEOUT (1000000000) > > -#define MAX_BLOCKSIZE 4096 > > typedef struct BDRVRawState { > int fd; > @@ -222,6 +221,7 @@ static void raw_probe_alignment(BlockDriverState *bs, int > fd, Error **errp) > BDRVRawState *s = bs->opaque; > char *buf; > unsigned int sector_size; > + size_t page_size = getpagesize(); > > /* For /dev/sg devices the alignment is not really used. > With buffered I/O, we don't have any restrictions. */ > @@ -264,9 +264,9 @@ static void raw_probe_alignment(BlockDriverState *bs, int > fd, Error **errp) > /* If we could not get the sizes so far, we can only guess them */ > if (!s->buf_align) { > size_t align; > - buf = qemu_memalign(MAX_BLOCKSIZE, 2 * MAX_BLOCKSIZE); > - for (align = 512; align <= MAX_BLOCKSIZE; align <<= 1) { > - if (pread(fd, buf + align, MAX_BLOCKSIZE, 0) >= 0) { > + buf = qemu_memalign(page_size, 2 * page_size); > + for (align = BDRV_SECTOR_SIZE; align <= page_size; align <<= 1) { > + if (pread(fd, buf + align, page_size, 0) >= 0) { > s->buf_align = align; > break; > } Here, I'd prefer MAX(getpagesize(), MAX_BLOCKSIZE) as well. Kevin