| Message ID | 1402065233-31894-4-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <akong@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@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); Why are you using HMP-via-QMP here and not QMP directly? > + } > + } > + > + /* 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. Test looks good, thanks for your effort. Regards, Andreas > + qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j); > + qmp_exec_hmp_cmd("", "device_del dev-%x.%x", i, j); > + } > + } > + > + qtest_end(); > +} > + > /* Tests only initialization */ > static void virtblk_init(void) > { > @@ -26,6 +56,7 @@ int main(int argc, char **argv) > > g_test_init(&argc, &argv, NULL); > qtest_add_func("/virtio/blk/pci/init", virtblk_init); > + qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug); > > ret = g_test_run(); > >
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 <akong@redhat.com> > > Reviewed-by: Stefan Hajnoczi <stefanha@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. test_blk_hotplug() was already decorated by 'static' and we can't decorate a function that returns nothing. tests/virtio-blk-test.c:16:19: error: function definition has qualified void return type [-Werror] static const void test_blk_hotplug(void) > Test looks good, thanks for your effort. > > Regards, > Andreas Thanks, Amos > > + qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j); > > + qmp_exec_hmp_cmd("", "device_del dev-%x.%x", i, j); > > + } > > + } > > + > > + qtest_end(); > > +} > > + > > /* Tests only initialization */ > > static void virtblk_init(void) > > { > > @@ -26,6 +56,7 @@ int main(int argc, char **argv) > > > > g_test_init(&argc, &argv, NULL); > > qtest_add_func("/virtio/blk/pci/init", virtblk_init); > > + qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug); > > > > ret = g_test_run(); > > > > > > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
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 <akong@redhat.com> >>> Reviewed-by: Stefan Hajnoczi <stefanha@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
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 */ + 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); + } + } + + /* hot-unplug doesn't work without cooperation of guest OS */ + for (i = 3; i <= 0x1f; i++) { + for (j = 7; j >= 0; j--) { + qmp_exec_hmp_cmd("", "drive_del drv-%x.%x", i, j); + qmp_exec_hmp_cmd("", "device_del dev-%x.%x", i, j); + } + } + + qtest_end(); +} + /* Tests only initialization */ static void virtblk_init(void) { @@ -26,6 +56,7 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); qtest_add_func("/virtio/blk/pci/init", virtblk_init); + qtest_add_func("/virtio/blk/pci/hotplug", test_blk_hotplug); ret = g_test_run();