From patchwork Wed Sep 13 18:40:54 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Huth X-Patchwork-Id: 813575 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3xsr9T62dTz9s76 for ; Thu, 14 Sep 2017 04:41:46 +1000 (AEST) Received: from localhost ([::1]:44113 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsCbX-0008I3-8q for incoming@patchwork.ozlabs.org; Wed, 13 Sep 2017 14:41:43 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dsCay-0008G7-Ut for qemu-devel@nongnu.org; Wed, 13 Sep 2017 14:41:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dsCas-0004nv-FE for qemu-devel@nongnu.org; Wed, 13 Sep 2017 14:41:08 -0400 Received: from mx1.redhat.com ([209.132.183.28]:19326) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dsCas-0004nP-5n for qemu-devel@nongnu.org; Wed, 13 Sep 2017 14:41:02 -0400 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D9C8F80F94; Wed, 13 Sep 2017 18:41:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com D9C8F80F94 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=thuth@redhat.com Received: from thh440s.redhat.com (ovpn-116-123.ams2.redhat.com [10.36.116.123]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4D8315D963; Wed, 13 Sep 2017 18:40:55 +0000 (UTC) From: Thomas Huth To: qemu-devel@nongnu.org, Peter Xu Date: Wed, 13 Sep 2017 20:40:54 +0200 Message-Id: <1505328054-23805-1-git-send-email-thuth@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Wed, 13 Sep 2017 18:41:01 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2] tests: Introduce generic device hot-plug/hot-unplug functions X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Paolo Bonzini , Gerd Hoffmann , Markus Armbruster , Amit Shah Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" A lot of tests provide code for adding and removing a device via the device_add and device_del QMP commands. Maintaining this code in so many places is cumbersome and error-prone (some of the code parts check the responses for device deletion in an incorrect way, for example, we've got to deal with both, error code and DEVICE_DEL event here). So let's provide some proper generic functions for adding and removing a device instead. The code for correctly unplugging a device has been taken from a patch from Peter Xu. Signed-off-by: Thomas Huth Reviewed-by: Peter Xu Tested-by: Peter Xu --- v2: - Use the unplug code from Peter Xu's patch - Renamed the functions to *device_add/del instead of *hotplug/unplug tests/libqos/pci.c | 19 ++---------- tests/libqos/usb.c | 30 ++++--------------- tests/libqtest.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++ tests/libqtest.h | 19 ++++++++++++ tests/usb-hcd-uhci-test.c | 26 ++-------------- tests/usb-hcd-xhci-test.c | 51 +++---------------------------- tests/virtio-scsi-test.c | 24 ++------------- tests/virtio-serial-test.c | 25 ++-------------- 8 files changed, 113 insertions(+), 156 deletions(-) diff --git a/tests/libqos/pci.c b/tests/libqos/pci.c index 2dcdead..df1f98e 100644 --- a/tests/libqos/pci.c +++ b/tests/libqos/pci.c @@ -394,21 +394,6 @@ QPCIBar qpci_legacy_iomap(QPCIDevice *dev, uint16_t addr) void qpci_plug_device_test(const char *driver, const char *id, uint8_t slot, const char *opts) { - QDict *response; - char *cmd; - - cmd = g_strdup_printf("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': '%s'," - " 'addr': '%d'," - " %s%s" - " 'id': '%s'" - "}}", driver, slot, - opts ? opts : "", opts ? "," : "", - id); - response = qmp(cmd); - g_free(cmd); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); + qtest_qmp_device_add(driver, id, "'addr': '%d'%s%s", slot, + opts ? ", " : "", opts ? opts : ""); } diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c index 0cdfaec..2a47604 100644 --- a/tests/libqos/usb.c +++ b/tests/libqos/usb.c @@ -40,34 +40,16 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) void usb_test_hotplug(const char *hcd_id, const int port, void (*port_check)(void)) { - QDict *response; - char *cmd; + char *id = g_strdup_printf("usbdev%d", port); - cmd = g_strdup_printf("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': 'usb-tablet'," - " 'port': '%d'," - " 'bus': '%s.0'," - " 'id': 'usbdev%d'" - "}}", port, hcd_id, port); - response = qmp(cmd); - g_free(cmd); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); + qtest_qmp_device_add("usb-tablet", id, "'port': '%d', 'bus': '%s.0'", + port, hcd_id); if (port_check) { port_check(); } - cmd = g_strdup_printf("{'execute': 'device_del'," - " 'arguments': {" - " 'id': 'usbdev%d'" - "}}", port); - response = qmp(cmd); - g_free(cmd); - g_assert(response); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); + qtest_qmp_device_del(id); + + g_free(id); } diff --git a/tests/libqtest.c b/tests/libqtest.c index b9a1f18..0c12b38 100644 --- a/tests/libqtest.c +++ b/tests/libqtest.c @@ -987,3 +987,78 @@ void qtest_cb_for_every_machine(void (*cb)(const char *machine)) qtest_end(); QDECREF(response); } + +/* + * Generic hot-plugging test via the device_add QMP command. + */ +void qtest_qmp_device_add(const char *driver, const char *id, const char *fmt, + ...) +{ + QDict *response; + char *cmd, *opts = NULL; + va_list va; + + if (fmt) { + va_start(va, fmt); + opts = g_strdup_vprintf(fmt, va); + va_end(va); + } + + cmd = g_strdup_printf("{'execute': 'device_add'," + " 'arguments': { 'driver': '%s', 'id': '%s'%s%s }}", + driver, id, opts ? ", " : "", opts ? opts : ""); + g_free(opts); + + response = qmp(cmd); + g_free(cmd); + g_assert(response); + g_assert(!qdict_haskey(response, "event")); /* We don't expect any events */ + g_assert(!qdict_haskey(response, "error")); + QDECREF(response); +} + +/* + * Generic hot-unplugging test via the device_del QMP command. + * Device deletion will get one response and one event. For example: + * + * {'execute': 'device_del','arguments': { 'id': 'scsi-hd'}} + * + * will get this one: + * + * {"timestamp": {"seconds": 1505289667, "microseconds": 569862}, + * "event": "DEVICE_DELETED", "data": {"device": "scsi-hd", + * "path": "/machine/peripheral/scsi-hd"}} + * + * and this one: + * + * {"return": {}} + * + * But the order of arrival may vary - so we've got to detect both. + */ +void qtest_qmp_device_del(const char *id) +{ + QDict *response1, *response2, *event = NULL; + char *cmd; + + cmd = g_strdup_printf("{'execute': 'device_del'," + " 'arguments': { 'id': '%s' }}", id); + response1 = qmp(cmd); + g_free(cmd); + g_assert(response1); + g_assert(!qdict_haskey(response1, "error")); + + response2 = qmp(""); + g_assert(response2); + g_assert(!qdict_haskey(response2, "error")); + + if (qdict_haskey(response1, "event")) { + event = response1; + } else if (qdict_haskey(response2, "event")) { + event = response2; + } + g_assert(event); + g_assert_cmpstr(qdict_get_str(event, "event"), ==, "DEVICE_DELETED"); + + QDECREF(response1); + QDECREF(response2); +} diff --git a/tests/libqtest.h b/tests/libqtest.h index 3ae5709..44803d7 100644 --- a/tests/libqtest.h +++ b/tests/libqtest.h @@ -927,4 +927,23 @@ QDict *qmp_fd(int fd, const char *fmt, ...); */ void qtest_cb_for_every_machine(void (*cb)(const char *machine)); +/** + * qtest_qmp_device_add: + * @driver: Name of the device that should be added + * @id: Identification string + * @fmt: printf-like format string for further options to device_add + * + * Generic hot-plugging test via the device_add QMP command. + */ +void qtest_qmp_device_add(const char *driver, const char *id, const char *fmt, + ...) GCC_FMT_ATTR(3, 4); + +/** + * qtest_qmp_device_del: + * @id: Identification string + * + * Generic hot-unplugging test via the device_del QMP command. + */ +void qtest_qmp_device_del(const char *id); + #endif diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c index 5b500fe..62e0c78 100644 --- a/tests/usb-hcd-uhci-test.c +++ b/tests/usb-hcd-uhci-test.c @@ -48,31 +48,9 @@ static void test_uhci_hotplug(void) static void test_usb_storage_hotplug(void) { - QDict *response; + qtest_qmp_device_add("usb-storage", "usbdev0", "'drive': 'drive0'"); - response = qmp("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': 'usb-storage'," - " 'drive': 'drive0'," - " 'id': 'usbdev0'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp("{'execute': 'device_del'," - " 'arguments': {" - " 'id': 'usbdev0'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp(""); - g_assert(response); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); + qtest_qmp_device_del("usbdev0"); } int main(int argc, char **argv) diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c index 031764d..9c14e30 100644 --- a/tests/usb-hcd-xhci-test.c +++ b/tests/usb-hcd-xhci-test.c @@ -23,59 +23,16 @@ static void test_xhci_hotplug(void) static void test_usb_uas_hotplug(void) { - QDict *response; - - response = qmp("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': 'usb-uas'," - " 'id': 'uas'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp("{'execute': 'device_add'," - " 'arguments': {" - " 'driver': 'scsi-hd'," - " 'drive': 'drive0'," - " 'id': 'scsi-hd'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); + qtest_qmp_device_add("usb-uas", "uas", NULL); + qtest_qmp_device_add("scsi-hd", "scsihd", "'drive': 'drive0'"); /* TODO: UAS HBA driver in libqos, to check that added disk is visible after BUS rescan */ - response = qmp("{'execute': 'device_del'," - " 'arguments': {" - " 'id': 'scsi-hd'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp(""); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); - - - response = qmp("{'execute': 'device_del'," - " 'arguments': {" - " 'id': 'uas'" - "}}"); - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp(""); - g_assert(response); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); + qtest_qmp_device_del("scsihd"); + qtest_qmp_device_del("uas"); } int main(int argc, char **argv) diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c index 87a3b6e..d148512 100644 --- a/tests/virtio-scsi-test.c +++ b/tests/virtio-scsi-test.c @@ -192,32 +192,12 @@ static void pci_nop(void) static void hotplug(void) { - QDict *response; QOSState *qs; qs = qvirtio_scsi_start( "-drive id=drv1,if=none,file=null-co://,format=raw"); - response = qmp("{\"execute\": \"device_add\"," - " \"arguments\": {" - " \"driver\": \"scsi-hd\"," - " \"id\": \"scsi-hd\"," - " \"drive\": \"drv1\"" - "}}"); - - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp("{\"execute\": \"device_del\"," - " \"arguments\": {" - " \"id\": \"scsi-hd\"" - "}}"); - - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); + qtest_qmp_device_add("scsi-hd", "scsihd", "'drive': 'drv1'"); + qtest_qmp_device_del("scsihd"); qvirtio_scsi_stop(qs); } diff --git a/tests/virtio-serial-test.c b/tests/virtio-serial-test.c index b14d943..7d1517d 100644 --- a/tests/virtio-serial-test.c +++ b/tests/virtio-serial-test.c @@ -17,28 +17,9 @@ static void pci_nop(void) static void hotplug(void) { - QDict *response; - - response = qmp("{\"execute\": \"device_add\"," - " \"arguments\": {" - " \"driver\": \"virtserialport\"," - " \"id\": \"hp-port\"" - "}}"); - - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - QDECREF(response); - - response = qmp("{\"execute\": \"device_del\"," - " \"arguments\": {" - " \"id\": \"hp-port\"" - "}}"); - - g_assert(response); - g_assert(!qdict_haskey(response, "error")); - g_assert(qdict_haskey(response, "event")); - g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); - QDECREF(response); + qtest_qmp_device_add("virtserialport", "hp-port", NULL); + + qtest_qmp_device_del("hp-port"); } int main(int argc, char **argv)