On Wed, Jun 04, 2014 at 05:35:06PM +0200, Stefan Hajnoczi wrote:
> 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?
OK
>
> > +{
> > + 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
OK
> > + 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,...".
drive_add [[<domain>:]<bus>:]<slot>
multiple function is enabled, slot parameter is needed.
> > + 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?
OK, we can update and merge this test first, I will investigate and
write a new subtest with PCI support.
--
Amos.
