From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56917) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsPyz-0008UD-Hw for qemu-devel@nongnu.org; Thu, 05 Jun 2014 01:13:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsPyu-0000g7-Lu for qemu-devel@nongnu.org; Thu, 05 Jun 2014 01:12:57 -0400 Received: from mx1.redhat.com ([209.132.183.28]:29978) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsPyu-0000fs-9h for qemu-devel@nongnu.org; Thu, 05 Jun 2014 01:12:52 -0400 Date: Thu, 5 Jun 2014 13:12:46 +0800 From: Amos Kong Message-ID: <20140605051246.GA21825@z.redhat.com> References: <1401174881-32731-1-git-send-email-akong@redhat.com> <1401174881-32731-3-git-send-email-akong@redhat.com> <20140604153506.GM11073@stefanha-thinkpad.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20140604153506.GM11073@stefanha-thinkpad.redhat.com> Subject: Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: afaerber@suse.de, qemu-devel@nongnu.org, stefanha@redhat.com, mst@redhat.com 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 > > --- > > 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 > > #include > > #include > > #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 [[:]:] 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.