On Tue, May 27, 2014 at 03:14:41PM +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 <ak...@redhat.com> > --- > tests/virtio-blk-test.c | 55 > +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 55 insertions(+) > > diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c > index 0fdec01..54d1272 100644 > --- a/tests/virtio-blk-test.c > +++ b/tests/virtio-blk-test.c > @@ -7,11 +7,65 @@ > * 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 exec_hmp_cmd(const char *cmd, const char *expected_ret)
This is a generic function, should it be moved to libqtest.c? > +{ > + QDict *response; > + const char *response_return; > + > + response = qmp("{\"execute\": \"human-monitor-command\"," > + " \"arguments\": {" > + " \"command-line\": \"%s\"" > + "}}", cmd); My only worry if we make this function generic is that cmd isn't escaped so callers need to be careful. Maybe something like g_strescape() should be used: https://developer.gnome.org/glib/2.37/glib-String-Utility-Functions.html#g-strescape > + g_assert(response); > + response_return = qdict_get_try_str(response, "return"); > + g_assert(response_return); > + g_assert(strcmp(response_return, expected_ret) == 0); Please use g_assert_cmpstr() so the error message will be pretty-printed with the values of response_return and expected_ret: https://developer.gnome.org/glib/stable/glib-Testing.html#g-assert-cmpstr > + QDECREF(response); > +} > +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 0x%s if=none,file=/dev/null,id=drv-%s", > + addr, addr); Does the address matter with if=none? I think you can just do "drive_add 0 if=none,...". > + 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); > + exec_hmp_cmd(cmd, ""); > + } > + } > + > + /* hot-unplug doesn't work without cooperation of guest OS */ We could implement hotplug/hotunplug in libqos, PCI support already exists: tests/libqos/pci.h. But I guess this test is useful too, it checks the case where the guest is not cooperating?