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?
>> + dwz_hdr = (struct virtio_blk_discard_write_zeroes *)req.data;
>> + dwz_hdr->sector = 0;
>> + dwz_hdr->num_sectors = 1;
>> + dwz_hdr->flags = 0;
>> +
>> + req_addr = virtio_blk_request(alloc, dev, &req, 512);
>> +
>> + g_free(req.data);
>> +
>> + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
>> + qvirtqueue_add(vq, req_addr + 16, 512, false, true);
>> + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
>> +
>> + qvirtqueue_kick(dev, vq, free_head);
>> +
>> + qvirtio_wait_used_elem(dev, vq, free_head, NULL,
>> + QVIRTIO_BLK_TIMEOUT_US);
>> + status = readb(req_addr + 528);
>> + g_assert_cmpint(status, ==, 0);
>> +
>> + guest_free(alloc, req_addr);
>> +
>> + /* Read request to check if the sector contains all zeroes */
>> + req.type = VIRTIO_BLK_T_IN;
>> + req.ioprio = 1;
>> + req.sector = 0;
>> + req.data = g_malloc0(512);
>> +
>> + req_addr = virtio_blk_request(alloc, dev, &req, 512);
>> +
>> + g_free(req.data);
>> +
>> + free_head = qvirtqueue_add(vq, req_addr, 16, false, true);
>> + qvirtqueue_add(vq, req_addr + 16, 512, true, true);
>> + qvirtqueue_add(vq, req_addr + 528, 1, true, false);
>> +
>> + qvirtqueue_kick(dev, vq, free_head);
>> +
>> + qvirtio_wait_used_elem(dev, vq, free_head, NULL,
>> + QVIRTIO_BLK_TIMEOUT_US);
>> + status = readb(req_addr + 528);
>> + g_assert_cmpint(status, ==, 0);
>> +
>> + data = g_malloc(512);
>> + expected = g_malloc0(512);
>> + memread(req_addr + 16, data, 512);
>> + g_assert_cmpmem(data, 512, expected, 512);
>> + g_free(expected);
>> + g_free(data);
>> +
>> + guest_free(alloc, req_addr);
>> + }
>> +
>> if (features & (1u << VIRTIO_F_ANY_LAYOUT)) {
>> /* Write and read with 2 descriptor layout */
>> /* Write request */
>>
>
>
