tests: Introduce generic device hot-plug/hot-unplug functions
PATCH v2 1/1 tests: Introduce generic device hot-plug/hot-unplug functions
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.
---
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(-)
tests/libqos/pci.c +2 -17
@@ -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 :"" ); }
tests/libqos/usb.c +6 -24
@@ -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); }
tests/libqtest.c +75 -0
78 lines not on this page · raw message
tests/libqtest.h +19 -0
@@ -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
tests/usb-hcd-uhci-test.c +2 -24
@@ -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)
tests/usb-hcd-xhci-test.c +4 -47
63 lines not on this page · raw message
tests/virtio-scsi-test.c +2 -22
@@ -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); }
tests/virtio-serial-test.c +3 -22
@@ -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)
Signature · 1 line
1.8.3.1
Quoted 11 lines · Thomas Huth
A lot of tests provide code for adding and removing a device via the
device_addanddevice_delQMP 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 andDEVICE_DELevent 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 <[email protected]>
I verified that this patch passes the "make check" either on master branch or my private branch (which broke before this one). So:
Thanks!
Signature · 1 line
Peter Xu
Quoted 17 lines · Thomas Huth
A lot of tests provide code for adding and removing a device via the
device_addanddevice_delQMP 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 andDEVICE_DELevent 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 <[email protected]>---
+ */
tests/libqtest.c +3
-0+void qtest_qmp_device_del (const char *id) +{ +QDict *response1, *response2, *event =NULL ;
tests/libqtest.c void qtest_cb_for_every_machine(void (*cb)(const char *machine)) · line 1050
Quoted 10 lines
tests/libqtest.c +10
-0+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 ("" );
Hmm, this new use of qmp("") means I get to rebase the efforts to remove the empty qmp command usage from the testsuite efforts. We really want to have two low-level functions (one to send, one to receive), and then have qmp() be the combination of both in a row; we have qmp_async() for the send, and only qtest_receive() for the receive, so I'll have to revive my work on unifying things...
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
Quoted 10 lines · Eric Blake
Quoted 9 lines · Thomas Huth
A lot of tests provide code for adding and removing a device via the
device_addanddevice_delQMP 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 andDEVICE_DELevent 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.
[...]
Quoted 8 lines
Quoted 8 lines
tests/libqtest.c +8
-0+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);
Quoted 8 lines
tests/libqtest.c void qtest_cb_for_every_machine(void (*cb)(const char *machine)) · line 1050
Quoted 5 lines
tests/libqtest.c +5
-0+g_free (cmd); +g_assert (response1); +g_assert (!qdict_haskey (response1,"error" )); + + response2 =qmp ("" );Hmm, this new use of
qmp("")means I get to rebase the efforts to remove the empty qmp command usage from the testsuite efforts.
Sorry about that :-/ I hope it's not too much of a hassle!
Maybe we should finally also add some assert(strlen(fmt) > 0) to that function to avoid that the problem sneaks in again?
Thomas
Quoted 9 lines · Thomas Huth
Quoted 4 lines
Quoted 1 line
+ response2 =
qmp("");Hmm, this new use of
qmp("")means I get to rebase the efforts to remove the empty qmp command usage from the testsuite efforts.Sorry about that :-/ I hope it's not too much of a hassle!
Maybe we should finally also add some assert(strlen(fmt) > 0) to that function to avoid that the problem sneaks in again?
Yep, already proposed (and thus part of my ongoing rebase work):
https://lists.gnu.org/archive/html/qemu-devel/2017-08/msg00599.html
Signature · 3 lines
Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org