| Message ID | 20200130152218.7600-3-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | request_alignment vs file size | expand |
On 30.01.20 16:22, Vladimir Sementsov-Ogievskiy wrote: > If we failed to probe request_align, we fallback to max_align. But > this is wrong, if file size is not aligned to our max_align. Let's > instead chose alignment so that file size is a multiple of it. It’s entirely possible that the file size is not aligned to the request alignment, though. If so, this patch will make the whole file inaccessible. > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > --- > block/file-posix.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index 7f366046c2..e9c4e509f6 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -385,11 +385,25 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) > align = alignments[i]; > if (raw_is_io_aligned(fd, buf, align)) { > /* Fallback to safe value. */ > - bs->bl.request_alignment = (align != 1) ? align : max_align; > + bs->bl.request_alignment = align; The comment above is now outdated. Max > break; > } > } > qemu_vfree(buf); > + > + if (bs->bl.request_alignment == 1) { > + /* > + * Succeed to read with alignment = 1. But it may be unallocated > + * area on XFS, and we'll fail later if keep request_alignment = 1. > + * > + * Chose safer alignment, keeping in mind file size if possible. > + */ > + > + int64_t len = raw_getlength_fd(bs, fd); > + > + bs->bl.request_alignment = > + len <= 0 ? max_align : MIN(max_align, len & ~(len - 1)); > + } > } > > if (!s->buf_align) { >
diff --git a/block/file-posix.c b/block/file-posix.c index 7f366046c2..e9c4e509f6 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -385,11 +385,25 @@ static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) align = alignments[i]; if (raw_is_io_aligned(fd, buf, align)) { /* Fallback to safe value. */ - bs->bl.request_alignment = (align != 1) ? align : max_align; + bs->bl.request_alignment = align; break; } } qemu_vfree(buf); + + if (bs->bl.request_alignment == 1) { + /* + * Succeed to read with alignment = 1. But it may be unallocated + * area on XFS, and we'll fail later if keep request_alignment = 1. + * + * Chose safer alignment, keeping in mind file size if possible. + */ + + int64_t len = raw_getlength_fd(bs, fd); + + bs->bl.request_alignment = + len <= 0 ? max_align : MIN(max_align, len & ~(len - 1)); + } } if (!s->buf_align) {
If we failed to probe request_align, we fallback to max_align. But this is wrong, if file size is not aligned to our max_align. Let's instead chose alignment so that file size is a multiple of it. Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> --- block/file-posix.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)