On Fri, Jan 25, 2019 at 07:07:35AM +0100, Thomas Huth wrote: > On 2019-01-25 07:01, Thomas Huth wrote: > > On 2019-01-24 18:23, Stefano Garzarella wrote: > >> If the WRITE_ZEROES feature is enabled, we check this > >> command in the test_basic(). > >> > >> Signed-off-by: Stefano Garzarella <sgarz...@redhat.com> > >> --- > >> tests/virtio-blk-test.c | 63 +++++++++++++++++++++++++++++++++++++++++ > >> 1 file changed, 63 insertions(+) > >> > >> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > >> index 04c608764b..8cabbcb85a 100644 > >> --- a/tests/virtio-blk-test.c > >> +++ b/tests/virtio-blk-test.c > >> @@ -231,6 +231,69 @@ static void test_basic(QVirtioDevice *dev, > >> QGuestAllocator *alloc, > >> > >> guest_free(alloc, req_addr); > >> > >> + if (features & (1u << VIRTIO_BLK_F_WRITE_ZEROES)) { > >> + struct virtio_blk_discard_write_zeroes *dwz_hdr; > >> + void *expected; > >> + > >> + /* > >> + * WRITE_ZEROES request on the same sector of previous test where > >> + * we wrote "TEST". > >> + */ > >> + req.type = VIRTIO_BLK_T_WRITE_ZEROES; > >> + req.data = g_malloc0(512); > > > > Wouldn't it be more interesting to do a memset(req.data, 0xaa, 512) or > > something similar here, to see whether zeroes or 0xaa is written? > > Ah, never mind, I thought req.data would be a sector buffer here, but > looking at the lines below, it apparently is something different. > > Why do you allocate 512 bytes here? I'd rather expect > g_malloc0(sizeof(struct virtio_blk_discard_write_zeroes)) here. ... and > then you could also use a local "struct virtio_blk_discard_write_zeroes > dwz_hdr" variable instead of a pointer, and drop the g_malloc0() completely? >
Hi Thomas, it was my initial implementation, but on the first test I discovered that virtio_blk_request() has an assert on the data_size and it requires a multiple of 512 bytes. Then I looked at the virtio-spec #1, and it seems that data should be multiple of 512 bytes also if it contains the struct virtio_blk_discard_write_zeroes. (I'm not sure) Anyway I tried to allocate only the space for that struct, commented the assert and the test works well. How do you suggest to proceed? [1](https://github.com/oasis-tcs/virtio-spec/blob/master/content.tex#L3944) Thanks, Stefano