[Qemu-devel] [PATCH v3 0/4] test virtio-blk hotplug

[Qemu-devel] [PATCH v3 0/4] test virtio-blk hotplug

[Amos Kong]

It's worth to add a hotplug test to qtest, but without
cooperation of guest OS, new devices can't be initialized
by guest, and hot-unplug doesn't work.

However, the new test can cover some part of code of
hotplug/unplug.

I will write another subtest to test hotplug with pci support.

V2: move qmp_exec_hmp_cmd() to libqtest.c
    excape hmp cmd (stefanha)
    use qmp_exec_hmp_cmd() in blockdev-test
V3: use vp_list to format string, free escaped string

Amos Kong (4):
  virtio-blk-test.c: change pci_nop() to virtblk_init()
  qtest: introduce qmp_exec_hmp_cmd()
  virtio-blk-test.c: add hotplug subtest
  qtest: use qmp_exec_hmp_cmd() in blockdev-test

 tests/blockdev-test.c   | 23 ++---------------------
 tests/libqtest.c        | 26 ++++++++++++++++++++++++++
 tests/libqtest.h        |  9 +++++++++
 tests/virtio-blk-test.c | 44 +++++++++++++++++++++++++++++++++++++-------
 4 files changed, 74 insertions(+), 28 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v3 1/4] virtio-blk-test.c: change pci_nop() to virtblk_init()

[Amos Kong]

I want to add a new subtest in virtio-blk-test.c, it will start
guest without network. The original pci_init() did nothing, but
it's good to reserve a very simple initialization testing.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 tests/virtio-blk-test.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c
index d53f875..0fdec01 100644
--- a/tests/virtio-blk-test.c
+++ b/tests/virtio-blk-test.c
@@ -12,9 +12,12 @@
 #include "libqtest.h"
 #include "qemu/osdep.h"
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
-static void pci_nop(void)
+/* Tests only initialization */
+static void virtblk_init(void)
 {
+    qtest_start("-drive id=drv0,if=none,file=/dev/null "
+                "-device virtio-blk-pci,drive=drv0");
+    qtest_end();
 }
 
 int main(int argc, char **argv)
@@ -22,13 +25,9 @@ int main(int argc, char **argv)
     int ret;
 
     g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/virtio/blk/pci/nop", pci_nop);
+    qtest_add_func("/virtio/blk/pci/init", virtblk_init);
 
-    qtest_start("-drive id=drv0,if=none,file=/dev/null "
-                "-device virtio-blk-pci,drive=drv0");
     ret = g_test_run();
 
-    qtest_end();
-
     return ret;
 }
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 1/4] virtio-blk-test.c: change pci_nop() to virtblk_init()

[Stefan Hajnoczi]

On Fri, Jun 06, 2014 at 09:11:57AM +0800, Amos Kong wrote:
> I want to add a new subtest in virtio-blk-test.c, it will start
> guest without network. The original pci_init() did nothing, but
> it's good to reserve a very simple initialization testing.
> 
> Signed-off-by: Amos Kong <akong@redhat.com>
> ---
>  tests/virtio-blk-test.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[Qemu-devel] [PATCH v3 2/4] qtest: introduce qmp_exec_hmp_cmd()

[Amos Kong]

This patch wraps a helper function to execute human command by
one QMP command (human-monitor-command). It also checks the return
string.

Signed-off-by: Amos Kong <akong@redhat.com>
---
 tests/libqtest.c | 26 ++++++++++++++++++++++++++
 tests/libqtest.h |  9 +++++++++
 2 files changed, 35 insertions(+)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 71468ac..c14615c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -646,3 +646,29 @@ void qmp_discard_response(const char *fmt, ...)
     qtest_qmpv_discard_response(global_qtest, fmt, ap);
     va_end(ap);
 }
+
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
+{
+    va_list ap;
+    char cmd[1024];
+    char *escaped_cmd;
+    QDict *response;
+    const char *response_return;
+
+    va_start(ap, fmt);
+    vsprintf(cmd, fmt, ap);
+    va_end(ap);
+
+    escaped_cmd = g_strescape(cmd, NULL);
+    response = qmp("{\"execute\": \"human-monitor-command\","
+                   " \"arguments\": {"
+                   "   \"command-line\": \"%s\""
+                   "}}", escaped_cmd);
+    free(escaped_cmd);
+
+    g_assert(response);
+    response_return = qdict_get_try_str(response, "return");
+    g_assert(response_return);
+    g_assert_cmpstr(response_return, ==, expected_ret);
+    QDECREF(response);
+}
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 8f323c7..d2959d3 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -375,6 +375,15 @@ QDict *qmp(const char *fmt, ...);
 void qmp_discard_response(const char *fmt, ...);
 
 /**
+ * qmp_exec_hmp_cmd:
+ * @expected_ret: expected return string
+ * @fmt...: HMP command to execute
+ *
+ * Executes HMP command by 'human-monitor-command'.
+ */
+void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...);
+
+/**
  * qmp_receive:
  *
  * Reads a QMP message from QEMU and returns the response.
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 2/4] qtest: introduce qmp_exec_hmp_cmd()

[Stefan Hajnoczi]

On Fri, Jun 06, 2014 at 09:11:58AM +0800, Amos Kong wrote:
> +void qmp_exec_hmp_cmd(const char *expected_ret, const char *fmt, ...)
> +{
> +    va_list ap;
> +    char cmd[1024];
> +    char *escaped_cmd;
> +    QDict *response;
> +    const char *response_return;
> +
> +    va_start(ap, fmt);
> +    vsprintf(cmd, fmt, ap);
> +    va_end(ap);
> +
> +    escaped_cmd = g_strescape(cmd, NULL);
> +    response = qmp("{\"execute\": \"human-monitor-command\","
> +                   " \"arguments\": {"
> +                   "   \"command-line\": \"%s\""
> +                   "}}", escaped_cmd);
> +    free(escaped_cmd);

glib uses g_malloc()/g_free() instead of malloc()/free().  Please use
g_free() since the string comes from g_strescape().

[Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[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 */
+    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();
 
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Gonglei (Arei)]

Hi, Amos

> -----Original Message-----
> From: Amos Kong [mailto:akong@redhat.com]
> Sent: Friday, June 06, 2014 9:12 AM
> To: qemu-devel@nongnu.org
> Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> 
> 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 */
> +    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);
> +        }
> +    }

Do the pci multifunction devices support hotplug/unplug yet?

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Amos Kong]

On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> Hi, Amos
> 
> > -----Original Message-----
> > From: Amos Kong [mailto:akong@redhat.com]
> > Sent: Friday, June 06, 2014 9:12 AM
> > To: qemu-devel@nongnu.org
> > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > 
> > 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.


> > +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);
> > +        }
> > +    }
> 
> Do the pci multifunction devices support hotplug/unplug yet?
 
Yes.

You can hotplug single functions one by one, guest will identify
all the functions when function 0 is added.
Hotunplug function 0, all functions in same slot will be removed.

> Best regards,
> -Gonglei

-- 
			Amos.

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Gonglei (Arei)]

> -----Original Message-----
> From: Amos Kong [mailto:akong@redhat.com]
> Sent: Friday, June 06, 2014 1:29 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> 
> On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> > Hi, Amos
> >
> > > -----Original Message-----
> > > From: Amos Kong [mailto:akong@redhat.com]
> > > Sent: Friday, June 06, 2014 9:12 AM
> > > To: qemu-devel@nongnu.org
> > > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > >
> > > 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.
> 
> 
> > > +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);
> > > +        }
> > > +    }
> >
> > Do the pci multifunction devices support hotplug/unplug yet?
> 
> Yes.
> 
> You can hotplug single functions one by one, guest will identify
> all the functions when function 0 is added.
> Hotunplug function 0, all functions in same slot will be removed.
> 
Thanks.

IMHO, if we hot-plug function 0 first, the other functions will be not support to
hot-plug. Similarly, it will not be allowed if we only hot-unplug function 1~7,
but function 0, right?

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Amos Kong]

On Fri, Jun 06, 2014 at 08:05:15AM +0000, Gonglei (Arei) wrote:
> > -----Original Message-----
> > From: Amos Kong [mailto:akong@redhat.com]
> > Sent: Friday, June 06, 2014 1:29 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > 
> > On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> > > Hi, Amos
> > >
> > > > -----Original Message-----
> > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > To: qemu-devel@nongnu.org
> > > > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > >
> > > > 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.
> > 
> > 
> > > > +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);
> > > > +        }
> > > > +    }
> > >
> > > Do the pci multifunction devices support hotplug/unplug yet?
> > 
> > Yes.
> > 
> > You can hotplug single functions one by one, guest will identify
> > all the functions when function 0 is added.
> > Hotunplug function 0, all functions in same slot will be removed.
> > 
> Thanks.
> 
> IMHO, if we hot-plug function 0 first, the other functions will be not support to
> hot-plug.

Actually other functions can also be identified by guest, I had a
commit to add this support.

| commit f382a086f3129edc152b8044b69ccc6682e637bb
| Author: Amos Kong <akong@redhat.com>
| Date:   Fri Nov 25 15:03:07 2011 +0800
| 
|     PCI: Can continually add funcs after adding func0


> Similarly, it will not be allowed if we only hot-unplug function 1~7,
> but function 0, right?

Yes. function 0 is always necessary.
 
> Best regards,
> -Gonglei

-- 
			Amos.

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Gonglei (Arei)]

Best regards,
-Gonglei


> -----Original Message-----
> From: Amos Kong [mailto:akong@redhat.com]
> Sent: Friday, June 06, 2014 4:24 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> 
> On Fri, Jun 06, 2014 at 08:05:15AM +0000, Gonglei (Arei) wrote:
> > > -----Original Message-----
> > > From: Amos Kong [mailto:akong@redhat.com]
> > > Sent: Friday, June 06, 2014 1:29 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > >
> > > On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> > > > Hi, Amos
> > > >
> > > > > -----Original Message-----
> > > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > > To: qemu-devel@nongnu.org
> > > > > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > > >
> > > > > 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.
> > >
> > >
> > > > > +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);
> > > > > +        }
> > > > > +    }
> > > >
> > > > Do the pci multifunction devices support hotplug/unplug yet?
> > >
> > > Yes.
> > >
> > > You can hotplug single functions one by one, guest will identify
> > > all the functions when function 0 is added.
> > > Hotunplug function 0, all functions in same slot will be removed.
> > >
> > Thanks.
> >
> > IMHO, if we hot-plug function 0 first, the other functions will be not support
> to
> > hot-plug.
> 
> Actually other functions can also be identified by guest, I had a
> commit to add this support.
> 
> | commit f382a086f3129edc152b8044b69ccc6682e637bb
> | Author: Amos Kong <akong@redhat.com>
> | Date:   Fri Nov 25 15:03:07 2011 +0800
> |
> |     PCI: Can continually add funcs after adding func0
> 
> 
Good, Thanks!

BTW, the limits of ACPI DSDT table have been resolved yet?

> > Similarly, it will not be allowed if we only hot-unplug function 1~7,
> > but function 0, right?
> 
> Yes. function 0 is always necessary.
> 
> > Best regards,
> > -Gonglei
> 
> --
> 			Amos.

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Amos Kong]

On Fri, Jun 06, 2014 at 09:31:57AM +0000, Gonglei (Arei) wrote:
> > -----Original Message-----
> > From: Amos Kong [mailto:akong@redhat.com]
> > Sent: Friday, June 06, 2014 4:24 PM
> > To: Gonglei (Arei)
> > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > 
> > On Fri, Jun 06, 2014 at 08:05:15AM +0000, Gonglei (Arei) wrote:
> > > > -----Original Message-----
> > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > Sent: Friday, June 06, 2014 1:29 PM
> > > > To: Gonglei (Arei)
> > > > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > > > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > >
> > > > On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> > > > > Hi, Amos
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > > > To: qemu-devel@nongnu.org
> > > > > > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > > > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > > > >
> > > > > > 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.
> > > >
> > > >
> > > > > > +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);
> > > > > > +        }
> > > > > > +    }
> > > > >
> > > > > Do the pci multifunction devices support hotplug/unplug yet?
> > > >
> > > > Yes.
> > > >
> > > > You can hotplug single functions one by one, guest will identify
> > > > all the functions when function 0 is added.
> > > > Hotunplug function 0, all functions in same slot will be removed.
> > > >
> > > Thanks.
> > >
> > > IMHO, if we hot-plug function 0 first, the other functions will be not support
> > to
> > > hot-plug.
> > 
> > Actually other functions can also be identified by guest, I had a
> > commit to add this support.
> > 
> > | commit f382a086f3129edc152b8044b69ccc6682e637bb
> > | Author: Amos Kong <akong@redhat.com>
> > | Date:   Fri Nov 25 15:03:07 2011 +0800
> > |
> > |     PCI: Can continually add funcs after adding func0
> > 
> > 
> Good, Thanks!
> 
> BTW, the limits of ACPI DSDT table have been resolved yet?

Which limit?

There was a hot-unplug bug of multiple function, I tried to
fix it by change ACPI DSDT table in seabios, but finally we
fixed this in kernel.

|commit ce29ca3ea40744f24c2b5d88431e8ac566d257cc
|Author: Amos Kong <kongjianjun@gmail.com>
|Date:   Wed May 23 10:20:35 2012 -0600
|
|    PCI: acpiphp: remove all functions in slot, even without ACPI _EJx

> > > Similarly, it will not be allowed if we only hot-unplug function 1~7,
> > > but function 0, right?
> > 
> > Yes. function 0 is always necessary.
> > 
> > > Best regards,
> > > -Gonglei
> > 
> > --
> > 			Amos.

-- 
			Amos.

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Gonglei (Arei)]

> -----Original Message-----
> From: Amos Kong [mailto:akong@redhat.com]
> Sent: Friday, June 06, 2014 6:11 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> 
> On Fri, Jun 06, 2014 at 09:31:57AM +0000, Gonglei (Arei) wrote:
> > > -----Original Message-----
> > > From: Amos Kong [mailto:akong@redhat.com]
> > > Sent: Friday, June 06, 2014 4:24 PM
> > > To: Gonglei (Arei)
> > > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > >
> > > On Fri, Jun 06, 2014 at 08:05:15AM +0000, Gonglei (Arei) wrote:
> > > > > -----Original Message-----
> > > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > > Sent: Friday, June 06, 2014 1:29 PM
> > > > > To: Gonglei (Arei)
> > > > > Cc: qemu-devel@nongnu.org; stefanha@gmail.com; afaerber@suse.de
> > > > > Subject: Re: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > > >
> > > > > On Fri, Jun 06, 2014 at 01:28:59AM +0000, Gonglei (Arei) wrote:
> > > > > > Hi, Amos
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: Amos Kong [mailto:akong@redhat.com]
> > > > > > > Sent: Friday, June 06, 2014 9:12 AM
> > > > > > > To: qemu-devel@nongnu.org
> > > > > > > Cc: stefanha@gmail.com; afaerber@suse.de; Gonglei (Arei)
> > > > > > > Subject: [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest
> > > > > > >
> > > > > > > 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.
> > > > >
> > > > >
> > > > > > > +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);
> > > > > > > +        }
> > > > > > > +    }
> > > > > >
> > > > > > Do the pci multifunction devices support hotplug/unplug yet?
> > > > >
> > > > > Yes.
> > > > >
> > > > > You can hotplug single functions one by one, guest will identify
> > > > > all the functions when function 0 is added.
> > > > > Hotunplug function 0, all functions in same slot will be removed.
> > > > >
> > > > Thanks.
> > > >
> > > > IMHO, if we hot-plug function 0 first, the other functions will be not
> support
> > > to
> > > > hot-plug.
> > >
> > > Actually other functions can also be identified by guest, I had a
> > > commit to add this support.
> > >
> > > | commit f382a086f3129edc152b8044b69ccc6682e637bb
> > > | Author: Amos Kong <akong@redhat.com>
> > > | Date:   Fri Nov 25 15:03:07 2011 +0800
> > > |
> > > |     PCI: Can continually add funcs after adding func0
> > >
> > >
> > Good, Thanks!
> >
> > BTW, the limits of ACPI DSDT table have been resolved yet?
> 
> Which limit?
> 
> There was a hot-unplug bug of multiple function, I tried to
> fix it by change ACPI DSDT table in seabios, but finally we
> fixed this in kernel.
> 
OK, understand it, " Note: the hot-unplug can't work without cooperation of guest OS."

I see your discussion at maillist:
https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg01103.html

but I don't find the patch in seabios.git, so I have a doubt mentioned above.

Thanks for your patient answer.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH v3 3/4] virtio-blk-test.c: add hotplug subtest

[Stefan Hajnoczi]

On Fri, Jun 06, 2014 at 09:11:59AM +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 <akong@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/virtio-blk-test.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[Qemu-devel] [PATCH v3 4/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test

[Amos Kong]

Signed-off-by: Amos Kong <akong@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/blockdev-test.c | 23 ++---------------------
 1 file changed, 2 insertions(+), 21 deletions(-)

diff --git a/tests/blockdev-test.c b/tests/blockdev-test.c
index c940e00..c9127c0 100644
--- a/tests/blockdev-test.c
+++ b/tests/blockdev-test.c
@@ -16,35 +16,16 @@
 
 static void test_drive_add_empty(void)
 {
-    QDict *response;
-    const char *response_return;
-
     /* Start with an empty drive */
     qtest_start("-drive if=none,id=drive0");
 
     /* Delete the drive */
-    response = qmp("{\"execute\": \"human-monitor-command\","
-                   " \"arguments\": {"
-                   "   \"command-line\": \"drive_del drive0\""
-                   "}}");
-    g_assert(response);
-    response_return = qdict_get_try_str(response, "return");
-    g_assert(response_return);
-    g_assert(strcmp(response_return, "") == 0);
-    QDECREF(response);
+    qmp_exec_hmp_cmd("", "drive_del drive0");
 
     /* Ensure re-adding the drive works - there should be no duplicate ID error
      * because the old drive must be gone.
      */
-    response = qmp("{\"execute\": \"human-monitor-command\","
-                   " \"arguments\": {"
-                   "   \"command-line\": \"drive_add 0 if=none,id=drive0\""
-                   "}}");
-    g_assert(response);
-    response_return = qdict_get_try_str(response, "return");
-    g_assert(response_return);
-    g_assert(strcmp(response_return, "OK\r\n") == 0);
-    QDECREF(response);
+    qmp_exec_hmp_cmd("OK\r\n", "drive_add 0 if=none,id=drive0");
 
     qtest_end();
 }
-- 
1.9.3

Re: [Qemu-devel] [PATCH v3 4/4] qtest: use qmp_exec_hmp_cmd() in blockdev-test

[Stefan Hajnoczi]

On Fri, Jun 06, 2014 at 09:12:00AM +0800, Amos Kong wrote:
> Signed-off-by: Amos Kong <akong@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  tests/blockdev-test.c | 23 ++---------------------
>  1 file changed, 2 insertions(+), 21 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>