Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
On Sun, Jun 08, 2014 at 05:57:11PM +0300, Michael S. Tsirkin wrote:
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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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 [[domain:]bus:]slot
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.
Which tree should this go in through?
The V4 had been reviewed by Stefan.
http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg02063.html
The file (tests/virtio-blk-test.c) was added and applied by Andreas.
Andreas, do you plan to apply the V4?
--
Amos.
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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 [[domain:]bus:]slot
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.
Which tree should this go in through?
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
Amos Kong ak...@redhat.com 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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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 [[domain:]bus:]slot
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.
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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?
+{
+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
+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,
+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?
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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 [[domain:]bus:]slot
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.
Re: [Qemu-devel] [PATCH 2/2] virtio-blk-test.c: add hotplug subtest
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 ak...@redhat.com
---
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 stdio.h
#include glib.h
#include string.h
#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 [[domain:]bus:]slot
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.
--
Amos.
