Re: [Qemu-devel] [PATCH v2 3/4] virtio-blk-test.c: add hotplug subtest

[Stefan Hajnoczi]

On Thu, Jun 05, 2014 at 01:45:17PM +0800, Amos Kong wrote:
> This patch adds a new subtest, it hotplugs 29 * 8 = 232 virtio-blk
> devices to guest, and try to hot-unplug them.
> 
> Note: the hot-unplug can't work without cooperation of guest OS.
> 
> Signed-off-by: Amos Kong <address@hidden>
> ---
>  tests/virtio-blk-test.c | 38 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 38 insertions(+)
> 
> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
> index 0fdec01..616599a 100644
> --- a/tests/virtio-blk-test.c
> +++ b/tests/virtio-blk-test.c
> @@ -7,11 +7,48 @@
>   * See the COPYING file in the top-level directory.
>   */
>  
> +#include <stdio.h>
>  #include <glib.h>
>  #include <string.h>
>  #include "libqtest.h"
>  #include "qemu/osdep.h"
>  
> +static void test_blk_hotplug(void)
> +{
> +    char addr[6];
> +    char cmd[100];
> +    int i, j;
> +
> +    /* start with no network/block device, slots 3~0x1f are free */
> +    qtest_start("-net none");
> +
> +    for (i = 3; i <= 0x1f; i++) {
> +        for (j = 7; j >= 0; j--) {
> +            sprintf(addr, "%x.%x", i, j);
> +            sprintf(cmd, "drive_add 0 if=none,file=/dev/null,id=drv-%s", 
> addr);
> +            qmp_exec_hmp_cmd(cmd, "OK\r\n");
> +
> +            sprintf(cmd, "device_add virtio-blk-pci,id=dev-%s,drive=drv-%s,"
> +                         "addr=0x%s,multifunction=on", addr, addr, addr);
> +            qmp_exec_hmp_cmd(cmd, "");
> +        }
> +    }
> +
> +    /* hot-unplug doesn't work without cooperation of guest OS */
> +    for (i = 3; i <= 0x1f; i++) {
> +        for (j = 7; j >= 0; j--) {
> +            sprintf(addr, "%x.%x", i, j);
> +            sprintf(cmd, "drive_del drv-%s", addr);
> +            qmp_exec_hmp_cmd(cmd, "");
> +
> +            sprintf(cmd, "device_del dev-%s", addr);
> +            qmp_exec_hmp_cmd(cmd, "");
> +        }
> +    }

All of this sprintf() usage makes me wonder about:
qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);

This way callers don't need to manage formatting buffers themselves.

Besides that:
Reviewed-by: Stefan Hajnoczi <address@hidden>