Re: [PATCH v7 4/4] tests/qtest: Introduce tests for UFS

[Thomas Huth <thuth@redhat.com>]

  Hi!

On 26/07/2023 07.30, Jeuk Kim wrote:
> This patch includes the following tests
>    Test mmio read
>    Test ufs device initialization and ufs-lu recognition
>    Test I/O (Performs a write followed by a read to verify)
> 
> Signed-off-by: Jeuk Kim <jeuk20.kim@samsung.com>
> ---
...
> diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
> new file mode 100644
> index 0000000000..5104a0a56a
> --- /dev/null
> +++ b/tests/qtest/ufs-test.c
> @@ -0,0 +1,575 @@
> +/*
> + * QTest testcase for UFS
> + *
> + * Copyright (c) 2023 Samsung Electronics Co., Ltd. All rights reserved.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qemu/units.h"
> +#include "libqtest.h"
> +#include "libqos/qgraph.h"
> +#include "libqos/pci.h"
> +#include "scsi/constants.h"
> +#include "include/block/ufs.h"
> +
> +/* Test images sizes in Bytes */
> +#define TEST_IMAGE_SIZE (64 * 1024 * 1024)
> +/* Timeout for various operations, in seconds. */
> +#define TIMEOUT_SECONDS 5

 From what I've seen in the past, it's possible that a process gets paused 
for 3 - 4 seconds on a very loaded CI machine, so 5 seconds is already close 
... I'd suggest to use 8 - 10 seconds for a timeout instead, just to be on 
the safe side.

> +static char *drive_create(void)
> +{
> +    int fd, ret;
> +    char *t_path;
> +
> +    /* Create a temporary raw image */
> +    fd = g_file_open_tmp("qtest.XXXXXX", &t_path, NULL);

Could you maybe use "qtest-ufs.XXXXXX" or something more prominent instead? 
... in case the files don't get deleted correctly, it's easier to know at 
which test to look at later.

With that change:
Acked-by: Thomas Huth <thuth@redhat.com>