Am 30.01.2020 um 12:30 hat Vladimir Sementsov-Ogievskiy geschrieben: > 30.01.2020 14:11, Kevin Wolf wrote: > > Am 30.01.2020 um 11:40 hat Vladimir Sementsov-Ogievskiy geschrieben: > > > 29.01.2020 21:01, Vladimir Sementsov-Ogievskiy wrote: > > > > Hi! > > > > > > > > I found a crash, which may be simply triggered for images unaligned to > > > > request_alignment: > > > > > > > > # ./qemu-io --image-opts -c 'write 0 512' > > > > driver=blkdebug,align=4096,image.driver=null-co,image.size=512 > > > > qemu-io: block/io.c:1505: bdrv_aligned_pwritev: Assertion `end_sector > > > > <= bs->total_sectors || child->perm & BLK_PERM_RESIZE' failed. > > > > Aborted (core dumped) > > > > > > > > The problem is obvious: 512 is aligned to 4096 and becomes larger than > > > > file size. > > > > > > > > I faced it after rebasing our downstream branches to newer Rhel > > > > versions. Seems that after some updates of alignment detection in > > > > file-posix.c, it started to detect 4096 alignment in our build > > > > environment, and iotest 152 started to crash (as it operates on file of > > > > 512 bytes). > > > > > > > > My question is: > > > > > > > > What is wrong? Should we restrict images to be aligned to > > > > request_alignment, or allow unaligned operations at EOF, if file is > > > > unaligned itself? > > > > > > > > > > > > > The problem started with commit > > > > > > commit a6b257a08e3d72219f03e461a52152672fec0612 > > > Author: Nir Soffer <[email protected]> > > > Date: Tue Aug 13 21:21:03 2019 +0300 > > > > > > file-posix: Handle undetectable alignment > > > > > > > > > It sets request_alignment to 4k, if probing of align=1 succeeded.. I > > > think it's wrong logic. It leads to crashes for images unaligned to 4k. > > > > > > If we force alignment to be 4k, we at least should check that file size > > > is aligned to 4k. Otherwise our assumption is definitely wrong. > > > > > > And still, I doubt that it's correct to force alignment to 4k, for > > > devices which doesn't request any alignment.. > > > > What backend is this? O_DIRECT with byte alignment sounds wrong, so I > > wonder if your storage really can do this or whether we just failed to > > detect the actual alignment. > > The problem was disabled odirect in virtuozzo container which lead to byte > alignment. So, yes, it's on our part.
Oh, I see, so to QEMU it looked like it would do O_DIRECT and probing was done, but what was actually opened was non-direct. Not sure if we could possibly distinguish a situation like this from one where O_DIRECT succeeds with byte alignment because the block was unallocated, but would require larger alignment later. > > I guess we could change the default to pick the largest size so that the > > image size is still a multiple of it. But if the image size isn't even > > aligned to 512 bytes, I think refusing to open the image with O_DIRECT > > feels more correct (I would be okay with doing the same with > 512 byte > > images, too, if the image size isn't a multiple of the alignment). > > > > OK, I'll think about a patch for file-posix.c, and may be blkdebug too. > > Also, we need to check it somewhere in generic layer too, to fail earlier > than assertion above. Yes, I agree, it should be checked while opening the image. Kevin
