From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37272) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsRSl-0002tm-8x for qemu-devel@nongnu.org; Thu, 05 Jun 2014 02:47:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsRSf-00039g-An for qemu-devel@nongnu.org; Thu, 05 Jun 2014 02:47:47 -0400 Received: from oxygen.pond.sub.org ([144.76.244.19]:53091) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsRSf-00039Q-28 for qemu-devel@nongnu.org; Thu, 05 Jun 2014 02:47:41 -0400 From: Markus Armbruster 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> <20140605051246.GA21825@z.redhat.com> <20140605054008.GC21825@z.redhat.com> Date: Thu, 05 Jun 2014 08:47:37 +0200 In-Reply-To: <20140605054008.GC21825@z.redhat.com> (Amos Kong's message of "Thu, 5 Jun 2014 13:40:08 +0800") Message-ID: <87d2eng8dy.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain 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: Amos Kong Cc: Stefan Hajnoczi , mst@redhat.com, afaerber@suse.de, stefanha@redhat.com, qemu-devel@nongnu.org Amos Kong writes: > On Thu, Jun 05, 2014 at 01:12:46PM +0800, Amos Kong wrote: >> 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. > > pci addr is assigned in device_add command, and it contains drive's id. > I tried to just use "drive_add 0 if=none,...", it also works. So does (qemu) drive_add "" if=none,file=tmp.qcow2 OK or even (qemu) drive_add "I'm a little teapot" if=none,file=tmp.qcow2 OK because the first argument is ignored. Fortunately we're getting close to replacing this badly designed command.