* [PULL 01/17] q800: move dp8393x_prom memory region to Q800MachineState
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 02/17] qtest: use correct boolean type for failover property Thomas Huth
` (16 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Mark Cave-Ayland, Thomas Huth,
Philippe Mathieu-Daudé, Laurent Vivier
From: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
There is no need to dynamically allocate the memory region from the heap.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-ID: <20231227210212.245106-1-mark.cave-ayland@ilande.co.uk>
Reviewed-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
include/hw/m68k/q800.h | 1 +
hw/m68k/q800.c | 7 +++----
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/hw/m68k/q800.h b/include/hw/m68k/q800.h
index a9661f65f6..34365c9860 100644
--- a/include/hw/m68k/q800.h
+++ b/include/hw/m68k/q800.h
@@ -55,6 +55,7 @@ struct Q800MachineState {
MOS6522Q800VIA1State via1;
MOS6522Q800VIA2State via2;
dp8393xState dp8393x;
+ MemoryRegion dp8393x_prom;
ESCCState escc;
OrIRQState escc_orgate;
SysBusESPState esp;
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 83d1571d02..b80a3b6d5f 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -253,7 +253,6 @@ static void q800_machine_init(MachineState *machine)
int bios_size;
ram_addr_t initrd_base;
int32_t initrd_size;
- MemoryRegion *dp8393x_prom = g_new(MemoryRegion, 1);
uint8_t *prom;
int i, checksum;
MacFbMode *macfb_mode;
@@ -406,13 +405,13 @@ static void q800_machine_init(MachineState *machine)
sysbus_connect_irq(sysbus, 0,
qdev_get_gpio_in(DEVICE(&m->glue), GLUE_IRQ_IN_SONIC));
- memory_region_init_rom(dp8393x_prom, NULL, "dp8393x-q800.prom",
+ memory_region_init_rom(&m->dp8393x_prom, NULL, "dp8393x-q800.prom",
SONIC_PROM_SIZE, &error_fatal);
memory_region_add_subregion(get_system_memory(), SONIC_PROM_BASE,
- dp8393x_prom);
+ &m->dp8393x_prom);
/* Add MAC address with valid checksum to PROM */
- prom = memory_region_get_ram_ptr(dp8393x_prom);
+ prom = memory_region_get_ram_ptr(&m->dp8393x_prom);
checksum = 0;
for (i = 0; i < 6; i++) {
prom[i] = revbit8(nd_table[0].macaddr.a[i]);
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 02/17] qtest: use correct boolean type for failover property
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
2024-01-11 17:00 ` [PULL 01/17] q800: move dp8393x_prom memory region to Q800MachineState Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 03/17] Revert "netdev: set timeout depending on loadavg" Thomas Huth
` (15 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Zhao Liu
From: Daniel P. Berrangé <berrange@redhat.com>
QMP device_add does not historically validate the parameter types.
At some point it will likely change to enforce correct types, to
match behaviour of -device. The failover property is expected to
be a boolean in JSON.
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240103123005.2400437-1-berrange@redhat.com>
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/virtio-net-failover.c | 36 +++++++++++++++----------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tests/qtest/virtio-net-failover.c b/tests/qtest/virtio-net-failover.c
index 0d40bc1f2d..73dfabc272 100644
--- a/tests/qtest/virtio-net-failover.c
+++ b/tests/qtest/virtio-net-failover.c
@@ -486,7 +486,7 @@ static void test_hotplug_1_reverse(void)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -517,7 +517,7 @@ static void test_hotplug_2(void)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -566,7 +566,7 @@ static void test_hotplug_2_reverse(void)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'rombar': 0,"
"'romfile': '',"
@@ -639,7 +639,7 @@ static void test_migrate_out(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -754,7 +754,7 @@ static void test_migrate_in(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -808,7 +808,7 @@ static void test_off_migrate_out(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'off',"
+ "'failover': false,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -876,7 +876,7 @@ static void test_off_migrate_in(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'off',"
+ "'failover': false,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -927,7 +927,7 @@ static void test_guest_off_migrate_out(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1003,7 +1003,7 @@ static void test_guest_off_migrate_in(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1054,7 +1054,7 @@ static void test_migrate_guest_off_abort(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1154,7 +1154,7 @@ static void test_migrate_abort_wait_unplug(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1243,7 +1243,7 @@ static void test_migrate_abort_active(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1342,7 +1342,7 @@ static void test_migrate_off_abort(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'off',"
+ "'failover': false,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1434,7 +1434,7 @@ static void test_migrate_abort_timeout(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1530,7 +1530,7 @@ static void test_multi_out(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1561,7 +1561,7 @@ static void test_multi_out(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby1",
"{'bus': 'root2',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs2',"
"'mac': '"MAC_STANDBY1"'}");
@@ -1700,7 +1700,7 @@ static void test_multi_in(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby0",
"{'bus': 'root0',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs0',"
"'mac': '"MAC_STANDBY0"'}");
@@ -1724,7 +1724,7 @@ static void test_multi_in(gconstpointer opaque)
qtest_qmp_device_add(qts, "virtio-net", "standby1",
"{'bus': 'root2',"
- "'failover': 'on',"
+ "'failover': true,"
"'netdev': 'hs2',"
"'mac': '"MAC_STANDBY1"'}");
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 03/17] Revert "netdev: set timeout depending on loadavg"
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
2024-01-11 17:00 ` [PULL 01/17] q800: move dp8393x_prom memory region to Q800MachineState Thomas Huth
2024-01-11 17:00 ` [PULL 02/17] qtest: use correct boolean type for failover property Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 04/17] Revert "osdep: add getloadavg" Thomas Huth
` (14 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit cadfc7293977ecadc2d6c48d7cffc553ed2f85f1.
The test was not timing out because of slow execution. It was
timing out due to a race condition leading to the client QEMU
attempting (and fatally failing) to connect before the server
QEMU was listening.
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-2-berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/netdev-socket.c | 28 +---------------------------
1 file changed, 1 insertion(+), 27 deletions(-)
diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index bb99d08b5e..7ba1eff120 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -18,32 +18,6 @@
#define CONNECTION_TIMEOUT 120
-static double connection_timeout(void)
-{
- double load;
- int ret = getloadavg(&load, 1);
-
- /*
- * If we can't get load data, or load is low because we just started
- * running, assume load of 1 (we are alone in this system).
- */
- if (ret < 1 || load < 1.0) {
- load = 1.0;
- }
- /*
- * No one wants to wait more than 10 minutes for this test. Higher load?
- * Too bad.
- */
- if (load > 10.0) {
- fprintf(stderr, "Warning: load %f higher than 10 - test might timeout\n",
- load);
- load = 10.0;
- }
-
- /* if load is high increase timeout as we might not get a chance to run */
- return load * CONNECTION_TIMEOUT;
-}
-
#define EXPECT_STATE(q, e, t) \
do { \
char *resp = NULL; \
@@ -57,7 +31,7 @@ do { \
if (g_str_equal(resp, e)) { \
break; \
} \
- } while (g_test_timer_elapsed() < connection_timeout()); \
+ } while (g_test_timer_elapsed() < CONNECTION_TIMEOUT); \
g_assert_cmpstr(resp, ==, e); \
g_free(resp); \
} while (0)
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 04/17] Revert "osdep: add getloadavg"
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (2 preceding siblings ...)
2024-01-11 17:00 ` [PULL 03/17] Revert "netdev: set timeout depending on loadavg" Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 05/17] Revert "tests/qtest/netdev-socket: Raise connection timeout to 120 seconds" Thomas Huth
` (13 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit dc864d3a3777424187280e50c9bfb84dced54f12.
This functionality is not required after the previous revert
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-3-berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
meson.build | 1 -
include/qemu/osdep.h | 10 ----------
2 files changed, 11 deletions(-)
diff --git a/meson.build b/meson.build
index 371edafae6..8dd6347d1b 100644
--- a/meson.build
+++ b/meson.build
@@ -2341,7 +2341,6 @@ config_host_data.set('HAVE_GLIB_WITH_SLICE_ALLOCATOR', glib_has_gslice)
config_host_data.set('HAVE_OPENPTY', cc.has_function('openpty', dependencies: util))
config_host_data.set('HAVE_STRCHRNUL', cc.has_function('strchrnul'))
config_host_data.set('HAVE_SYSTEM_FUNCTION', cc.has_function('system', prefix: '#include <stdlib.h>'))
-config_host_data.set('HAVE_GETLOADAVG_FUNCTION', cc.has_function('getloadavg', prefix: '#include <stdlib.h>'))
if rbd.found()
config_host_data.set('HAVE_RBD_NAMESPACE_EXISTS',
cc.has_function('rbd_namespace_exists',
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index db366d6796..9a405bed89 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -781,16 +781,6 @@ static inline int platform_does_not_support_system(const char *command)
}
#endif /* !HAVE_SYSTEM_FUNCTION */
-/**
- * If the load average was unobtainable, -1 is returned
- */
-#ifndef HAVE_GETLOADAVG_FUNCTION
-static inline int getloadavg(double loadavg[], int nelem)
-{
- return -1;
-}
-#endif /* !HAVE_GETLOADAVG_FUNCTION */
-
#ifdef __cplusplus
}
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 05/17] Revert "tests/qtest/netdev-socket: Raise connection timeout to 120 seconds"
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (3 preceding siblings ...)
2024-01-11 17:00 ` [PULL 04/17] Revert "osdep: add getloadavg" Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 06/17] net: add explicit info about connecting/listening state Thomas Huth
` (12 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
This reverts commit 0daaf2761f6d268ffaa2d01d450e202e127452b1.
The test was not timing out because of slow execution. It was
timing out due to a race condition leading to the client QEMU
attempting (and fatally failing) to connect before the server
QEMU was listening.
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-4-berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/netdev-socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 7ba1eff120..3fc2ac26d0 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -16,7 +16,7 @@
#include "qapi/qobject-input-visitor.h"
#include "qapi/qapi-visit-sockets.h"
-#define CONNECTION_TIMEOUT 120
+#define CONNECTION_TIMEOUT 60
#define EXPECT_STATE(q, e, t) \
do { \
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 06/17] net: add explicit info about connecting/listening state
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (4 preceding siblings ...)
2024-01-11 17:00 ` [PULL 05/17] Revert "tests/qtest/netdev-socket: Raise connection timeout to 120 seconds" Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 07/17] net: handle QIOTask completion to report useful error message Thomas Huth
` (11 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
When running 'info network', if the stream backend is still in
the process of connecting, or waiting for an incoming connection,
no information is displayed.
There is also no way to distinguish whether the server is still
in the process of setting up the listener socket, or whether it
is ready to accept incoming client connections.
This leads to a race condition in the netdev-socket qtest which
launches a server process followed by a client process. Under
high load conditions it is possible for the client to attempt
to connect before the server is accepting clients. For the
scenarios which do not set the 'reconnect' option, this opens
up a race which can lead to the test scenario failing to reach
the expected state.
Now that 'info network' can distinguish between initialization
phase and the listening phase, the netdev-socket qtest will
correctly synchronize, such that the client QEMU is not spawned
until the server is ready.
This should solve the non-deterministic failures seen with the
netdev-socket qtest.
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-5-berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
net/stream.c | 5 ++++-
tests/qtest/netdev-socket.c | 10 +++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/net/stream.c b/net/stream.c
index 9204b4c96e..0defb21d45 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -173,7 +173,7 @@ static gboolean net_stream_send(QIOChannel *ioc,
net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
s->nc.link_down = true;
- qemu_set_info_str(&s->nc, "%s", "");
+ qemu_set_info_str(&s->nc, "listening");
qapi_event_send_netdev_stream_disconnected(s->nc.name);
net_stream_arm_reconnect(s);
@@ -292,6 +292,7 @@ static void net_stream_server_listening(QIOTask *task, gpointer opaque)
s->nc.link_down = true;
s->listener = qio_net_listener_new();
+ qemu_set_info_str(&s->nc, "listening");
net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
qio_net_listener_set_client_func(s->listener, net_stream_listen, s, NULL);
qio_net_listener_add(s->listener, listen_sioc);
@@ -309,6 +310,7 @@ static int net_stream_server_init(NetClientState *peer,
nc = qemu_new_net_client(&net_stream_info, peer, model, name);
s = DO_UPCAST(NetStreamState, nc, nc);
+ qemu_set_info_str(&s->nc, "initializing");
s->listen_ioc = QIO_CHANNEL(listen_sioc);
qio_channel_socket_listen_async(listen_sioc, addr, 0,
@@ -400,6 +402,7 @@ static int net_stream_client_init(NetClientState *peer,
nc = qemu_new_net_client(&net_stream_info, peer, model, name);
s = DO_UPCAST(NetStreamState, nc, nc);
+ qemu_set_info_str(&s->nc, "connecting");
s->ioc = QIO_CHANNEL(sioc);
s->nc.link_down = true;
diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 3fc2ac26d0..91441f7922 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -127,7 +127,7 @@ static void test_stream_inet_ipv4(void)
"addr.ipv4=on,addr.ipv6=off,"
"addr.host=127.0.0.1,addr.port=%d", port);
- EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+ EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
qts1 = qtest_initf("-nodefaults -M none "
"-netdev stream,server=false,id=st0,addr.type=inet,"
@@ -200,7 +200,7 @@ static void test_stream_unix_reconnect(void)
"-netdev stream,id=st0,server=true,addr.type=unix,"
"addr.path=%s", path);
- EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+ EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
qts1 = qtest_initf("-nodefaults -M none "
"-netdev stream,server=false,id=st0,addr.type=unix,"
@@ -250,7 +250,7 @@ static void test_stream_inet_ipv6(void)
"addr.ipv4=off,addr.ipv6=on,"
"addr.host=::1,addr.port=%d", port);
- EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+ EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
qts1 = qtest_initf("-nodefaults -M none "
"-netdev stream,server=false,id=st0,addr.type=inet,"
@@ -282,7 +282,7 @@ static void test_stream_unix(void)
"addr.type=unix,addr.path=%s,",
path);
- EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+ EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
qts1 = qtest_initf("-nodefaults -M none "
"-netdev stream,id=st0,server=false,"
@@ -314,7 +314,7 @@ static void test_stream_unix_abstract(void)
"addr.abstract=on",
path);
- EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0);
+ EXPECT_STATE(qts0, "st0: index=0,type=stream,listening\r\n", 0);
qts1 = qtest_initf("-nodefaults -M none "
"-netdev stream,id=st0,server=false,"
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 07/17] net: handle QIOTask completion to report useful error message
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (5 preceding siblings ...)
2024-01-11 17:00 ` [PULL 06/17] net: add explicit info about connecting/listening state Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 08/17] qtest: ensure netdev-socket tests have non-overlapping names Thomas Huth
` (10 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Daniel P. Berrangé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
The network stream backend uses the async QIO socket APIs for listening
and connecting sockets. It does not check the task object completion
status, however, instead just looking at whether the socket FD is -1
or not.
By checking the task completion, we can set a useful error message for
users instead of the non-actionable "connection error" string.
eg so users will see:
(qemu) info network
net: index=0,type=stream,error: Failed to connect to '/foo.unix': No such file or directory
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-6-berrange@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
net/stream.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/stream.c b/net/stream.c
index 0defb21d45..97e6ec6679 100644
--- a/net/stream.c
+++ b/net/stream.c
@@ -165,6 +165,7 @@ static gboolean net_stream_send(QIOChannel *ioc,
s->ioc_write_tag = 0;
}
if (s->listener) {
+ qemu_set_info_str(&s->nc, "listening");
qio_net_listener_set_client_func(s->listener, net_stream_listen,
s, NULL);
}
@@ -173,7 +174,6 @@ static gboolean net_stream_send(QIOChannel *ioc,
net_socket_rs_init(&s->rs, net_stream_rs_finalize, false);
s->nc.link_down = true;
- qemu_set_info_str(&s->nc, "listening");
qapi_event_send_netdev_stream_disconnected(s->nc.name);
net_stream_arm_reconnect(s);
@@ -272,9 +272,11 @@ static void net_stream_server_listening(QIOTask *task, gpointer opaque)
QIOChannelSocket *listen_sioc = QIO_CHANNEL_SOCKET(s->listen_ioc);
SocketAddress *addr;
int ret;
+ Error *err = NULL;
- if (listen_sioc->fd < 0) {
- qemu_set_info_str(&s->nc, "connection error");
+ if (qio_task_propagate_error(task, &err)) {
+ qemu_set_info_str(&s->nc, "error: %s", error_get_pretty(err));
+ error_free(err);
return;
}
@@ -327,9 +329,11 @@ static void net_stream_client_connected(QIOTask *task, gpointer opaque)
SocketAddress *addr;
gchar *uri;
int ret;
+ Error *err = NULL;
- if (sioc->fd < 0) {
- qemu_set_info_str(&s->nc, "connection error");
+ if (qio_task_propagate_error(task, &err)) {
+ qemu_set_info_str(&s->nc, "error: %s", error_get_pretty(err));
+ error_free(err);
goto error;
}
@@ -384,6 +388,7 @@ static gboolean net_stream_reconnect(gpointer data)
static void net_stream_arm_reconnect(NetStreamState *s)
{
if (s->reconnect && s->timer_tag == 0) {
+ qemu_set_info_str(&s->nc, "connecting");
s->timer_tag = g_timeout_add_seconds(s->reconnect,
net_stream_reconnect, s);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 08/17] qtest: ensure netdev-socket tests have non-overlapping names
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (6 preceding siblings ...)
2024-01-11 17:00 ` [PULL 07/17] net: handle QIOTask completion to report useful error message Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 09/17] tests/qtest/virtio-ccw: Fix device presence checking Thomas Huth
` (9 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Daniel P. Berrangé,
Philippe Mathieu-Daudé, Stefan Hajnoczi
From: Daniel P. Berrangé <berrange@redhat.com>
When naming glib tests if the name of one test is a substring of the
name of another test, it is not possible to use the '-p /the/name'
option to run a single test.
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240104162942.211458-7-berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/netdev-socket.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c
index 91441f7922..fc7d11961e 100644
--- a/tests/qtest/netdev-socket.c
+++ b/tests/qtest/netdev-socket.c
@@ -526,7 +526,7 @@ int main(int argc, char **argv)
#ifndef _WIN32
qtest_add_func("/netdev/dgram/unix", test_dgram_unix);
#endif
- qtest_add_func("/netdev/stream/unix", test_stream_unix);
+ qtest_add_func("/netdev/stream/unix/oneshot", test_stream_unix);
qtest_add_func("/netdev/stream/unix/reconnect",
test_stream_unix_reconnect);
#ifdef CONFIG_LINUX
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 09/17] tests/qtest/virtio-ccw: Fix device presence checking
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (7 preceding siblings ...)
2024-01-11 17:00 ` [PULL 08/17] qtest: ensure netdev-socket tests have non-overlapping names Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 10/17] gitlab: fix s390x tag for avocado-system-centos Thomas Huth
` (8 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Samuel Tardieu, Philippe Mathieu-Daudé
From: Samuel Tardieu <sam@rfc1149.net>
An apparent copy-paste error tests for the presence of the
virtio-rng-ccw device in order to perform tests on the virtio-scsi-ccw
device.
Signed-off-by: Samuel Tardieu <sam@rfc1149.net>
Message-ID: <20240106130121.1244993-1-sam@rfc1149.net>
Fixes: 65331bf5d1 ("tests/qtest: Check for virtio-ccw devices before using them")
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/qtest/virtio-ccw-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/qtest/virtio-ccw-test.c b/tests/qtest/virtio-ccw-test.c
index f4f5858b84..7a5357c212 100644
--- a/tests/qtest/virtio-ccw-test.c
+++ b/tests/qtest/virtio-ccw-test.c
@@ -85,7 +85,7 @@ int main(int argc, char **argv)
if (qtest_has_device("virtio-rng-ccw")) {
qtest_add_func("/virtio/rng/nop", virtio_rng_nop);
}
- if (qtest_has_device("virtio-rng-ccw")) {
+ if (qtest_has_device("virtio-scsi-ccw")) {
qtest_add_func("/virtio/scsi/nop", virtio_scsi_nop);
qtest_add_func("/virtio/scsi/hotplug", virtio_scsi_hotplug);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 10/17] gitlab: fix s390x tag for avocado-system-centos
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (8 preceding siblings ...)
2024-01-11 17:00 ` [PULL 09/17] tests/qtest/virtio-ccw: Fix device presence checking Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 11/17] target/s390x/kvm/pv: Provide some more useful information if decryption fails Thomas Huth
` (7 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Nicholas Piggin, Cédric Le Goater,
Philippe Mathieu-Daudé
From: Nicholas Piggin <npiggin@gmail.com>
The 390x tag should be s390x.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Message-ID: <20240107170119.82222-2-npiggin@gmail.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
.gitlab-ci.d/buildtest.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 91663946de..cfe737aca2 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -184,7 +184,7 @@ avocado-system-centos:
variables:
IMAGE: centos8
MAKE_CHECK_ARGS: check-avocado
- AVOCADO_TAGS: arch:ppc64 arch:or1k arch:390x arch:x86_64 arch:rx
+ AVOCADO_TAGS: arch:ppc64 arch:or1k arch:s390x arch:x86_64 arch:rx
arch:sh4 arch:nios2
build-system-opensuse:
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 11/17] target/s390x/kvm/pv: Provide some more useful information if decryption fails
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (9 preceding siblings ...)
2024-01-11 17:00 ` [PULL 10/17] gitlab: fix s390x tag for avocado-system-centos Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 12/17] hw/s390x/ccw: Replace basename() with g_path_get_basename() Thomas Huth
` (6 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Philippe Mathieu-Daudé, Cédric Le Goater,
Claudio Imbrenda
It's a common scenario to copy guest images from one host to another
to run the guest on the other machine. This (of course) does not work
with "secure execution" guests since they are encrypted with one certain
host key. However, if you still (accidentally) do it, you only get a
very user-unfriendly error message that looks like this:
qemu-system-s390x: KVM PV command 2 (KVM_PV_SET_SEC_PARMS) failed:
header rc 108 rrc 5 IOCTL rc: -22
Let's provide at least a somewhat nicer hint to the users so that they
are able to figure out what might have gone wrong.
Buglink: https://issues.redhat.com/browse/RHEL-18212
Message-ID: <20240110142916.850605-1-thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/s390x/ipl.h | 2 +-
target/s390x/kvm/pv.h | 5 +++--
hw/s390x/ipl.c | 5 ++---
hw/s390x/s390-virtio-ccw.c | 5 ++++-
target/s390x/kvm/pv.c | 25 ++++++++++++++++++++-----
5 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 7fc86e7905..57cd125769 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -107,7 +107,7 @@ typedef union IplParameterBlock IplParameterBlock;
int s390_ipl_set_loadparm(uint8_t *loadparm);
void s390_ipl_update_diag308(IplParameterBlock *iplb);
-int s390_ipl_prepare_pv_header(void);
+int s390_ipl_prepare_pv_header(Error **errp);
int s390_ipl_pv_unpack(void);
void s390_ipl_prepare_cpu(S390CPU *cpu);
IplParameterBlock *s390_ipl_get_iplb(void);
diff --git a/target/s390x/kvm/pv.h b/target/s390x/kvm/pv.h
index 7b935e2246..5877d28ff1 100644
--- a/target/s390x/kvm/pv.h
+++ b/target/s390x/kvm/pv.h
@@ -42,7 +42,7 @@ int s390_pv_query_info(void);
int s390_pv_vm_enable(void);
void s390_pv_vm_disable(void);
bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms);
-int s390_pv_set_sec_parms(uint64_t origin, uint64_t length);
+int s390_pv_set_sec_parms(uint64_t origin, uint64_t length, Error **errp);
int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak);
void s390_pv_prep_reset(void);
int s390_pv_verify(void);
@@ -62,7 +62,8 @@ static inline int s390_pv_query_info(void) { return 0; }
static inline int s390_pv_vm_enable(void) { return 0; }
static inline void s390_pv_vm_disable(void) {}
static inline bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms) { return false; }
-static inline int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; }
+static inline int s390_pv_set_sec_parms(uint64_t origin, uint64_t length,
+ Error **errp) { return 0; }
static inline int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0; }
static inline void s390_pv_prep_reset(void) {}
static inline int s390_pv_verify(void) { return 0; }
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 76110e8f58..e934bf89d1 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -702,7 +702,7 @@ static void s390_ipl_prepare_qipl(S390CPU *cpu)
cpu_physical_memory_unmap(addr, len, 1, len);
}
-int s390_ipl_prepare_pv_header(void)
+int s390_ipl_prepare_pv_header(Error **errp)
{
IplParameterBlock *ipib = s390_ipl_get_iplb_pv();
IPLBlockPV *ipib_pv = &ipib->pv;
@@ -711,8 +711,7 @@ int s390_ipl_prepare_pv_header(void)
cpu_physical_memory_read(ipib_pv->pv_header_addr, hdr,
ipib_pv->pv_header_len);
- rc = s390_pv_set_sec_parms((uintptr_t)hdr,
- ipib_pv->pv_header_len);
+ rc = s390_pv_set_sec_parms((uintptr_t)hdr, ipib_pv->pv_header_len, errp);
g_free(hdr);
return rc;
}
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1169e20b94..eaf61d3640 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -391,7 +391,7 @@ static int s390_machine_protect(S390CcwMachineState *ms)
}
/* Set SE header and unpack */
- rc = s390_ipl_prepare_pv_header();
+ rc = s390_ipl_prepare_pv_header(&local_err);
if (rc) {
goto out_err;
}
@@ -410,6 +410,9 @@ static int s390_machine_protect(S390CcwMachineState *ms)
return rc;
out_err:
+ if (local_err) {
+ error_report_err(local_err);
+ }
s390_machine_unprotect(ms);
return rc;
}
diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 6a69be7e5c..7ca7faec73 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -29,7 +29,8 @@ static bool info_valid;
static struct kvm_s390_pv_info_vm info_vm;
static struct kvm_s390_pv_info_dump info_dump;
-static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data)
+static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data,
+ int *pvrc)
{
struct kvm_pv_cmd pv_cmd = {
.cmd = cmd,
@@ -46,6 +47,9 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data)
"IOCTL rc: %d", cmd, cmdname, pv_cmd.rc, pv_cmd.rrc,
rc);
}
+ if (pvrc) {
+ *pvrc = pv_cmd.rc;
+ }
return rc;
}
@@ -53,12 +57,13 @@ static int __s390_pv_cmd(uint32_t cmd, const char *cmdname, void *data)
* This macro lets us pass the command as a string to the function so
* we can print it on an error.
*/
-#define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data)
+#define s390_pv_cmd(cmd, data) __s390_pv_cmd(cmd, #cmd, data, NULL)
+#define s390_pv_cmd_pvrc(cmd, data, pvrc) __s390_pv_cmd(cmd, #cmd, data, pvrc)
#define s390_pv_cmd_exit(cmd, data) \
{ \
int rc; \
\
- rc = __s390_pv_cmd(cmd, #cmd, data);\
+ rc = __s390_pv_cmd(cmd, #cmd, data, NULL); \
if (rc) { \
exit(1); \
} \
@@ -142,14 +147,24 @@ bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms)
return true;
}
-int s390_pv_set_sec_parms(uint64_t origin, uint64_t length)
+int s390_pv_set_sec_parms(uint64_t origin, uint64_t length, Error **errp)
{
+ int ret, pvrc;
struct kvm_s390_pv_sec_parm args = {
.origin = origin,
.length = length,
};
- return s390_pv_cmd(KVM_PV_SET_SEC_PARMS, &args);
+ ret = s390_pv_cmd_pvrc(KVM_PV_SET_SEC_PARMS, &args, &pvrc);
+ if (ret) {
+ error_setg(errp, "Failed to set secure execution parameters");
+ if (pvrc == 0x108) {
+ error_append_hint(errp, "Please check whether the image is "
+ "correctly encrypted for this host\n");
+ }
+ }
+
+ return ret;
}
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 12/17] hw/s390x/ccw: Replace basename() with g_path_get_basename()
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (10 preceding siblings ...)
2024-01-11 17:00 ` [PULL 11/17] target/s390x/kvm/pv: Provide some more useful information if decryption fails Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 13/17] hw/s390x/ccw: Replace dirname() with g_path_get_dirname() Thomas Huth
` (5 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu, Cédric Le Goater, Eric Farman
From: Zhao Liu <zhao1.liu@intel.com>
g_path_get_basename() is a portable utility function that has the
advantage of not modifying the string argument, so it should be
preferred over basename().
And also to avoid potential compile breakage with the Musl C library
similar to [1], replace basename() with g_path_get_basename().
[1]: https://lore.kernel.org/all/20231212010228.2701544-1-raj.khem@gmail.com/
Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Message-ID: <20231221171921.57784-2-zhao1.liu@linux.intel.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/s390x/s390-ccw.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index e2d86d96e7..ab7022a3ab 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -76,7 +76,8 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
Error **errp)
{
unsigned int cssid, ssid, devid;
- char dev_path[PATH_MAX] = {0}, *tmp;
+ char dev_path[PATH_MAX] = {0};
+ g_autofree char *tmp = NULL;
if (!sysfsdev) {
error_setg(errp, "No host device provided");
@@ -92,7 +93,7 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
cdev->mdevid = g_path_get_basename(dev_path);
- tmp = basename(dirname(dev_path));
+ tmp = g_path_get_basename(dirname(dev_path));
if (sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid) != 3) {
error_setg_errno(errp, errno, "Failed to read %s", tmp);
return;
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 13/17] hw/s390x/ccw: Replace dirname() with g_path_get_dirname()
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (11 preceding siblings ...)
2024-01-11 17:00 ` [PULL 12/17] hw/s390x/ccw: Replace basename() with g_path_get_basename() Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 14/17] scripts/checkpatch: Support codespell checking Thomas Huth
` (4 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu, Cédric Le Goater, Eric Farman
From: Zhao Liu <zhao1.liu@intel.com>
As commit 3e015d815b3f ("use g_path_get_basename instead of basename")
said, g_path_get_dirname() should be preferred over dirname() since
the former is a portable utility function that has the advantage of not
modifying the string argument.
Replace dirname() with g_path_get_dirname().
Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Message-ID: <20231221171921.57784-3-zhao1.liu@linux.intel.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Farman <farman@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
hw/s390x/s390-ccw.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index ab7022a3ab..5261e66724 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -77,6 +77,7 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
{
unsigned int cssid, ssid, devid;
char dev_path[PATH_MAX] = {0};
+ g_autofree char *tmp_dir = NULL;
g_autofree char *tmp = NULL;
if (!sysfsdev) {
@@ -93,7 +94,8 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
cdev->mdevid = g_path_get_basename(dev_path);
- tmp = g_path_get_basename(dirname(dev_path));
+ tmp_dir = g_path_get_dirname(dev_path);
+ tmp = g_path_get_basename(tmp_dir);
if (sscanf(tmp, "%2x.%1x.%4x", &cssid, &ssid, &devid) != 3) {
error_setg_errno(errp, errno, "Failed to read %s", tmp);
return;
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 14/17] scripts/checkpatch: Support codespell checking
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (12 preceding siblings ...)
2024-01-11 17:00 ` [PULL 13/17] hw/s390x/ccw: Replace dirname() with g_path_get_dirname() Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 15/17] target/s390x: Fix LAE setting a wrong access register Thomas Huth
` (3 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Zhao Liu, Yongwei Ma, Samuel Tardieu
From: Zhao Liu <zhao1.liu@intel.com>
Add two spelling check options (--codespell and --codespellfile) to
enhance spelling check through dictionary, which copied the Linux
kernel's implementation in checkpatch.pl.
This check uses the dictionary at "/usr/share/codespell/dictionary.txt"
by default, if there is no dictionary specified under this path, it
will look for the dictionary of python3's codespell (This requires user
to add python3's path in environment variable $PATH, and to install
codespell by "pip install codespell").
Tested-by: Yongwei Ma <yongwei.ma@intel.com>
Tested-by: Samuel Tardieu <sam@rfc1149.net>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
Message-ID: <20240105083848.267192-1-zhao1.liu@linux.intel.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
scripts/checkpatch.pl | 125 +++++++++++++++++++++++++++++++++++-------
1 file changed, 105 insertions(+), 20 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 6e4100d2a4..7026895074 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -35,6 +35,9 @@ my $summary_file = 0;
my $root;
my %debug;
my $help = 0;
+my $codespell = 0;
+my $codespellfile = "/usr/share/codespell/dictionary.txt";
+my $user_codespellfile = "";
sub help {
my ($exitcode) = @_;
@@ -66,6 +69,9 @@ Options:
is all off)
--test-only=WORD report only warnings/errors containing WORD
literally
+ --codespell Use the codespell dictionary for spelling/typos
+ (default: $codespellfile)
+ --codespellfile Use this codespell dictionary
--color[=WHEN] Use colors 'always', 'never', or only when output
is a terminal ('auto'). Default is 'auto'.
-h, --help, --version display this help and exit
@@ -85,28 +91,50 @@ foreach (@ARGV) {
}
GetOptions(
- 'q|quiet+' => \$quiet,
- 'tree!' => \$tree,
- 'signoff!' => \$chk_signoff,
- 'patch!' => \$chk_patch,
- 'branch!' => \$chk_branch,
- 'emacs!' => \$emacs,
- 'terse!' => \$terse,
- 'f|file!' => \$file,
- 'strict!' => \$no_warnings,
- 'root=s' => \$root,
- 'summary!' => \$summary,
- 'mailback!' => \$mailback,
- 'summary-file!' => \$summary_file,
-
- 'debug=s' => \%debug,
- 'test-only=s' => \$tst_only,
- 'color=s' => \$color,
- 'no-color' => sub { $color = 'never'; },
- 'h|help' => \$help,
- 'version' => \$help
+ 'q|quiet+' => \$quiet,
+ 'tree!' => \$tree,
+ 'signoff!' => \$chk_signoff,
+ 'patch!' => \$chk_patch,
+ 'branch!' => \$chk_branch,
+ 'emacs!' => \$emacs,
+ 'terse!' => \$terse,
+ 'f|file!' => \$file,
+ 'strict!' => \$no_warnings,
+ 'root=s' => \$root,
+ 'summary!' => \$summary,
+ 'mailback!' => \$mailback,
+ 'summary-file!' => \$summary_file,
+ 'debug=s' => \%debug,
+ 'test-only=s' => \$tst_only,
+ 'codespell!' => \$codespell,
+ 'codespellfile=s' => \$user_codespellfile,
+ 'color=s' => \$color,
+ 'no-color' => sub { $color = 'never'; },
+ 'h|help' => \$help,
+ 'version' => \$help
) or help(1);
+if ($user_codespellfile) {
+ # Use the user provided codespell file unconditionally
+ $codespellfile = $user_codespellfile;
+} elsif (!(-f $codespellfile)) {
+ # If /usr/share/codespell/dictionary.txt is not present, try to find it
+ # under codespell's install directory: <codespell_root>/data/dictionary.txt
+ if (($codespell || $help) && which("python3") ne "") {
+ my $python_codespell_dict = << "EOF";
+
+import os.path as op
+import codespell_lib
+codespell_dir = op.dirname(codespell_lib.__file__)
+codespell_file = op.join(codespell_dir, 'data', 'dictionary.txt')
+print(codespell_file, end='')
+EOF
+
+ my $codespell_dict = `python3 -c "$python_codespell_dict" 2> /dev/null`;
+ $codespellfile = $codespell_dict if (-f $codespell_dict);
+ }
+}
+
help(0) if ($help);
my $exit = 0;
@@ -337,6 +365,36 @@ our @typeList = (
qr{guintptr},
);
+# Load common spelling mistakes and build regular expression list.
+my $misspellings;
+my %spelling_fix;
+
+if ($codespell) {
+ if (open(my $spelling, '<', $codespellfile)) {
+ while (<$spelling>) {
+ my $line = $_;
+
+ $line =~ s/\s*\n?$//g;
+ $line =~ s/^\s*//g;
+
+ next if ($line =~ m/^\s*#/);
+ next if ($line =~ m/^\s*$/);
+ next if ($line =~ m/, disabled/i);
+
+ $line =~ s/,.*$//;
+
+ my ($suspect, $fix) = split(/->/, $line);
+
+ $spelling_fix{$suspect} = $fix;
+ }
+ close($spelling);
+ } else {
+ warn "No codespell typos will be found - file '$codespellfile': $!\n";
+ }
+}
+
+$misspellings = join("|", sort keys %spelling_fix) if keys %spelling_fix;
+
# This can be modified by sub possible. Since it can be empty, be careful
# about regexes that always match, because they can cause infinite loops.
our @modifierList = (
@@ -477,6 +535,18 @@ sub top_of_kernel_tree {
return 1;
}
+sub which {
+ my ($bin) = @_;
+
+ foreach my $path (split(/:/, $ENV{PATH})) {
+ if (-e "$path/$bin") {
+ return "$path/$bin";
+ }
+ }
+
+ return "";
+}
+
sub expand_tabs {
my ($str) = @_;
@@ -1585,6 +1655,21 @@ sub process {
WARN("8-bit UTF-8 used in possible commit log\n" . $herecurr);
}
+# Check for various typo / spelling mistakes
+ if (defined($misspellings) &&
+ ($in_commit_log || $line =~ /^(?:\+|Subject:)/i)) {
+ while ($rawline =~ /(?:^|[^\w\-'`])($misspellings)(?:[^\w\-'`]|$)/gi) {
+ my $typo = $1;
+ my $blank = copy_spacing($rawline);
+ my $ptr = substr($blank, 0, $-[1]) . "^" x length($typo);
+ my $hereptr = "$hereline$ptr\n";
+ my $typo_fix = $spelling_fix{lc($typo)};
+ $typo_fix = ucfirst($typo_fix) if ($typo =~ /^[A-Z]/);
+ $typo_fix = uc($typo_fix) if ($typo =~ /^[A-Z]+$/);
+ WARN("'$typo' may be misspelled - perhaps '$typo_fix'?\n" . $hereptr);
+ }
+ }
+
# ignore non-hunk lines and lines being removed
next if (!$hunk_line || $line =~ /^-/);
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 15/17] target/s390x: Fix LAE setting a wrong access register
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (13 preceding siblings ...)
2024-01-11 17:00 ` [PULL 14/17] scripts/checkpatch: Support codespell checking Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 16/17] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Thomas Huth
` (2 subsequent siblings)
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Ilya Leoshkevich, Ido Plat, qemu-stable,
David Hildenbrand
From: Ilya Leoshkevich <iii@linux.ibm.com>
LAE should set the access register corresponding to the first operand,
instead, it always modifies access register 1.
Co-developed-by: Ido Plat <Ido.Plat@ibm.com>
Cc: qemu-stable@nongnu.org
Fixes: a1c7610a6879 ("target-s390x: implement LAY and LAEY instructions")
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240111092328.929421-2-iii@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
target/s390x/tcg/translate.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index 62ab2be8b1..8df00b7df9 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -3221,6 +3221,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
{
int b2 = get_field(s, b2);
TCGv ar1 = tcg_temp_new_i64();
+ int r1 = get_field(s, r1);
o->out = o->in2;
o->in2 = NULL;
@@ -3244,7 +3245,7 @@ static DisasJumpType op_mov2e(DisasContext *s, DisasOps *o)
break;
}
- tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[1]));
+ tcg_gen_st32_i64(ar1, tcg_env, offsetof(CPUS390XState, aregs[r1]));
return DISAS_NEXT;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 16/17] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (14 preceding siblings ...)
2024-01-11 17:00 ` [PULL 15/17] target/s390x: Fix LAE setting a wrong access register Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-11 17:00 ` [PULL 17/17] .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large Thomas Huth
2024-01-12 17:27 ` [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Peter Maydell
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Ilya Leoshkevich
From: Ilya Leoshkevich <iii@linux.ibm.com>
Add a small test to prevent regressions. Userspace runs in primary
mode, so LAE should always set the access register to 0.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-ID: <20240111092328.929421-3-iii@linux.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/tcg/s390x/lae.c | 31 +++++++++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
2 files changed, 32 insertions(+)
create mode 100644 tests/tcg/s390x/lae.c
diff --git a/tests/tcg/s390x/lae.c b/tests/tcg/s390x/lae.c
new file mode 100644
index 0000000000..59712b5e37
--- /dev/null
+++ b/tests/tcg/s390x/lae.c
@@ -0,0 +1,31 @@
+/*
+ * Test the LOAD ADDRESS EXTENDED instruction.
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <assert.h>
+#include <stdlib.h>
+
+int main(void)
+{
+ unsigned long long ar = -1, b2 = 100000, r, x2 = 500;
+ /*
+ * Hardcode the register number, since clang does not allow using %rN in
+ * place of %aN.
+ */
+ register unsigned long long r2 __asm__("2");
+ int tmp;
+
+ asm("ear %[tmp],%%a2\n"
+ "lae %%r2,42(%[x2],%[b2])\n"
+ "ear %[ar],%%a2\n"
+ "sar %%a2,%[tmp]"
+ : [tmp] "=&r" (tmp), "=&r" (r2), [ar] "+r" (ar)
+ : [b2] "r" (b2), [x2] "r" (x2)
+ : "memory");
+ r = r2;
+ assert(ar == 0xffffffff00000000ULL);
+ assert(r == 100542);
+
+ return EXIT_SUCCESS;
+}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 0e670f3f8b..30994dcf9c 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -44,6 +44,7 @@ TESTS+=clgebr
TESTS+=clc
TESTS+=laalg
TESTS+=add-logical-with-carry
+TESTS+=lae
cdsg: CFLAGS+=-pthread
cdsg: LDFLAGS+=-pthread
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PULL 17/17] .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (15 preceding siblings ...)
2024-01-11 17:00 ` [PULL 16/17] tests/tcg/s390x: Test LOAD ADDRESS EXTENDED Thomas Huth
@ 2024-01-11 17:00 ` Thomas Huth
2024-01-12 17:27 ` [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Peter Maydell
17 siblings, 0 replies; 19+ messages in thread
From: Thomas Huth @ 2024-01-11 17:00 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, qemu-stable, Philippe Mathieu-Daudé
From: Peter Maydell <peter.maydell@linaro.org>
Sometimes the CI "pages" job fails with a message like this from
htags:
$ htags -anT --tree-view=filetree -m qemu_init -t "Welcome to the QEMU sourcecode"
htags: Negative exec line limit = -371
This is due to a bug in hflags where if the environment is too large it
falls over:
https://lists.gnu.org/archive/html/bug-global/2024-01/msg00000.html
This happens to us because GitLab CI puts the commit message of the
commit under test into the CI_COMMIT_MESSAGE and/or CI_COMMIT_TAG_MESSAGE
environment variables, so the job will fail if the commit happens to
have a verbose commit message.
Work around the htags bug by unsetting these variables while running
htags.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2080
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-ID: <20240111125543.1573473-1-peter.maydell@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
.gitlab-ci.d/buildtest.yml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index cfe737aca2..9b4df24e9a 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -647,7 +647,10 @@ pages:
- mkdir -p public
# HTML-ised source tree
- make gtags
- - htags -anT --tree-view=filetree -m qemu_init
+ # We unset variables to work around a bug in some htags versions
+ # which causes it to fail when the environment is large
+ - CI_COMMIT_MESSAGE= CI_COMMIT_TAG_MESSAGE= htags
+ -anT --tree-view=filetree -m qemu_init
-t "Welcome to the QEMU sourcecode"
- mv HTML public/src
# Project documentation
--
2.43.0
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci)
2024-01-11 17:00 [PULL 00/17] Misc patches (qtests, s390x, m68k, gitlab-ci) Thomas Huth
` (16 preceding siblings ...)
2024-01-11 17:00 ` [PULL 17/17] .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large Thomas Huth
@ 2024-01-12 17:27 ` Peter Maydell
17 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2024-01-12 17:27 UTC (permalink / raw)
To: Thomas Huth; +Cc: qemu-devel
On Thu, 11 Jan 2024 at 17:00, Thomas Huth <thuth@redhat.com> wrote:
>
> Hi Peter!
>
> The following changes since commit 34eac35f893664eb8545b98142e23d9954722766:
>
> Merge tag 'pull-riscv-to-apply-20240110' of https://github.com/alistair23/qemu into staging (2024-01-10 11:41:56 +0000)
>
> are available in the Git repository at:
>
> https://gitlab.com/thuth/qemu.git tags/pull-request-2024-01-11
>
> for you to fetch changes up to 52a21689cd829c1cc931b59b5ee5bdb10dd578c1:
>
> .gitlab-ci.d/buildtest.yml: Work around htags bug when environment is large (2024-01-11 17:49:21 +0100)
>
> ----------------------------------------------------------------
> * Fix non-deterministic failures of the 'netdev-socket' qtest
> * Fix device presence checking in the virtio-ccw qtest
> * Support codespell checking in checkpatch.pl
> * Fix emulation of LAE s390x instruction
> * Work around htags bug when environment is large
> * Some other small clean-ups here and there
>
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread