From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxD4i-000410-9s for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:26:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WxD4Z-0004La-Sp for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:26:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:49691 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WxD4Z-0004LW-N0 for qemu-devel@nongnu.org; Wed, 18 Jun 2014 06:26:31 -0400 Message-ID: <53A16955.9020704@suse.de> Date: Wed, 18 Jun 2014 12:26:29 +0200 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1402065233-31894-1-git-send-email-akong@redhat.com> <1402065233-31894-4-git-send-email-akong@redhat.com> <53A041CE.1060408@suse.de> <20140618064024.GB18929@z.redhat.com> In-Reply-To: <20140618064024.GB18929@z.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 3/4] virtio-blk-test.c: add hotplug subtest List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: stefanha@gmail.com, arei.gonglei@huawei.com, qemu-devel@nongnu.org Am 18.06.2014 08:40, schrieb Amos Kong: > On Tue, Jun 17, 2014 at 03:25:34PM +0200, Andreas F=E4rber wrote: >> Am 06.06.2014 16:33, schrieb Amos Kong: >>> This patch adds a new subtest, it hotplugs 29 * 8 =3D 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 >>> Reviewed-by: Stefan Hajnoczi >>> --- >>> tests/virtio-blk-test.c | 31 +++++++++++++++++++++++++++++++ >>> 1 file changed, 31 insertions(+) >>> >>> diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c >>> index 0fdec01..7358203 100644 >>> --- a/tests/virtio-blk-test.c >>> +++ b/tests/virtio-blk-test.c >>> @@ -7,11 +7,41 @@ >>> * See the COPYING file in the top-level directory. >>> */ >>> =20 >>> +#include >>> #include >>> #include >>> #include "libqtest.h" >>> #include "qemu/osdep.h" >>> =20 >>> +static void test_blk_hotplug(void) >>> +{ >>> + int i, j; >>> + >>> + /* start with no network/block device, slots 3~0x1f are free */ >> >> "3-0x1f" or "3 to 0x1f"? >> >>> + qtest_start("-net none"); >>> + >>> + for (i =3D 3; i <=3D 0x1f; i++) { >>> + for (j =3D 7; j >=3D 0; j--) { >>> + qmp_exec_hmp_cmd("OK\r\n", >>> + "drive_add 0 if=3Dnone,file=3D/dev/null= ,id=3Ddrv-%x.%x", >>> + i, j); >>> + qmp_exec_hmp_cmd("", >>> + "device_add virtio-blk-pci,id=3Ddev-%x.%x,dri= ve=3Ddrv-%x.%x," >>> + "addr=3D0x%x.%x,multifunction=3Don", i, j, i,= j, i, j); >=20 > Hi Andreas, >=20 > =20 >> Why are you using HMP-via-QMP here and not QMP directly? >=20 > I referenced existed test code. > =20 >>> + } >>> + } >>> + >>> + /* hot-unplug doesn't work without cooperation of guest OS */ >>> + for (i =3D 3; i <=3D 0x1f; i++) { >>> + for (j =3D 7; j >=3D 0; j--) { >> >> While the function is still small, using a define or static const woul= d >> be a small improvement. :) Could be a follow-up of course. >=20 > Sorry I don't get it.=20 You are adding two new loops that are supposed to match. My suggestion is to avoid the four magic numbers by using #define MIN_PCI_SLOT 3 or static const int max_pci_slot =3D 0x1f; etc. to enforce they always match and can more easily be tweaked once, e.g., another slot is taken by default. Just a cosmetic cleanup. Cheers, Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg