Am 18.06.2014 08:40, schrieb Amos Kong:
> On Tue, Jun 17, 2014 at 03:25:34PM +0200, Andreas Färber wrote:
>> Am 06.06.2014 16:33, schrieb Amos Kong:
>>> 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>
>>> Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
>>> ---
>>> 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.
>>> */
>>>
>>> +#include <stdio.h>
>>> #include <glib.h>
>>> #include <string.h>
>>> #include "libqtest.h"
>>> #include "qemu/osdep.h"
>>>
>>> +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 = 3; i <= 0x1f; i++) {
>>> + for (j = 7; j >= 0; j--) {
>>> + qmp_exec_hmp_cmd("OK\r\n",
>>> + "drive_add 0
>>> if=none,file=/dev/null,id=drv-%x.%x",
>>> + i, j);
>>> + qmp_exec_hmp_cmd("",
>>> + "device_add
>>> virtio-blk-pci,id=dev-%x.%x,drive=drv-%x.%x,"
>>> + "addr=0x%x.%x,multifunction=on", i, j, i, j, i, j);
>
> Hi Andreas,
>
>
>> Why are you using HMP-via-QMP here and not QMP directly?
>
> I referenced existed test code.
>
>>> + }
>>> + }
>>> +
>>> + /* hot-unplug doesn't work without cooperation of guest OS */
>>> + for (i = 3; i <= 0x1f; i++) {
>>> + for (j = 7; j >= 0; j--) {
>>
>> While the function is still small, using a define or static const would
>> be a small improvement. :) Could be a follow-up of course.
>
> Sorry I don't get it.
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 = 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
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
