Re: [PATCH] iothread: Simplify expression in qemu_in_iothread()
Le 08/02/2024 à 11:16, Kevin Wolf a écrit :
'a == b ? false : true' is a rather convoluted way of writing 'a != b'.
Use the more obvious way to write it.
Signed-off-by: Kevin Wolf
---
iothread.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/iothread.c b/iothread.c
index 6c1fc8c856..e1e9e04736 100644
--- a/iothread.c
+++ b/iothread.c
@@ -404,6 +404,5 @@ IOThread *iothread_by_id(const char *id)
bool qemu_in_iothread(void)
{
-return qemu_get_current_aio_context() == qemu_get_aio_context() ?
-false : true;
+return qemu_get_current_aio_context() != qemu_get_aio_context();
}
Reviewed-by: Laurent Vivier
Re: [PULL 1/6] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
Le 12/05/2023 à 14:08, John Paul Adrian Glaubitz a écrit : Hello Laurent! On Fri, 2023-05-12 at 13:13 +0200, Laurent Vivier wrote: This patch breaks something with LTP (20230127) test fchown05_16 on sid/sparc64: tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 702 701 fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 704 fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 705 expected result; tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed Where do these tests reside? I'm not sure I know what LTP is. In any case, the patch should be correct as QEMU needs to differentiate between 32-bit and 64-bit SPARC. I agree, it could be a side effect. I didn't check. https://github.com/linux-test-project/ltp/releases/tag/20230127 Thanks, Laurent
Re: [PULL 1/6] linux-user/sparc: Don't use 16-bit UIDs on SPARC V9
On 3/30/23 15:18, Philippe Mathieu-Daudé wrote:
The 64-bit SPARC V9 syscall ABI uses 32-bit UIDs. Only enable
the 16-bit UID wrappers for 32-bit SPARC (V7 and V8).
Possibly missed in commit 992f48a036 ("Support for 32 bit
ABI on 64 bit targets (only enabled Sparc64)").
Reported-by: Gregor Riepl
Tested-by: John Paul Adrian Glaubitz
Tested-by: Zach van Rijn
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1394
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Richard Henderson
Acked-by: Laurent Vivier
Message-Id: <20230327131910.78564-1-phi...@linaro.org>
---
linux-user/syscall_defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 614a1cbc8e..cc37054cb5 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -61,7 +61,7 @@
#if (defined(TARGET_I386) && defined(TARGET_ABI32)) \
|| (defined(TARGET_ARM) && defined(TARGET_ABI32)) \
-|| defined(TARGET_SPARC) \
+|| (defined(TARGET_SPARC) && defined(TARGET_ABI32)) \
|| defined(TARGET_M68K) || defined(TARGET_SH4) || defined(TARGET_CRIS)
/* 16 bit uid wrappers emulation */
#define USE_UID16
This patch breaks something with LTP (20230127) test fchown05_16 on sid/sparc64:
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed
fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 702 701
fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed
fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 704
fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed
fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed
fchown05.c:49: TFAIL: testfile: incorrect ownership set, expected 703 705
expected result;
tst_test.c:1558: TINFO: Timeout per run is 0h 00m 30s
fchown05.c:44: TPASS: fchown(3, 700, 701), change owner/group ids passed
fchown05.c:44: TPASS: fchown(3, 702, -1), change owner id only passed
fchown05.c:44: TPASS: fchown(3, 703, 701), change owner id only passed
fchown05.c:44: TPASS: fchown(3, -1, 704), change group id only passed
fchown05.c:44: TPASS: fchown(3, 703, 705), change group id only passed
fchown05.c:44: TPASS: fchown(3, -1, -1), no change passed
Thanks,
Laurent
Re: [PATCH-for-8.0] block/nbd: Add missing include
Le 12/01/2023 à 13:00, Philippe Mathieu-Daudé a écrit : Hi, can this reviewed patch get merged via a block tree? I can take this via trivial. Thanks, Laurent On 25/11/22 18:53, Philippe Mathieu-Daudé wrote: The inlined nbd_readXX() functions call beXX_to_cpu(), themselves declared in . This fixes when refactoring: In file included from ../../block/nbd.c:44: include/block/nbd.h: In function 'nbd_read16': include/block/nbd.h:383:12: error: implicit declaration of function 'be16_to_cpu' [-Werror=implicit-function-declaration] 383 | *val = be##bits##_to_cpu(*val); \ | ^~ include/block/nbd.h:387:1: note: in expansion of macro 'DEF_NBD_READ_N' 387 | DEF_NBD_READ_N(16) /* Defines nbd_read16(). */ | ^~ Signed-off-by: Philippe Mathieu-Daudé --- include/block/nbd.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/block/nbd.h b/include/block/nbd.h index 4ede3b2bd0..a4c98169c3 100644 --- a/include/block/nbd.h +++ b/include/block/nbd.h @@ -24,6 +24,7 @@ #include "io/channel-socket.h" #include "crypto/tlscreds.h" #include "qapi/error.h" +#include "qemu/bswap.h" extern const BlockExportDriver blk_exp_nbd;
Re: [PATCH] block/qcow2-bitmap: Add missing cast to silent GCC error
Le 22/09/2022 à 17:37, Kevin Wolf a écrit :
Am 19.09.2022 um 20:27 hat Philippe Mathieu-Daudé geschrieben:
Commit d1258dd0c8 ("qcow2: autoloading dirty bitmaps") added the
set_readonly_helper() GFunc handler, correctly casting the gpointer
user_data in both the g_slist_foreach() caller and the handler.
Few commits later (commit 1b6b0562db), the handler is reused in
qcow2_reopen_bitmaps_rw() but missing the gpointer cast, resulting
in the following error when using Homebrew GCC 12.2.0:
[2/658] Compiling C object libblock.fa.p/block_qcow2-bitmap.c.o
../../block/qcow2-bitmap.c: In function 'qcow2_reopen_bitmaps_rw':
../../block/qcow2-bitmap.c:1211:60: error: incompatible type for argument 3
of 'g_slist_foreach'
1211 | g_slist_foreach(ro_dirty_bitmaps, set_readonly_helper, false);
|^
||
|_Bool
In file included from
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gmain.h:26,
from
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/giochannel.h:33,
from
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib.h:54,
from /Users/philmd/source/qemu/include/glib-compat.h:32,
from /Users/philmd/source/qemu/include/qemu/osdep.h:144,
from ../../block/qcow2-bitmap.c:28:
/opt/homebrew/Cellar/glib/2.72.3_1/include/glib-2.0/glib/gslist.h:127:61:
note: expected 'gpointer' {aka 'void *'} but argument is of type '_Bool'
127 | gpointer
user_data);
| ~~^
At top level:
FAILED: libblock.fa.p/block_qcow2-bitmap.c.o
Fix by adding the missing gpointer cast.
Fixes: 1b6b0562db ("qcow2: support .bdrv_reopen_bitmaps_rw")
Signed-off-by: Philippe Mathieu-Daudé
Thanks, applied to the block branch. And in case qemu-trivial picks it
up anyway:
Reviewed-by: Kevin Wolf
It doesn't seem to be merged yet. Applied to my trivial-patches branch.
If you push it first I will remove it.
Thanks,
Laurent
Re: [PATCH 4/4] hw/ide/piix: Ignore writes of hardwired PCI command register bits
CC:
John Snow (supporter:IDE)
qemu-block@nongnu.org (open list:IDE)
Le 28/05/2022 à 23:02, Lev Kujawski a écrit :
One method to enable PCI bus mastering for IDE controllers, often used
by x86 firmware, is to write 0x7 to the PCI command register. Neither
the PIIX3 specification nor actual hardware (a Tyan S1686D system)
permit modification of the Memory Space Enable (MSE) bit, 1, and thus
the command register would be left in an unspecified state without
this patch.
Signed-off-by: Lev Kujawski
---
hw/ide/piix.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 76ea8fd9f6..f1d1168ecd 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -25,6 +25,8 @@
* References:
* [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
* 290550-002, Intel Corporation, April 1997.
+ * [2] 82371AB PCI-TO-ISA / IDE XCELERATOR (PIIX4), 290562-001,
+ * Intel Corporation, April 1997.
*/
#include "qemu/osdep.h"
@@ -32,6 +34,7 @@
#include "migration/vmstate.h"
#include "qapi/error.h"
#include "qemu/module.h"
+#include "qemu/range.h"
#include "sysemu/block-backend.h"
#include "sysemu/blockdev.h"
#include "sysemu/dma.h"
@@ -220,6 +223,26 @@ static void pci_piix_ide_exitfn(PCIDevice *dev)
}
}
+static void piix_pci_config_write(PCIDevice *d, uint32_t addr,
+ uint32_t val, int l)
+{
+/*
+ * Mask all IDE PCI command register bits except for Bus Master
+ * Function Enable (bit 2) and I/O Space Enable (bit 1), as the
+ * remainder are hardwired to 0 [1, p.48] [2, p.89-90].
+ *
+ * NOTE: According to the PIIX3 datasheet [1], the Memory Space
+ * Enable (MSE bit) is hardwired to 1, but this is contradicted by
+ * actual PIIX3 hardware, the datasheet itself (viz., Default
+ * Value: h), and the PIIX4 datasheet [2].
+ */
+if (range_covers_byte(addr, l, PCI_COMMAND)) {
+val &= ~(0xfffa << ((PCI_COMMAND - addr) << 3));
+}
+
+pci_default_write_config(d, addr, val, l);
+}
+
/* NOTE: for the PIIX3, the IRQs and IOports are hardcoded */
static void piix3_ide_class_init(ObjectClass *klass, void *data)
{
@@ -232,6 +255,7 @@ static void piix3_ide_class_init(ObjectClass *klass, void
*data)
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
}
@@ -260,6 +284,7 @@ static void piix4_ide_class_init(ObjectClass *klass, void
*data)
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
k->class_id = PCI_CLASS_STORAGE_IDE;
+k->config_write = piix_pci_config_write;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
dc->hotpluggable = false;
}
Re: [PATCH 2/4] hw/ide/core: Clear LBA and drive bits for EXECUTE DEVICE DIAGNOSTIC
CC:
John Snow (supporter:IDE)
qemu-block@nongnu.org (open list:IDE)
Le 28/05/2022 à 23:02, Lev Kujawski a écrit :
Prior to this patch, cmd_exec_dev_diagnostic relied upon
ide_set_signature to clear the device register. While the
preservation of the drive bit by ide_set_signature is necessary for
the DEVICE RESET, IDENTIFY DEVICE, and READ SECTOR commands,
ATA/ATAPI-6 specifies that "DEV shall be cleared to zero" for EXECUTE
DEVICE DIAGNOSTIC.
This deviation was uncovered by the ATACT Device Testing Program
written by Hale Landis.
Signed-off-by: Lev Kujawski
---
hw/ide/core.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index c2caa54285..5a24547e49 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1704,8 +1704,14 @@ static bool cmd_identify_packet(IDEState *s, uint8_t cmd)
return false;
}
+/* EXECUTE DEVICE DIAGNOSTIC */
static bool cmd_exec_dev_diagnostic(IDEState *s, uint8_t cmd)
{
+/*
+ * Clear the device register per the ATA (v6) specification,
+ * because ide_set_signature does not clear LBA or drive bits.
+ */
+s->select = (ATA_DEV_ALWAYS_ON);
ide_set_signature(s);
if (s->drive_kind == IDE_CD) {
Re: [PATCH 3/4] piix_ide_reset: Use pci_set_* functions instead of direct access
CC:
John Snow (supporter:IDE)
qemu-block@nongnu.org (open list:IDE)
Le 28/05/2022 à 23:02, Lev Kujawski a écrit :
Eliminates the remaining TODOs in hw/ide/piix.c by:
- Using pci_set_{size} functions to write the PIIX PCI configuration
space instead of manipulating it directly as an array; and
- Documenting default register values by reference to the controlling
specification.
Signed-off-by: Lev Kujawski
---
hw/ide/piix.c | 17 +
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index ce89fd0aa3..76ea8fd9f6 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -21,6 +21,10 @@
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
+ *
+ * References:
+ * [1] 82371FB (PIIX) AND 82371SB (PIIX3) PCI ISA IDE XCELERATOR,
+ * 290550-002, Intel Corporation, April 1997.
*/
#include "qemu/osdep.h"
@@ -114,14 +118,11 @@ static void piix_ide_reset(DeviceState *dev)
ide_bus_reset(>bus[i]);
}
-/* TODO: this is the default. do not override. */
-pci_conf[PCI_COMMAND] = 0x00;
-/* TODO: this is the default. do not override. */
-pci_conf[PCI_COMMAND + 1] = 0x00;
-/* TODO: use pci_set_word */
-pci_conf[PCI_STATUS] = PCI_STATUS_FAST_BACK;
-pci_conf[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM >> 8;
-pci_conf[0x20] = 0x01; /* BMIBA: 20-23h */
+/* PCI command register default value (h) per [1, p.48]. */
+pci_set_word(pci_conf + PCI_COMMAND, 0x);
+pci_set_word(pci_conf + PCI_STATUS,
+ PCI_STATUS_DEVSEL_MEDIUM | PCI_STATUS_FAST_BACK);
+pci_set_byte(pci_conf + 0x20, 0x01); /* BMIBA: 20-23h */
}
static int pci_piix_init_ports(PCIIDEState *d)
Re: [PATCH 1/2] Trivial: 3 char repeat typos
Le 14/06/2022 à 12:40, Dr. David Alan Gilbert (git) a écrit :
From: "Dr. David Alan Gilbert"
Inspired by Julia Lawall's fixing of Linux
kernel comments, I looked at qemu, although I did it manually.
Signed-off-by: Dr. David Alan Gilbert
---
hw/intc/openpic.c| 2 +-
hw/net/imx_fec.c | 2 +-
hw/pci/pcie_aer.c| 2 +-
hw/pci/shpc.c| 3 ++-
hw/ppc/spapr_caps.c | 2 +-
hw/scsi/spapr_vscsi.c| 2 +-
qapi/net.json| 2 +-
tools/virtiofsd/passthrough_ll.c | 2 +-
ui/input.c | 2 +-
9 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
index 49504e740f..b0787e8ee7 100644
--- a/hw/intc/openpic.c
+++ b/hw/intc/openpic.c
@@ -729,7 +729,7 @@ static void openpic_tmr_set_tmr(OpenPICTimer *tmr, uint32_t
val, bool enabled)
}
/*
- * Returns the currrent tccr value, i.e., timer value (in clocks) with
+ * Returns the current tccr value, i.e., timer value (in clocks) with
* appropriate TOG.
*/
static uint64_t openpic_tmr_get_timer(OpenPICTimer *tmr)
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 0db9aaf76a..8c11b237de 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -438,7 +438,7 @@ static void imx_eth_update(IMXFECState *s)
* assignment fail.
*
* To ensure that all versions of Linux work, generate ENET_INT_MAC
- * interrrupts on both interrupt lines. This should be changed if and when
+ * interrupts on both interrupt lines. This should be changed if and when
* qemu supports IOMUX.
*/
if (s->regs[ENET_EIR] & s->regs[ENET_EIMR] &
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 92bd0530dd..eff62f3945 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -323,7 +323,7 @@ static void pcie_aer_msg_root_port(PCIDevice *dev, const
PCIEAERMsg *msg)
*/
}
-/* Errro Message Received: Root Error Status register */
+/* Error Message Received: Root Error Status register */
switch (msg->severity) {
case PCI_ERR_ROOT_CMD_COR_EN:
if (root_status & PCI_ERR_ROOT_COR_RCV) {
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index f822f18b98..e71f3a7483 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -480,7 +480,8 @@ static const MemoryRegionOps shpc_mmio_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
.valid = {
/* SHPC ECN requires dword accesses, but the original 1.0 spec
doesn't.
- * It's easier to suppport all sizes than worry about it. */
+ * It's easier to support all sizes than worry about it.
+ */
.min_access_size = 1,
.max_access_size = 4,
},
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 655ab856a0..b4283055c1 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -553,7 +553,7 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr,
uint8_t val,
* instruction is a harmless no-op. It won't correctly
* implement the cache count flush *but* if we have
* count-cache-disabled in the host, that flush is
- * unnnecessary. So, specifically allow this case. This
+ * unnecessary. So, specifically allow this case. This
* allows us to have better performance on POWER9 DD2.3,
* while still working on POWER9 DD2.2 and POWER8 host
* cpus.
diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index a07a8e1523..e320ccaa23 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -1013,7 +1013,7 @@ static int vscsi_send_capabilities(VSCSIState *s,
vscsi_req *req)
}
/*
- * Current implementation does not suppport any migration or
+ * Current implementation does not support any migration or
* reservation capabilities. Construct the response telling the
* guest not to use them.
*/
diff --git a/qapi/net.json b/qapi/net.json
index d6f7cfd4d6..9af11e9a3b 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -298,7 +298,7 @@
#
# @udp: use the udp version of l2tpv3 encapsulation
#
-# @cookie64: use 64 bit coookies
+# @cookie64: use 64 bit cookies
#
# @counter: have sequence counter
#
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index b15c631ca5..7a73dfcce9 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -2319,7 +2319,7 @@ static int do_lo_create(fuse_req_t req, struct lo_inode
*parent_inode,
* If security.selinux has not been remapped and selinux is enabled,
* use fscreate to set context before file creation. If not, use
* tmpfile method for regular files. Otherwise fallback to
- * non-atomic method of file creation and xattr settting.
+ * non-atomic method of file creation and xattr setting.
*/
if
Re: [PATCH 2/2] trivial typos: namesapce
Le 14/06/2022 à 12:40, Dr. David Alan Gilbert (git) a écrit :
From: "Dr. David Alan Gilbert"
'namespace' is misspelled in a bunch of places.
Signed-off-by: Dr. David Alan Gilbert
---
hw/9pfs/9p-xattr-user.c | 8
hw/acpi/nvdimm.c| 2 +-
hw/nvme/ctrl.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/9pfs/9p-xattr-user.c b/hw/9pfs/9p-xattr-user.c
index f2ae9582e6..535677ed60 100644
--- a/hw/9pfs/9p-xattr-user.c
+++ b/hw/9pfs/9p-xattr-user.c
@@ -27,7 +27,7 @@ static ssize_t mp_user_getxattr(FsContext *ctx, const char
*path,
{
if (strncmp(name, "user.virtfs.", 12) == 0) {
/*
- * Don't allow fetch of user.virtfs namesapce
+ * Don't allow fetch of user.virtfs namespace
* in case of mapped security
*/
errno = ENOATTR;
@@ -49,7 +49,7 @@ static ssize_t mp_user_listxattr(FsContext *ctx, const char
*path,
name_size -= 12;
} else {
/*
- * Don't allow fetch of user.virtfs namesapce
+ * Don't allow fetch of user.virtfs namespace
* in case of mapped security
*/
return 0;
@@ -74,7 +74,7 @@ static int mp_user_setxattr(FsContext *ctx, const char *path,
const char *name,
{
if (strncmp(name, "user.virtfs.", 12) == 0) {
/*
- * Don't allow fetch of user.virtfs namesapce
+ * Don't allow fetch of user.virtfs namespace
* in case of mapped security
*/
errno = EACCES;
@@ -88,7 +88,7 @@ static int mp_user_removexattr(FsContext *ctx,
{
if (strncmp(name, "user.virtfs.", 12) == 0) {
/*
- * Don't allow fetch of user.virtfs namesapce
+ * Don't allow fetch of user.virtfs namespace
* in case of mapped security
*/
errno = EACCES;
diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
index 0d43da19ea..5f85b16327 100644
--- a/hw/acpi/nvdimm.c
+++ b/hw/acpi/nvdimm.c
@@ -476,7 +476,7 @@ struct NvdimmFuncGetLabelDataOut {
/* the size of buffer filled by QEMU. */
uint32_t len;
uint32_t func_ret_status; /* return status code. */
-uint8_t out_buf[]; /* the data got via Get Namesapce Label function. */
+uint8_t out_buf[]; /* the data got via Get Namespace Label function. */
} QEMU_PACKED;
typedef struct NvdimmFuncGetLabelDataOut NvdimmFuncGetLabelDataOut;
QEMU_BUILD_BUG_ON(sizeof(NvdimmFuncGetLabelDataOut) > NVDIMM_DSM_MEMORY_SIZE);
diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 1e6e0fcad9..770a38381a 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -71,7 +71,7 @@
* the SUBNQN field in the controller will report the NQN of the subsystem
* device. This also enables multi controller capability represented in
* Identify Controller data structure in CMIC (Controller Multi-path I/O and
- * Namesapce Sharing Capabilities).
+ * Namespace Sharing Capabilities).
*
* - `aerl`
* The Asynchronous Event Request Limit (AERL). Indicates the maximum number
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v2 03/11] scsi-disk: add MODE_PAGE_APPLE_VENDOR quirk for Macintosh
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
One of the mechanisms MacOS uses to identify drives compatible with MacOS is to
send a custom MODE SELECT command for page 0x30 to the drive. The response to
this is a hard-coded manufacturer string which must match in order for the
drive to be usable within MacOS.
Add an implementation of the MODE SELECT page 0x30 response guarded by a newly
defined SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR quirk bit so that drives attached
to non-Apple machines function exactly as before.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/scsi-disk.c | 17 +
include/hw/scsi/scsi.h | 3 +++
include/scsi/constants.h | 1 +
3 files changed, 21 insertions(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d89cdd4e4a..5de4506b97 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -1085,6 +1085,7 @@ static int mode_sense_page(SCSIDiskState *s, int page,
uint8_t **p_outbuf,
[MODE_PAGE_R_W_ERROR] = (1 << TYPE_DISK) | (1 <<
TYPE_ROM),
[MODE_PAGE_AUDIO_CTL] = (1 << TYPE_ROM),
[MODE_PAGE_CAPABILITIES] = (1 << TYPE_ROM),
+[MODE_PAGE_APPLE_VENDOR] = (1 << TYPE_ROM),
};
uint8_t *p = *p_outbuf + 2;
@@ -1229,6 +1230,20 @@ static int mode_sense_page(SCSIDiskState *s, int page,
uint8_t **p_outbuf,
p[19] = (16 * 176) & 0xff;
break;
+ case MODE_PAGE_APPLE_VENDOR:
+if (s->quirks & (1 << SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR)) {
+length = 0x24;
+if (page_control == 1) { /* Changeable Values */
+break;
+}
+
+memset(p, 0, length);
+strcpy((char *)p + 8, "APPLE COMPUTER, INC ");
+break;
+} else {
+return -1;
+}
+
default:
return -1;
}
@@ -3042,6 +3057,8 @@ static Property scsi_hd_properties[] = {
DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
DEFINE_PROP_INT32("scsi_version", SCSIDiskState,
qdev.default_scsi_version,
5),
+DEFINE_PROP_BIT("quirk_mode_page_apple_vendor", SCSIDiskState, quirks,
+SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR, 0),
DEFINE_BLOCK_CHS_PROPERTIES(SCSIDiskState, qdev.conf),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h
index 1ffb367f94..975d462347 100644
--- a/include/hw/scsi/scsi.h
+++ b/include/hw/scsi/scsi.h
@@ -226,4 +226,7 @@ SCSIDevice *scsi_device_get(SCSIBus *bus, int channel, int
target, int lun);
/* scsi-generic.c. */
extern const SCSIReqOps scsi_generic_req_ops;
+/* scsi-disk.c */
+#define SCSI_DISK_QUIRK_MODE_PAGE_APPLE_VENDOR 0
+
#endif
diff --git a/include/scsi/constants.h b/include/scsi/constants.h
index 2a32c08b5e..891aa0f45c 100644
--- a/include/scsi/constants.h
+++ b/include/scsi/constants.h
@@ -234,6 +234,7 @@
#define MODE_PAGE_FAULT_FAIL 0x1c
#define MODE_PAGE_TO_PROTECT 0x1d
#define MODE_PAGE_CAPABILITIES0x2a
+#define MODE_PAGE_APPLE_VENDOR0x30
#define MODE_PAGE_ALLS0x3f
/* Not in Mt. Fuji, but in ATAPI 2.6 -- deprecated now in favor
* of MODE_PAGE_SENSE_POWER */
Reviewed-by: Laurent Vivier
Re: [PATCH v2 04/11] q800: implement compat_props to enable quirk_mode_page_apple_vendor for scsi-hd devices
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
By default quirk_mode_page_apple_vendor should be enabled for all scsi-hd
devices
connected to the q800 machine to enable MacOS to detect and use them.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 099a758c6f..42bf7bb4f0 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -686,6 +686,11 @@ static void q800_init(MachineState *machine)
}
}
+static GlobalProperty hw_compat_q800[] = {
+{ "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+};
+static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
+
static void q800_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -695,6 +700,7 @@ static void q800_machine_class_init(ObjectClass *oc, void
*data)
mc->max_cpus = 1;
mc->block_default_type = IF_SCSI;
mc->default_ram_id = "m68k_mac.ram";
+compat_props_add(mc->compat_props, hw_compat_q800, hw_compat_q800_len);
}
static const TypeInfo q800_machine_typeinfo = {
Reviewed-by: Laurent Vivier
Re: [PATCH v2 06/11] q800: implement compat_props to enable quirk_mode_sense_rom_force_dbd for scsi-cd devices
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
By default quirk_mode_sense_rom_force_dbd should be enabled for all scsi-cd
devices
connected to the q800 machine to correctly report the CDROM block descriptor
back
to A/UX.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 42bf7bb4f0..f27ed01785 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -688,6 +688,7 @@ static void q800_init(MachineState *machine)
static GlobalProperty hw_compat_q800[] = {
{ "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+{ "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
};
static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
Reviewed-by: Laurent Vivier
Re: [PATCH v2 11/11] q800: add default vendor and product information for scsi-cd devices
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
The MacOS CDROM driver uses a SCSI INQUIRY command to check that any SCSI CDROMs
detected match a whitelist of vendors and products before adding them to the
list of available devices.
Add known-good default vendor and product information using the existing
compat_prop mechanism so the user doesn't have to use long command lines to set
the qdev properties manually.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index abb549f8d8..8b34776c8e 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -692,6 +692,9 @@ static GlobalProperty hw_compat_q800[] = {
{ "scsi-hd", "product", " ST225N" },
{ "scsi-hd", "ver", "1.0 " },
{ "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
+{ "scsi-cd", "vendor", "MATSHITA" },
+{ "scsi-cd", "product", "CD-ROM CR-8005" },
+{ "scsi-cd", "ver", "1.0k" },
};
static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
Reviewed-by: Laurent Vivier
Re: [PATCH v2 02/11] scsi-disk: add new quirks bitmap to SCSIDiskState
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
Since the MacOS SCSI implementation is quite old (and Apple added some firmware
customisations to their drives for m68k Macs) there is need to add a mechanism
to correctly handle Apple-specific quirks.
Add a new quirks bitmap to SCSIDiskState that can be used to enable these
features as required.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/scsi-disk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 090679f3b5..d89cdd4e4a 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -94,6 +94,7 @@ struct SCSIDiskState {
uint16_t port_index;
uint64_t max_unmap_size;
uint64_t max_io_size;
+uint32_t quirks;
QEMUBH *bh;
char *version;
char *serial;
Reviewed-by: Laurent Vivier
Re: [PATCH v2 10/11] q800: add default vendor and product information for scsi-hd devices
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit :
The Apple HD SC Setup program uses a SCSI INQUIRY command to check that any SCSI
hard disks detected match a whitelist of vendors and products before allowing
the "Initialise" button to prepare an empty disk.
Add known-good default vendor and product information using the existing
compat_prop mechanism so the user doesn't have to use long command lines to set
the qdev properties manually.
Signed-off-by: Mark Cave-Ayland
---
hw/m68k/q800.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index f27ed01785..abb549f8d8 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -688,6 +688,9 @@ static void q800_init(MachineState *machine)
static GlobalProperty hw_compat_q800[] = {
{ "scsi-hd", "quirk_mode_page_apple_vendor", "on"},
+{ "scsi-hd", "vendor", " SEAGATE" },
+{ "scsi-hd", "product", " ST225N" },
+{ "scsi-hd", "ver", "1.0 " },
{ "scsi-cd", "quirk_mode_sense_rom_force_dbd", "on"},
};
static const size_t hw_compat_q800_len = G_N_ELEMENTS(hw_compat_q800);
Reviewed-by: Laurent Vivier
Re: [PATCH v2 01/11] scsi-disk: add FORMAT UNIT command
Le 24/04/2022 à 18:49, Mark Cave-Ayland a écrit : When initialising a drive ready to install MacOS, Apple HD SC Setup first attempts to format the drive. Add a simple FORMAT UNIT command which simply returns success to allow the format to succeed. Signed-off-by: Mark Cave-Ayland --- hw/scsi/scsi-disk.c | 4 hw/scsi/trace-events | 1 + 2 files changed, 5 insertions(+) Reviewed-by: Laurent Vivier
Re: [PATCH] tests/qtest: Move the fuzz tests to x86 only
On 14/04/2022 15:01, Thomas Huth wrote: The fuzz tests are currently scheduled for all targets, but their setup code limits the run to "i386", so that these tests always show "SKIP" on other targets. Move it to the right x86 list in meson.build, then we can drop the architecture check during runtime, too. Signed-off-by: Thomas Huth --- tests/qtest/fuzz-lsi53c895a-test.c | 8 ++-- tests/qtest/fuzz-megasas-test.c | 12 tests/qtest/fuzz-sb16-test.c| 12 tests/qtest/fuzz-sdcard-test.c | 12 tests/qtest/fuzz-virtio-scsi-test.c | 8 ++-- tests/qtest/meson.build | 13 ++--- 6 files changed, 22 insertions(+), 43 deletions(-) Reviewed-by: Laurent Vivier
Re: [PATCH v2 4/5] Replace qemu_gettimeofday() with g_get_real_time()
Le 07/03/2022 à 11:35, Marc-André Lureau a écrit :
Hi
On Mon, Mar 7, 2022 at 2:13 PM Laurent Vivier mailto:laur...@vivier.eu>> wrote:
Le 05/03/2022 à 20:17, Marc-André Lureau a écrit :
> On Sat, Mar 5, 2022 at 1:18 AM mailto:marcandre.lur...@redhat.com>> wrote:
>>
>> From: Marc-André Lureau mailto:marcandre.lur...@redhat.com>>
>>
>> GLib g_get_real_time() is an alternative to gettimeofday() which allows
>> to simplify our code.
>>
>> For semihosting, a few bits are lost on POSIX host, but this shouldn't
>> be a big concern.
>>
>> Signed-off-by: Marc-André Lureau mailto:marcandre.lur...@redhat.com>>
>> Reviewed-by: Laurent Vivier mailto:laur...@vivier.eu>>
>> ---
>> blockdev.c | 8
>> hw/rtc/m41t80.c | 6 +++---
>> hw/virtio/virtio-balloon.c | 9 +
>> qapi/qmp-event.c | 12 +---
>> qemu-img.c | 8
>> target/m68k/m68k-semi.c | 22 ++
>> target/nios2/nios2-semi.c | 23 ++-
>> 7 files changed, 37 insertions(+), 51 deletions(-)
>>
...
>> index 19d3cd003833..025716b3ec37 100644
>> --- a/qapi/qmp-event.c
>> +++ b/qapi/qmp-event.c
>> @@ -20,15 +20,13 @@
>>
>> static void timestamp_put(QDict *qdict)
>> {
>> - int err;
>> QDict *ts;
>> - qemu_timeval tv;
>> + int64_t rt = g_get_real_time();
>>
>> - err = qemu_gettimeofday();
>> - /* Put -1 to indicate failure of getting host time */
>> - ts = qdict_from_jsonf_nofail("{ 'seconds': %lld, 'microseconds': %lld
}",
>> - err < 0 ? -1LL : (long long)tv.tv_sec,
>> - err < 0 ? -1LL : (long
long)tv.tv_usec);
>> + ts = qdict_from_jsonf_nofail("{ 'seconds': %" G_GINT64_FORMAT
>> + ", 'microseconds': %" G_GINT64_FORMAT
"}",
>> + rt / G_USEC_PER_SEC,
>> + rt % G_USEC_PER_SEC);
>
> NACK this, fixed in v3
Why keeping the %lld is better than moving to %G_GINT64_FORMAT?
It's not supported by json-parser.c parse_interpolation(). We could address
this in a different patch.
Yes, it would be a simple fix: it already supports "%i" and "%ld" but not "%li".
Thanks,
Laurent
Re: [PATCH v2 4/5] Replace qemu_gettimeofday() with g_get_real_time()
Le 05/03/2022 à 20:17, Marc-André Lureau a écrit :
On Sat, Mar 5, 2022 at 1:18 AM wrote:
From: Marc-André Lureau
GLib g_get_real_time() is an alternative to gettimeofday() which allows
to simplify our code.
For semihosting, a few bits are lost on POSIX host, but this shouldn't
be a big concern.
Signed-off-by: Marc-André Lureau
Reviewed-by: Laurent Vivier
---
blockdev.c | 8
hw/rtc/m41t80.c| 6 +++---
hw/virtio/virtio-balloon.c | 9 +
qapi/qmp-event.c | 12 +---
qemu-img.c | 8
target/m68k/m68k-semi.c| 22 ++
target/nios2/nios2-semi.c | 23 ++-
7 files changed, 37 insertions(+), 51 deletions(-)
...
index 19d3cd003833..025716b3ec37 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -20,15 +20,13 @@
static void timestamp_put(QDict *qdict)
{
-int err;
QDict *ts;
-qemu_timeval tv;
+int64_t rt = g_get_real_time();
-err = qemu_gettimeofday();
-/* Put -1 to indicate failure of getting host time */
-ts = qdict_from_jsonf_nofail("{ 'seconds': %lld, 'microseconds': %lld }",
- err < 0 ? -1LL : (long long)tv.tv_sec,
- err < 0 ? -1LL : (long long)tv.tv_usec);
+ts = qdict_from_jsonf_nofail("{ 'seconds': %" G_GINT64_FORMAT
+ ", 'microseconds': %" G_GINT64_FORMAT "}",
+ rt / G_USEC_PER_SEC,
+ rt % G_USEC_PER_SEC);
NACK this, fixed in v3
Why keeping the %lld is better than moving to %G_GINT64_FORMAT?
Thanks,
Laurent
Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()
Le 04/03/2022 à 17:08, Laurent Vivier a écrit :
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit :
From: Marc-André Lureau
GLib g_get_real_time() is an alternative to gettimeofday().
For semihosting, a few bits are lost on POSIX host, but this shouldn't
be a big concern.
Signed-off-by: Marc-André Lureau
---
blockdev.c | 8
hw/rtc/m41t80.c | 6 +++---
hw/virtio/virtio-balloon.c | 9 +
qapi/qmp-event.c | 12 +---
qemu-img.c | 8
qga/commands-posix.c | 11 +--
target/m68k/m68k-semi.c | 22 ++
target/nios2/nios2-semi.c | 24 +++-
8 files changed, 39 insertions(+), 61 deletions(-)
...
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68ea9..3311a4ca0167 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode,
Error **errp)
int64_t qmp_guest_get_time(Error **errp)
{
- int ret;
- qemu_timeval tq;
-
- ret = qemu_gettimeofday();
- if (ret < 0) {
- error_setg_errno(errp, errno, "Failed to get time");
- return -1;
- }
-
- return tq.tv_sec * 10LL + tq.tv_usec * 1000;
+ return g_get_real_time();
now you return microseconds, before it was nanoseconds.
qga/qapi-schema.json:
##
# @guest-get-time:
#
# Get the information about guest's System Time relative to
# the Epoch of 1970-01-01 in UTC.
#
# Returns: Time in nanoseconds.
#
# Since: 1.5
##
{ 'command': 'guest-get-time',
'returns': 'int' }
Except this problem:
Reviewed-by: Laurent Vivier
Perhaps you can also remove the windows version from qga/commands-win32.c and move the function to
qga/commands.c ?
Thanks,
Laurent
Re: [PATCH 4/4] oslib: drop qemu_gettimeofday()
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit : From: Marc-André Lureau No longer used after the previous patches. Signed-off-by: Marc-André Lureau --- include/sysemu/os-posix.h | 3 --- include/sysemu/os-win32.h | 6 -- util/oslib-win32.c| 20 3 files changed, 29 deletions(-) Reviewed-by: Laurent Vivier
Re: [PATCH 3/4] Replace qemu_gettimeofday() with g_get_real_time()
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit :
From: Marc-André Lureau
GLib g_get_real_time() is an alternative to gettimeofday().
For semihosting, a few bits are lost on POSIX host, but this shouldn't
be a big concern.
Signed-off-by: Marc-André Lureau
---
blockdev.c | 8
hw/rtc/m41t80.c| 6 +++---
hw/virtio/virtio-balloon.c | 9 +
qapi/qmp-event.c | 12 +---
qemu-img.c | 8
qga/commands-posix.c | 11 +--
target/m68k/m68k-semi.c| 22 ++
target/nios2/nios2-semi.c | 24 +++-
8 files changed, 39 insertions(+), 61 deletions(-)
...
diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 75dbaab68ea9..3311a4ca0167 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -138,16 +138,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode,
Error **errp)
int64_t qmp_guest_get_time(Error **errp)
{
- int ret;
- qemu_timeval tq;
-
- ret = qemu_gettimeofday();
- if (ret < 0) {
- error_setg_errno(errp, errno, "Failed to get time");
- return -1;
- }
-
- return tq.tv_sec * 10LL + tq.tv_usec * 1000;
+return g_get_real_time();
now you return microseconds, before it was nanoseconds.
qga/qapi-schema.json:
##
# @guest-get-time:
#
# Get the information about guest's System Time relative to
# the Epoch of 1970-01-01 in UTC.
#
# Returns: Time in nanoseconds.
#
# Since: 1.5
##
{ 'command': 'guest-get-time',
'returns': 'int' }
Except this problem:
Reviewed-by: Laurent Vivier
Re: [PATCH 2/4] qtest: replace gettimeofday with GTimer
Le 04/03/2022 à 16:54, Laurent Vivier a écrit :
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit :
From: Marc-André Lureau
glib provides a convenience helper to measure elapsed time. It isn't
subject to wall-clock time changes.
Note that this changes the initial OPENED time, which used to print the
current time.
Time is printed using FMT_timeval ("%ld.%06ld"), but g_timer_elapsed() returns
a gdouble.
Are you sure it works properly?
Sorry, I missed this part:
-#define FMT_timeval "%ld.%06ld"
+#define FMT_timeval "%.06f"
Reviewed-by: Laurent Vivier
Re: [PATCH 2/4] qtest: replace gettimeofday with GTimer
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit :
From: Marc-André Lureau
glib provides a convenience helper to measure elapsed time. It isn't
subject to wall-clock time changes.
Note that this changes the initial OPENED time, which used to print the
current time.
Time is printed using FMT_timeval ("%ld.%06ld"), but g_timer_elapsed() returns
a gdouble.
Are you sure it works properly?
Laurent
Signed-off-by: Marc-André Lureau
---
softmmu/qtest.c | 39 ++-
1 file changed, 10 insertions(+), 29 deletions(-)
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index 8b7cb6aa8e46..b2bbd17d 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -58,12 +58,12 @@ static FILE *qtest_log_fp;
static QTest *qtest;
static GString *inbuf;
static int irq_levels[MAX_IRQ];
-static qemu_timeval start_time;
+static GTimer *timer;
static bool qtest_opened;
static void (*qtest_server_send)(void*, const char*);
static void *qtest_server_send_opaque;
-#define FMT_timeval "%ld.%06ld"
+#define FMT_timeval "%.06f"
/**
* DOC: QTest Protocol
@@ -264,28 +264,13 @@ static int hex2nib(char ch)
}
}
-static void qtest_get_time(qemu_timeval *tv)
-{
-qemu_gettimeofday(tv);
-tv->tv_sec -= start_time.tv_sec;
-tv->tv_usec -= start_time.tv_usec;
-if (tv->tv_usec < 0) {
-tv->tv_usec += 100;
-tv->tv_sec -= 1;
-}
-}
-
static void qtest_send_prefix(CharBackend *chr)
{
-qemu_timeval tv;
-
if (!qtest_log_fp || !qtest_opened) {
return;
}
-qtest_get_time();
-fprintf(qtest_log_fp, "[S +" FMT_timeval "] ",
-(long) tv.tv_sec, (long) tv.tv_usec);
+fprintf(qtest_log_fp, "[S +" FMT_timeval "] ", g_timer_elapsed(timer,
NULL));
}
static void GCC_FMT_ATTR(1, 2) qtest_log_send(const char *fmt, ...)
@@ -386,12 +371,9 @@ static void qtest_process_command(CharBackend *chr, gchar
**words)
command = words[0];
if (qtest_log_fp) {
-qemu_timeval tv;
int i;
-qtest_get_time();
-fprintf(qtest_log_fp, "[R +" FMT_timeval "]",
-(long) tv.tv_sec, (long) tv.tv_usec);
+fprintf(qtest_log_fp, "[R +" FMT_timeval "]", g_timer_elapsed(timer,
NULL));
for (i = 0; words[i]; i++) {
fprintf(qtest_log_fp, " %s", words[i]);
}
@@ -846,21 +828,20 @@ static void qtest_event(void *opaque, QEMUChrEvent event)
for (i = 0; i < ARRAY_SIZE(irq_levels); i++) {
irq_levels[i] = 0;
}
-qemu_gettimeofday(_time);
+
+g_clear_pointer(, g_timer_destroy);
+timer = g_timer_new();
qtest_opened = true;
if (qtest_log_fp) {
-fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
-(long) start_time.tv_sec, (long) start_time.tv_usec);
+fprintf(qtest_log_fp, "[I " FMT_timeval "] OPENED\n",
g_timer_elapsed(timer, NULL));
}
break;
case CHR_EVENT_CLOSED:
qtest_opened = false;
if (qtest_log_fp) {
-qemu_timeval tv;
-qtest_get_time();
-fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n",
-(long) tv.tv_sec, (long) tv.tv_usec);
+fprintf(qtest_log_fp, "[I +" FMT_timeval "] CLOSED\n",
g_timer_elapsed(timer, NULL));
}
+g_clear_pointer(, g_timer_destroy);
break;
default:
break;
Re: [PATCH 1/4] m68k/nios2-semi: fix gettimeofday() result check
Le 04/03/2022 à 16:27, marcandre.lur...@redhat.com a écrit :
From: Marc-André Lureau
gettimeofday() returns 0 for success.
Signed-off-by: Marc-André Lureau
---
target/m68k/m68k-semi.c | 2 +-
target/nios2/nios2-semi.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/m68k/m68k-semi.c b/target/m68k/m68k-semi.c
index 44ec7e4612c6..c5c164e096c8 100644
--- a/target/m68k/m68k-semi.c
+++ b/target/m68k/m68k-semi.c
@@ -381,7 +381,7 @@ void do_m68k_semihosting(CPUM68KState *env, int nr)
qemu_timeval tv;
struct gdb_timeval *p;
result = qemu_gettimeofday();
-if (result != 0) {
+if (result == 0) {
if (!(p = lock_user(VERIFY_WRITE,
arg0, sizeof(struct gdb_timeval), 0))) {
/* FIXME - check error code? */
diff --git a/target/nios2/nios2-semi.c b/target/nios2/nios2-semi.c
index fe5598bae4d7..5a7ad0c7108d 100644
--- a/target/nios2/nios2-semi.c
+++ b/target/nios2/nios2-semi.c
@@ -403,7 +403,7 @@ void do_nios2_semihosting(CPUNios2State *env)
qemu_timeval tv;
struct gdb_timeval *p;
result = qemu_gettimeofday();
-if (result != 0) {
+if (result == 0) {
p = lock_user(VERIFY_WRITE, arg0, sizeof(struct gdb_timeval),
0);
if (!p) {
It seems to have never worked correctly...
Reviewed-by: Laurent Vivier
Re: [PATCH v2] hw/scsi/megasas: Simplify using the ldst API
Le 18/12/2021 à 12:19, Philippe Mathieu-Daudé a écrit :
This code is easier to review using the load/store API.
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
v2: Fixed offset in megasas_setup_inquiry (rth)
---
hw/scsi/megasas.c | 17 +++--
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
index 619b66ef0f3..9a4e9ba87e6 100644
--- a/hw/scsi/megasas.c
+++ b/hw/scsi/megasas.c
@@ -375,8 +375,7 @@ static int megasas_setup_inquiry(uint8_t *cdb, int pg, int
len)
cdb[1] = 0x1;
cdb[2] = pg;
}
-cdb[3] = (len >> 8) & 0xff;
-cdb[4] = (len & 0xff);
+stw_be_p([3], len);
return len;
}
@@ -392,18 +391,8 @@ static void megasas_encode_lba(uint8_t *cdb, uint64_t lba,
} else {
cdb[0] = READ_16;
}
-cdb[2] = (lba >> 56) & 0xff;
-cdb[3] = (lba >> 48) & 0xff;
-cdb[4] = (lba >> 40) & 0xff;
-cdb[5] = (lba >> 32) & 0xff;
-cdb[6] = (lba >> 24) & 0xff;
-cdb[7] = (lba >> 16) & 0xff;
-cdb[8] = (lba >> 8) & 0xff;
-cdb[9] = (lba) & 0xff;
-cdb[10] = (len >> 24) & 0xff;
-cdb[11] = (len >> 16) & 0xff;
-cdb[12] = (len >> 8) & 0xff;
-cdb[13] = (len) & 0xff;
+stq_be_p([2], lba);
+stl_be_p([2 + 8], len);
}
/*
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 02/28] glib-compat: Introduce g_memdup2() wrapper
Alex,
I've added this patch to my trivial patches branch, do you want I drop it?
Thanks,
Laurent
On 17/12/2021 12:10, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
On 12/16/21 15:11, Alex Bennée wrote:
Philippe Mathieu-Daudé writes:
When experimenting raising GLIB_VERSION_MIN_REQUIRED to 2.68
(Fedora 34 provides GLib 2.68.1) we get:
hw/virtio/virtio-crypto.c:245:24: error: 'g_memdup' is deprecated: Use
'g_memdup2' instead [-Werror,-Wdeprecated-declarations]
...
g_memdup() has been updated by g_memdup2() to fix eventual security
issues (size argument is 32-bit and could be truncated / wrapping).
GLib recommends to copy their static inline version of g_memdup2():
https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
Our glib-compat.h provides a comment explaining how to deal with
these deprecated declarations (see commit e71e8cc0355
"glib: enforce the minimum required version and warn about old APIs").
+#define g_memdup2(m, s) g_memdup2_qemu(m, s)
+
As per our style wouldn't it make sense to just call it qemu_memdup(m,
s)?
I followed the documentation in include/glib-compat.h:
/*
* Note that because of the GLIB_VERSION_MAX_ALLOWED constant above,
allowing
* use of functions from newer GLib via this compat header needs a little
* trickery to prevent warnings being emitted.
*
* Consider a function from newer glib-X.Y that we want to use
*
*int g_foo(const char *wibble)
*
* We must define a static inline function with the same signature that does
* what we need, but with a "_qemu" suffix e.g.
*
* static inline void g_foo_qemu(const char *wibble)
* {
* #if GLIB_CHECK_VERSION(X, Y, 0)
*g_foo(wibble)
* #else
*g_something_equivalent_in_older_glib(wibble);
* #endif
* }
*
* The #pragma at the top of this file turns off -Wdeprecated-declarations,
* ensuring this wrapper function impl doesn't trigger the compiler warning
* about using too new glib APIs. Finally we can do
*
* #define g_foo(a) g_foo_qemu(a)
*
* So now the code elsewhere in QEMU, which *does* have the
* -Wdeprecated-declarations warning active, can call g_foo(...) as normal,
* without generating warnings.
*/
which is how g_unix_get_passwd_entry_qemu() is implemented.
Yet later we have qemu_g_test_slow following the style guide. Also I'm
confused by the usage of g_unix_get_passwd_entry_qemu because the only
place I see it in qga/commands-posix-ssh.c right before it does:
#define g_unix_get_passwd_entry_qemu(username, err) \
test_get_passwd_entry(username, err)
although I think that only hold when the file is built with
QGA_BUILD_UNIT_TEST.
Should we reword the documentation first?
The original wording in glib-compat.h was added by Daniel in 2018 but
the commit that added the password function comments:
Since the fallback version is still unsafe, I would rather keep the
_qemu postfix, to make sure it's not being misused by mistake. When/if
necessary, we can implement a safer fallback and drop the _qemu suffix.
So if we are going to make a distinction between a qemu prefix and
suffix we should agree that and add it to the style document.
Re: [PATCH v3 00/28] glib: Replace g_memdup() by g_memdup2()
Le 15/12/2021 à 17:54, Philippe Mathieu-Daudé a écrit : Hi Laurent, On 9/3/21 19:44, Philippe Mathieu-Daudé wrote: This series provides the safely equivalent g_memdup2() wrapper, and replace all g_memdup() calls by it. Philippe Mathieu-Daudé (28): hw/hyperv/vmbus: Remove unused vmbus_load/save_req() glib-compat: Introduce g_memdup2() wrapper qapi: Replace g_memdup() by g_memdup2() accel/tcg: Replace g_memdup() by g_memdup2() block/qcow2-bitmap: Replace g_memdup() by g_memdup2() softmmu: Replace g_memdup() by g_memdup2() hw/9pfs: Replace g_memdup() by g_memdup2() hw/acpi: Avoid truncating acpi_data_len() to 32-bit hw/acpi: Replace g_memdup() by g_memdup2() hw/core/machine: Replace g_memdup() by g_memdup2() hw/hppa/machine: Replace g_memdup() by g_memdup2() hw/i386/multiboot: Replace g_memdup() by g_memdup2() hw/net/eepro100: Replace g_memdup() by g_memdup2() hw/nvram/fw_cfg: Replace g_memdup() by g_memdup2() hw/scsi/mptsas: Replace g_memdup() by g_memdup2() hw/ppc/spapr_pci: Replace g_memdup() by g_memdup2() hw/rdma: Replace g_memdup() by g_memdup2() hw/vfio/pci: Replace g_memdup() by g_memdup2() hw/virtio: Replace g_memdup() by g_memdup2() net/colo: Replace g_memdup() by g_memdup2() ui/clipboard: Replace g_memdup() by g_memdup2() linux-user: Replace g_memdup() by g_memdup2() tests/unit: Replace g_memdup() by g_memdup2() tests/qtest: Replace g_memdup() by g_memdup2() target/arm: Replace g_memdup() by g_memdup2() target/ppc: Replace g_memdup() by g_memdup2() contrib: Replace g_memdup() by g_memdup2() checkpatch: Do not allow deprecated g_memdup( Is it possible to take the reviewed patches 2, 24 and 28 via qemu-trivial, so the rest can be merged slowly by each submaintainer? Done for these 3 patches. Thanks, Laurent
Re: [PATCH v3 28/28] checkpatch: Do not allow deprecated g_memdup()
Le 03/09/2021 à 19:45, Philippe Mathieu-Daudé a écrit :
g_memdup() is insecure and as been deprecated in GLib 2.68.
QEMU provides the safely equivalent g_memdup2() wrapper.
Do not allow more g_memdup() calls in the repository, provide
a hint to use g_memdup2().
Signed-off-by: Philippe Mathieu-Daudé
---
scripts/checkpatch.pl | 5 +
1 file changed, 5 insertions(+)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index cb8eff233e0..5caa739db48 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2850,6 +2850,11 @@ sub process {
WARN("consider using g_path_get_$1() in preference to
g_strdup($1())\n" . $herecurr);
}
+# enforce g_memdup2() over g_memdup()
+ if ($line =~ /\bg_memdup\s*\(/) {
+ ERROR("use g_memdup2() instead of unsafe g_memdup()\n"
. $herecurr);
+ }
+
# recommend qemu_strto* over strto* for numeric conversions
if ($line =~ /\b(strto[^kd].*?)\s*\(/) {
ERROR("consider using qemu_$1 in preference to $1\n" .
$herecurr);
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 24/28] tests/qtest: Replace g_memdup() by g_memdup2()
Le 03/09/2021 à 19:45, Philippe Mathieu-Daudé a écrit :
Per
https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
The old API took the size of the memory to duplicate as a guint,
whereas most memory functions take memory sizes as a gsize. This
made it easy to accidentally pass a gsize to g_memdup(). For large
values, that would lead to a silent truncation of the size from 64
to 32 bits, and result in a heap area being returned which is
significantly smaller than what the caller expects. This can likely
be exploited in various modules to cause a heap buffer overflow.
Replace g_memdup() by the safer g_memdup2() wrapper.
Signed-off-by: Philippe Mathieu-Daudé
---
tests/qtest/libqos/ahci.c | 6 +++---
tests/qtest/libqos/qgraph.c | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/tests/qtest/libqos/ahci.c b/tests/qtest/libqos/ahci.c
index fba3e7a954e..eaa2096512e 100644
--- a/tests/qtest/libqos/ahci.c
+++ b/tests/qtest/libqos/ahci.c
@@ -639,8 +639,8 @@ void ahci_exec(AHCIQState *ahci, uint8_t port,
AHCIOpts *opts;
uint64_t buffer_in;
-opts = g_memdup((opts_in == NULL ? _opts : opts_in),
-sizeof(AHCIOpts));
+opts = g_memdup2((opts_in == NULL ? _opts : opts_in),
+ sizeof(AHCIOpts));
buffer_in = opts->buffer;
@@ -860,7 +860,7 @@ AHCICommand *ahci_command_create(uint8_t command_name)
g_assert(!props->ncq || props->lba48);
/* Defaults and book-keeping */
-cmd->props = g_memdup(props, sizeof(AHCICommandProp));
+cmd->props = g_memdup2(props, sizeof(AHCICommandProp));
cmd->name = command_name;
cmd->xbytes = props->size;
cmd->prd_size = 4096;
diff --git a/tests/qtest/libqos/qgraph.c b/tests/qtest/libqos/qgraph.c
index d1dc4919305..109ff04e1e8 100644
--- a/tests/qtest/libqos/qgraph.c
+++ b/tests/qtest/libqos/qgraph.c
@@ -93,7 +93,7 @@ static void add_edge(const char *source, const char *dest,
edge->type = type;
edge->dest = g_strdup(dest);
edge->edge_name = g_strdup(opts->edge_name ?: dest);
-edge->arg = g_memdup(opts->arg, opts->size_arg);
+edge->arg = g_memdup2(opts->arg, opts->size_arg);
edge->before_cmd_line =
opts->before_cmd_line ? g_strconcat(" ", opts->before_cmd_line, NULL)
: NULL;
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 02/28] glib-compat: Introduce g_memdup2() wrapper
Le 03/09/2021 à 19:44, Philippe Mathieu-Daudé a écrit :
When experimenting raising GLIB_VERSION_MIN_REQUIRED to 2.68
(Fedora 34 provides GLib 2.68.1) we get:
hw/virtio/virtio-crypto.c:245:24: error: 'g_memdup' is deprecated: Use
'g_memdup2' instead [-Werror,-Wdeprecated-declarations]
...
g_memdup() has been updated by g_memdup2() to fix eventual security
issues (size argument is 32-bit and could be truncated / wrapping).
GLib recommends to copy their static inline version of g_memdup2():
https://discourse.gnome.org/t/port-your-module-from-g-memdup-to-g-memdup2-now/5538
Our glib-compat.h provides a comment explaining how to deal with
these deprecated declarations (see commit e71e8cc0355
"glib: enforce the minimum required version and warn about old APIs").
Following this comment suggestion, implement the g_memdup2_qemu()
wrapper to g_memdup2(), and use the safer equivalent inlined when
we are using pre-2.68 GLib.
Reported-by: Eric Blake
Signed-off-by: Philippe Mathieu-Daudé
---
include/glib-compat.h | 37 +
1 file changed, 37 insertions(+)
diff --git a/include/glib-compat.h b/include/glib-compat.h
index 9e95c888f54..8d01a8c01fb 100644
--- a/include/glib-compat.h
+++ b/include/glib-compat.h
@@ -68,6 +68,43 @@
* without generating warnings.
*/
+/*
+ * g_memdup2_qemu:
+ * @mem: (nullable): the memory to copy.
+ * @byte_size: the number of bytes to copy.
+ *
+ * Allocates @byte_size bytes of memory, and copies @byte_size bytes into it
+ * from @mem. If @mem is %NULL it returns %NULL.
+ *
+ * This replaces g_memdup(), which was prone to integer overflows when
+ * converting the argument from a #gsize to a #guint.
+ *
+ * This static inline version is a backport of the new public API from
+ * GLib 2.68, kept internal to GLib for backport to older stable releases.
+ * See https://gitlab.gnome.org/GNOME/glib/-/issues/2319.
+ *
+ * Returns: (nullable): a pointer to the newly-allocated copy of the memory,
+ * or %NULL if @mem is %NULL.
+ */
+static inline gpointer g_memdup2_qemu(gconstpointer mem, gsize byte_size)
+{
+#if GLIB_CHECK_VERSION(2, 68, 0)
+return g_memdup2(mem, byte_size);
+#else
+gpointer new_mem;
+
+if (mem && byte_size != 0) {
+new_mem = g_malloc(byte_size);
+memcpy(new_mem, mem, byte_size);
+} else {
+new_mem = NULL;
+}
+
+return new_mem;
+#endif
+}
+#define g_memdup2(m, s) g_memdup2_qemu(m, s)
+
#if defined(G_OS_UNIX)
/*
* Note: The fallback implementation is not MT-safe, and it returns a copy of
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH 1/3] qdict: make available dump_qobject(), dump_qdict(), dump_qlist()
On 10/11/2021 17:17, Markus Armbruster wrote: Laurent Vivier writes: move them from block/qapi.c to qobject/qdict.c, qobject/qlist.c, qobject/qobject.c This is useful to debug qobjects Signed-off-by: Laurent Vivier I think qobject_to_json_pretty() is better suited to debugging, because it preserves differences like the one between the string "1" and the number 1. Yes, you're right. I didn't think about this solution. So we can drop this patch. Thanks, Laurent
[PATCH 3/3] tests/qtest: add some tests for virtio-net failover
Add test cases to test several error cases that must be
generated by invalid failover configuration.
Add a combination of coldplug and hotplug test cases to be
sure the primary is correctly managed according the
presence or not of the STANDBY feature.
Signed-off-by: Laurent Vivier
---
tests/qtest/meson.build | 3 +
tests/qtest/virtio-net-failover.c | 567 ++
2 files changed, 570 insertions(+)
create mode 100644 tests/qtest/virtio-net-failover.c
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index c9d8458062ff..6d66bf522156 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -22,6 +22,9 @@ qtests_generic = \
(config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ?
['fuzz-virtio-scsi-test'] : []) + \
(config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) + \
(config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : [])
+ \
+ (config_all_devices.has_key('CONFIG_VIRTIO_NET') and \
+ config_all_devices.has_key('CONFIG_Q35') and \
+ config_all_devices.has_key('CONFIG_VIRTIO_PCI') ? ['virtio-net-failover'] :
[]) + \
[
'cdrom-test',
'device-introspect-test',
diff --git a/tests/qtest/virtio-net-failover.c
b/tests/qtest/virtio-net-failover.c
new file mode 100644
index ..c7b03b887179
--- /dev/null
+++ b/tests/qtest/virtio-net-failover.c
@@ -0,0 +1,567 @@
+#include "qemu/osdep.h"
+#include "libqos/libqtest.h"
+#include "libqos/pci.h"
+#include "libqos/pci-pc.h"
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qjson.h"
+#include "libqos/malloc-pc.h"
+#include "libqos/virtio-pci.h"
+#include "hw/pci/pci.h"
+
+#define BASE_MACHINE "-M q35 -nodefaults " \
+"-device pcie-root-port,id=root0,addr=0x1,bus=pcie.0,chassis=1 " \
+"-device pcie-root-port,id=root1,addr=0x2,bus=pcie.0,chassis=2 "
+
+#define MAC_PRIMARY "52:54:00:11:11:11"
+#define MAC_STANDBY "52:54:00:22:22:22"
+
+static QGuestAllocator guest_malloc;
+static QPCIBus *pcibus;
+
+static QTestState *machine_start(const char *args)
+{
+QTestState *qts;
+QPCIDevice *dev;
+
+qts = qtest_init(args);
+
+pc_alloc_init(_malloc, qts, 0);
+pcibus = qpci_new_pc(qts, _malloc);
+g_assert(qpci_secondary_buses_init(pcibus) == 2);
+
+dev = qpci_device_find(pcibus, QPCI_DEVFN(1, 0)); /* root0 */
+g_assert_nonnull(dev);
+
+qpci_device_enable(dev);
+qpci_iomap(dev, 4, NULL);
+
+g_free(dev);
+
+dev = qpci_device_find(pcibus, QPCI_DEVFN(2, 0)); /* root1 */
+g_assert_nonnull(dev);
+
+qpci_device_enable(dev);
+qpci_iomap(dev, 4, NULL);
+
+g_free(dev);
+
+return qts;
+}
+
+static void machine_stop(QTestState *qts)
+{
+qpci_free_pc(pcibus);
+alloc_destroy(_malloc);
+qtest_quit(qts);
+}
+
+static void test_error_id(void)
+{
+QTestState *qts;
+QDict *resp;
+QDict *err;
+
+qts = machine_start(BASE_MACHINE
+"-device
virtio-net,bus=root0,id=standby0,failover=on");
+
+resp = qtest_qmp(qts, "{'execute': 'device_add',"
+ "'arguments': {"
+ "'driver': 'virtio-net',"
+ "'bus': 'root1',"
+ "'failover_pair_id': 'standby0'"
+ "} }");
+g_assert(qdict_haskey(resp, "error"));
+
+err = qdict_get_qdict(resp, "error");
+g_assert(qdict_haskey(err, "desc"));
+
+g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+"Device with failover_pair_id needs to have id");
+
+qobject_unref(resp);
+
+machine_stop(qts);
+}
+
+static void test_error_pcie(void)
+{
+QTestState *qts;
+QDict *resp;
+QDict *err;
+
+qts = machine_start(BASE_MACHINE
+"-device
virtio-net,bus=root0,id=standby0,failover=on");
+
+resp = qtest_qmp(qts, "{'execute': 'device_add',"
+ "'arguments': {"
+ "'driver': 'virtio-net',"
+ "'id': 'primary0',"
+ "'bus': 'pcie.0',"
+ "'failover_pair_id': 'standby0'"
+ "} }");
+g_assert(qdict_haskey(resp, "error"));
+
+err = qdict_get_qdict(resp, "error");
+g_assert(qdict_haskey(err, "desc"));
+
+g_assert_cmpstr(qdict_get_str(err, "desc"), ==,
+"Bus 'pcie.0' does not support hotplugging");
+
+qobject_unref(resp);
+
+machine_stop(qts);
+}
+
+static QDict *find_device(QDict *bus, const char *name)
+{
+const QObject *obj;
+
[PATCH 2/3] qtest/libqos: add a function to initialize secondary PCI buses
Scan the PCI devices to find bridge and set PCI_SECONDARY_BUS and
PCI_SUBORDINATE_BUS (algorithm from seabios)
Signed-off-by: Laurent Vivier
---
include/hw/pci/pci_bridge.h | 8 +++
tests/qtest/libqos/pci.c| 118
tests/qtest/libqos/pci.h| 1 +
3 files changed, 127 insertions(+)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index a94d350034bf..30691a6e5728 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -138,6 +138,7 @@ typedef struct PCIBridgeQemuCap {
uint64_t mem_pref_64; /* Prefetchable memory to reserve (64-bit MMIO) */
} PCIBridgeQemuCap;
+#define REDHAT_PCI_CAP_TYPE_OFFSET 3
#define REDHAT_PCI_CAP_RESOURCE_RESERVE 1
/*
@@ -152,6 +153,13 @@ typedef struct PCIResReserve {
uint64_t mem_pref_64;
} PCIResReserve;
+#define REDHAT_PCI_CAP_RES_RESERVE_BUS_RES 4
+#define REDHAT_PCI_CAP_RES_RESERVE_IO 8
+#define REDHAT_PCI_CAP_RES_RESERVE_MEM 16
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_32 20
+#define REDHAT_PCI_CAP_RES_RESERVE_PREF_MEM_64 24
+#define REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE32
+
int pci_bridge_qemu_reserve_cap_init(PCIDevice *dev, int cap_offset,
PCIResReserve res_reserve, Error **errp);
diff --git a/tests/qtest/libqos/pci.c b/tests/qtest/libqos/pci.c
index e1e96189c821..3f0b18f4750b 100644
--- a/tests/qtest/libqos/pci.c
+++ b/tests/qtest/libqos/pci.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
#include "pci.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_regs.h"
#include "qemu/host-utils.h"
#include "qgraph.h"
@@ -99,6 +101,122 @@ void qpci_device_init(QPCIDevice *dev, QPCIBus *bus,
QPCIAddress *addr)
g_assert(!addr->device_id || device_id == addr->device_id);
}
+static uint8_t qpci_find_resource_reserve_capability(QPCIDevice *dev)
+{
+uint16_t device_id;
+uint8_t cap = 0;
+
+if (qpci_config_readw(dev, PCI_VENDOR_ID) != PCI_VENDOR_ID_REDHAT) {
+return 0;
+}
+
+device_id = qpci_config_readw(dev, PCI_DEVICE_ID);
+
+if (device_id != PCI_DEVICE_ID_REDHAT_PCIE_RP &&
+device_id != PCI_DEVICE_ID_REDHAT_BRIDGE) {
+return 0;
+}
+
+do {
+cap = qpci_find_capability(dev, PCI_CAP_ID_VNDR, cap);
+} while (cap &&
+ qpci_config_readb(dev, cap + REDHAT_PCI_CAP_TYPE_OFFSET) !=
+ REDHAT_PCI_CAP_RESOURCE_RESERVE);
+if (cap) {
+uint8_t cap_len = qpci_config_readb(dev, cap + PCI_CAP_FLAGS);
+if (cap_len < REDHAT_PCI_CAP_RES_RESERVE_CAP_SIZE) {
+return 0;
+}
+}
+return cap;
+}
+
+static void qpci_secondary_buses_rec(QPCIBus *qbus, int bus, int *pci_bus)
+{
+QPCIDevice *dev;
+uint16_t class;
+uint8_t pribus, secbus, subbus;
+int i;
+
+for (i = 0; i < 32; i++) {
+dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+if (dev == NULL) {
+continue;
+}
+class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+if (class == PCI_CLASS_BRIDGE_PCI) {
+qpci_config_writeb(dev, PCI_SECONDARY_BUS, 255);
+qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 0);
+}
+g_free(dev);
+}
+
+for (i = 0; i < 32; i++) {
+dev = qpci_device_find(qbus, QPCI_DEVFN(bus + i, 0));
+if (dev == NULL) {
+continue;
+}
+class = qpci_config_readw(dev, PCI_CLASS_DEVICE);
+if (class != PCI_CLASS_BRIDGE_PCI) {
+continue;
+}
+
+pribus = qpci_config_readb(dev, PCI_PRIMARY_BUS);
+if (pribus != bus) {
+qpci_config_writeb(dev, PCI_PRIMARY_BUS, bus);
+}
+
+secbus = qpci_config_readb(dev, PCI_SECONDARY_BUS);
+(*pci_bus)++;
+if (*pci_bus != secbus) {
+secbus = *pci_bus;
+qpci_config_writeb(dev, PCI_SECONDARY_BUS, secbus);
+}
+
+subbus = qpci_config_readb(dev, PCI_SUBORDINATE_BUS);
+qpci_config_writeb(dev, PCI_SUBORDINATE_BUS, 255);
+
+qpci_secondary_buses_rec(qbus, secbus << 5, pci_bus);
+
+if (subbus != *pci_bus) {
+uint8_t res_bus = *pci_bus;
+uint8_t cap = qpci_find_resource_reserve_capability(dev);
+
+if (cap) {
+uint32_t tmp_res_bus;
+
+tmp_res_bus = qpci_config_readl(dev, cap +
+
REDHAT_PCI_CAP_RES_RESERVE_BUS_RES);
+if (tmp_res_bus != (uint32_t)-1) {
+res_bus = tmp_res_bus & 0xFF;
+if ((uint8_t)(res_bus + secbus) < secbus ||
+(uint8_t)(res_bus + secbus) < res_bus) {
+res_bus = 0;
+
[PATCH 1/3] qdict: make available dump_qobject(), dump_qdict(), dump_qlist()
move them from block/qapi.c to qobject/qdict.c, qobject/qlist.c,
qobject/qobject.c
This is useful to debug qobjects
Signed-off-by: Laurent Vivier
---
block/qapi.c | 82 +-
include/qapi/qmp/qdict.h | 2 +
include/qapi/qmp/qlist.h | 1 +
include/qapi/qmp/qobject.h | 1 +
qobject/qdict.c| 25
qobject/qlist.c| 17
qobject/qobject.c | 35
7 files changed, 82 insertions(+), 81 deletions(-)
diff --git a/block/qapi.c b/block/qapi.c
index cf557e3aea7c..26bbc45a67f5 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -697,86 +697,6 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
g_free(sizing);
}
-static void dump_qdict(int indentation, QDict *dict);
-static void dump_qlist(int indentation, QList *list);
-
-static void dump_qobject(int comp_indent, QObject *obj)
-{
-switch (qobject_type(obj)) {
-case QTYPE_QNUM: {
-QNum *value = qobject_to(QNum, obj);
-char *tmp = qnum_to_string(value);
-qemu_printf("%s", tmp);
-g_free(tmp);
-break;
-}
-case QTYPE_QSTRING: {
-QString *value = qobject_to(QString, obj);
-qemu_printf("%s", qstring_get_str(value));
-break;
-}
-case QTYPE_QDICT: {
-QDict *value = qobject_to(QDict, obj);
-dump_qdict(comp_indent, value);
-break;
-}
-case QTYPE_QLIST: {
-QList *value = qobject_to(QList, obj);
-dump_qlist(comp_indent, value);
-break;
-}
-case QTYPE_QBOOL: {
-QBool *value = qobject_to(QBool, obj);
-qemu_printf("%s", qbool_get_bool(value) ? "true" : "false");
-break;
-}
-default:
-abort();
-}
-}
-
-static void dump_qlist(int indentation, QList *list)
-{
-const QListEntry *entry;
-int i = 0;
-
-for (entry = qlist_first(list); entry; entry = qlist_next(entry), i++) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-qemu_printf("%*s[%i]:%c", indentation * 4, "", i,
-composite ? '\n' : ' ');
-dump_qobject(indentation + 1, entry->value);
-if (!composite) {
-qemu_printf("\n");
-}
-}
-}
-
-static void dump_qdict(int indentation, QDict *dict)
-{
-const QDictEntry *entry;
-
-for (entry = qdict_first(dict); entry; entry = qdict_next(dict, entry)) {
-QType type = qobject_type(entry->value);
-bool composite = (type == QTYPE_QDICT || type == QTYPE_QLIST);
-char *key = g_malloc(strlen(entry->key) + 1);
-int i;
-
-/* replace dashes with spaces in key (variable) names */
-for (i = 0; entry->key[i]; i++) {
-key[i] = entry->key[i] == '-' ? ' ' : entry->key[i];
-}
-key[i] = 0;
-qemu_printf("%*s%s:%c", indentation * 4, "", key,
-composite ? '\n' : ' ');
-dump_qobject(indentation + 1, entry->value);
-if (!composite) {
-qemu_printf("\n");
-}
-g_free(key);
-}
-}
-
void bdrv_image_info_specific_dump(ImageInfoSpecific *info_spec)
{
QObject *obj, *data;
@@ -785,7 +705,7 @@ void bdrv_image_info_specific_dump(ImageInfoSpecific
*info_spec)
visit_type_ImageInfoSpecific(v, NULL, _spec, _abort);
visit_complete(v, );
data = qdict_get(qobject_to(QDict, obj), "data");
-dump_qobject(1, data);
+dump_qobject(1, data, qemu_printf);
qobject_unref(obj);
visit_free(v);
}
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index d5b5430e21a9..e529a9a9a29c 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -67,4 +67,6 @@ QDict *qdict_clone_shallow(const QDict *src);
QObject *qdict_crumple(const QDict *src, Error **errp);
void qdict_flatten(QDict *qdict);
+void dump_qdict(int indentation, QDict *dict, int (*qemu_printf)(const char
*fmt, ...));
+
#endif /* QDICT_H */
diff --git a/include/qapi/qmp/qlist.h b/include/qapi/qmp/qlist.h
index 06e98ad5f498..4af55d82d3e4 100644
--- a/include/qapi/qmp/qlist.h
+++ b/include/qapi/qmp/qlist.h
@@ -62,4 +62,5 @@ static inline const QListEntry *qlist_next(const QListEntry
*entry)
return QTAILQ_NEXT(entry, next);
}
+void dump_qlist(int indentation, QList *list, int (*qemu_printf)(const char
*fmt, ...));
#endif /* QLIST_H */
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 9003b71fd32d..bf893b146217 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -135,4 +135,5 @@ static inline QObject *qobject_check_type(const QObject
*obj, QType type)
}
}
+void dump_qobject(i
[PATCH 0/3] tests/qtest: add some tests for virtio-net failover
This series adds a qtest entry to test virtio-net failover feature. We check following error cases: - check missing id on device with failover_pair_id triggers an error - check a primary device plugged on a bus that doesn't support hotplug triggers an error We check the status of the machine before and after hotplugging cards and feature negotiation: - check we don't see the primary device at boot if failover is on - check we see the primary device at boot if failover is off - check we don't see the primary device if failover is on but failover_pair_id is not the one with on (I think this should be changed) - check the primary device is plugged after the feature negotiation - check the result if the primary device is plugged before standby device and vice-versa - check the if the primary device is coldplugged and the standy device hotplugged and vice-versa - check the migration triggers the unplug -> this one needs to be improved as we can't actualy unplug the card as the qtest framework doesn't allow to really do the OS level unplug. So we receive the UNPLUG_PRIMARY event but nothing more. There are two preliminary patches in the series: - PATCH 1 makes available functions that helped me to debug the qmp command result. I think it's a good point to have them available widely - PATCH 2 introduces a function to enable PCI bridge. Failover needs to be plugged on a pcie-root-port and while the root port is not configured the cards behind it are not available Laurent Vivier (3): qdict: make available dump_qobject(), dump_qdict(), dump_qlist() qtest/libqos: add a function to initialize secondary PCI buses tests/qtest: add some tests for virtio-net failover block/qapi.c | 82 + include/hw/pci/pci_bridge.h | 8 + include/qapi/qmp/qdict.h | 2 + include/qapi/qmp/qlist.h | 1 + include/qapi/qmp/qobject.h| 1 + qobject/qdict.c | 25 ++ qobject/qlist.c | 17 + qobject/qobject.c | 35 ++ tests/qtest/libqos/pci.c | 118 +++ tests/qtest/libqos/pci.h | 1 + tests/qtest/meson.build | 3 + tests/qtest/virtio-net-failover.c | 567 ++ 12 files changed, 779 insertions(+), 81 deletions(-) create mode 100644 tests/qtest/virtio-net-failover.c -- 2.31.1
Re: [PATCH v8 5/8] qmp: decode feature & status bits in virtio-status
On 27/10/2021 13:59, David Hildenbrand wrote: On 27.10.21 13:41, Jonah Palmer wrote: From: Laurent Vivier Display feature names instead of bitmaps for host, guest, and backend for VirtIODevice. Display status names instead of bitmaps for VirtIODevice. Display feature names instead of bitmaps for backend, protocol, acked, and features (hdev->features) for vhost devices. Decode features according to device type. Decode status according to configuration status bitmap (config_status_map). Decode vhost user protocol features according to vhost user protocol bitmap (vhost_user_protocol_map). Transport features are on the first line. Undecoded bits (if any) are stored in a separate field. Vhost device field wont show if there's no vhost active for a given VirtIODevice. Signed-off-by: Jonah Palmer --- hw/block/virtio-blk.c | 28 ++ hw/char/virtio-serial-bus.c| 11 + hw/display/virtio-gpu-base.c | 18 +- hw/input/virtio-input.c| 11 +- hw/net/virtio-net.c| 47 hw/scsi/virtio-scsi.c | 17 ++ hw/virtio/vhost-user-fs.c | 10 + hw/virtio/vhost-vsock-common.c | 10 + hw/virtio/virtio-balloon.c | 14 + hw/virtio/virtio-crypto.c | 10 + hw/virtio/virtio-iommu.c | 14 + hw/virtio/virtio.c | 273 ++- include/hw/virtio/vhost.h | 3 + include/hw/virtio/virtio.h | 17 ++ qapi/virtio.json | 577 ++--- Any particular reason we're not handling virtio-mem? (there is only a single feature bit so far, a second one to be introduced soon) I think this is because the v1 of the series has been written in March 2020 and it has not been update when virtio-mem has been added (June 2020). Thanks, Laurent
Re: [PATCH v2 12/15] virtio-net: Store failover primary opts pointer locally
On 08/10/2021 15:34, Kevin Wolf wrote:
Instead of accessing the global QemuOptsList, which really belong to the
command line parser and shouldn't be accessed from devices, store a
pointer to the QemuOpts in a new VirtIONet field.
This is not the final state, but just an intermediate step to get rid of
QemuOpts in devices. It will later be replaced with an options QDict.
Before this patch, two "primary" devices could be hidden for the same
standby device, but only one of them would actually be enabled and the
other one would be kept hidden forever, so this doesn't make sense.
After this patch, configuring a second primary device is an error.
Signed-off-by: Kevin Wolf
---
include/hw/virtio/virtio-net.h | 1 +
hw/net/virtio-net.c| 26 ++
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/include/hw/virtio/virtio-net.h b/include/hw/virtio/virtio-net.h
index 824a69c23f..d118c95f69 100644
--- a/include/hw/virtio/virtio-net.h
+++ b/include/hw/virtio/virtio-net.h
@@ -209,6 +209,7 @@ struct VirtIONet {
bool failover_primary_hidden;
bool failover;
DeviceListener primary_listener;
+QemuOpts *primary_opts;
Notifier migration_state;
VirtioNetRssData rss_data;
struct NetRxPkt *rx_pkt;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index a17d5739fc..ed9a9012e9 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -858,27 +858,24 @@ static DeviceState
*failover_find_primary_device(VirtIONet *n)
static void failover_add_primary(VirtIONet *n, Error **errp)
{
Error *err = NULL;
-QemuOpts *opts;
-char *id;
DeviceState *dev = failover_find_primary_device(n);
if (dev) {
return;
}
-id = failover_find_primary_device_id(n);
-if (!id) {
+if (!n->primary_opts) {
error_setg(errp, "Primary device not found");
error_append_hint(errp, "Virtio-net failover will not work. Make "
"sure primary device has parameter"
" failover_pair_id=%s\n", n->netclient_name);
return;
}
-opts = qemu_opts_find(qemu_find_opts("device"), id);
-g_assert(opts); /* cannot be NULL because id was found using opts list */
-dev = qdev_device_add(opts, );
+
+dev = qdev_device_add(n->primary_opts, );
if (err) {
-qemu_opts_del(opts);
+qemu_opts_del(n->primary_opts);
+n->primary_opts = NULL;
} else {
object_unref(OBJECT(dev));
}
@@ -3317,6 +3314,19 @@ static bool failover_hide_primary_device(DeviceListener
*listener,
return false;
}
+if (n->primary_opts) {
+error_setg(errp, "Cannot attach more than one primary device to '%s'",
+ n->netclient_name);
+return false;
+}
+
This part has introduced a regression, I've sent a patch to fix that.
https://patchew.org/QEMU/20211019071532.682717-1-lviv...@redhat.com/
Thanks,
Laurent
Re: [PATCH v2 14/15] qdev: Base object creation on QDict rather than QemuOpts
On 08/10/2021 15:34, Kevin Wolf wrote:
QDicts are both what QMP natively uses and what the keyval parser
produces. Going through QemuOpts isn't useful for either one, so switch
the main device creation function to QDicts. By sharing more code with
the -object/object-add code path, we can even reduce the code size a
bit.
This commit doesn't remove the detour through QemuOpts from any code
path yet, but it allows the following commits to do so.
Signed-off-by: Kevin Wolf
---
include/hw/qdev-core.h | 11 +++---
include/hw/virtio/virtio-net.h | 3 +-
include/monitor/qdev.h | 2 +
hw/core/qdev.c | 7 ++--
hw/net/virtio-net.c| 23 +++-
hw/vfio/pci.c | 4 +-
softmmu/qdev-monitor.c | 69 +++---
7 files changed, 60 insertions(+), 59 deletions(-)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 74d8b614a7..910042c650 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -180,7 +180,7 @@ struct DeviceState {
char *canonical_path;
bool realized;
bool pending_deleted_event;
-QemuOpts *opts;
+QDict *opts;
int hotplugged;
bool allow_unplug_during_migration;
BusState *parent_bus;
@@ -205,8 +205,8 @@ struct DeviceListener {
* On errors, it returns false and errp is set. Device creation
* should fail in this case.
*/
-bool (*hide_device)(DeviceListener *listener, QemuOpts *device_opts,
-Error **errp);
+bool (*hide_device)(DeviceListener *listener, const QDict *device_opts,
+bool from_json, Error **errp);
QTAILQ_ENTRY(DeviceListener) link;
};
@@ -835,13 +835,14 @@ void device_listener_unregister(DeviceListener *listener);
/**
* @qdev_should_hide_device:
- * @opts: QemuOpts as passed on cmdline.
+ * @opts: options QDict > + * @from_json: true if @opts entries are typed,
false for all strings
Add the errp here too:
* @errp: pointer to error object
*
* Check if a device should be added.
* When a device is added via qdev_device_add() this will be called,
* and return if the device should be added now or not.
*/
-bool qdev_should_hide_device(QemuOpts *opts, Error **errp);
+bool qdev_should_hide_device(const QDict *opts, bool from_json, Error **errp);
typedef enum MachineInitPhase {
/* current_machine is NULL. */
Thank you to have added the errp pointer in the hide_device helpers, it helps
in my series.
Laurent
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
On 06/10/2021 12:53, Kevin Wolf wrote: Am 06.10.2021 um 11:20 hat Laurent Vivier geschrieben: On 06/10/2021 10:21, Juan Quintela wrote: Kevin Wolf wrote: Am 05.10.2021 um 17:52 hat Damien Hedde geschrieben: Hi Usage - The primary device can be hotplugged or be part of the startup configuration -device virtio-net-pci,netdev=hostnet1,id=net1, mac=52:54:00:6f:55:cc,bus=root2,failover=on With the parameter failover=on the VIRTIO_NET_F_STANDBY feature will be enabled. -device vfio-pci,host=5e:00.2,id=hostdev0,bus=root1, failover_pair_id=net1 failover_pair_id references the id of the virtio-net standby device. This is only for pairing the devices within QEMU. The guest kernel module net_failover will match devices with identical MAC addresses. Hotplug --- Both primary and standby device can be hotplugged via the QEMU monitor. Note that if the virtio-net device is plugged first a warning will be issued that it couldn't find the primary device. So maybe this whole primary device lookup can happen during the -device CLI option creation loop. And we can indeed have un-created devices still in the list ? Yes, that's the only case for which I could imagine for an inconsistency between the qdev tree and QemuOpts, but failover_add_primary() is only called after feature negotiation with the guest driver, so we can be sure that the -device loop has completed long ago. And even if it hadn't completed yet, the paragraph also says that even hotplugging the device later is supported, so creating devices in the wrong order should still succeed. I hope that some of the people I added to CC have some more hints. Failover is ... interesting. You have two devices: primary and seconday. seconday is virtio-net, primary can be vfio and some other emulated devices. In the command line, devices can appear on any order, primary then secondary, secondary then primary, or only one of them. You can add (any of them) later in the toplevel. And now, what all this mess is about. We only enable the primary if the guest knows about failover. Otherwise we use only the virtio device (*). The important bit here is that we need to wait until the guest is booted, and the virtio-net driver is loaded, and then it tells us if it understands failover (or not). At that point we decide if we want to "really" create the primary. I know that it abuses device_add() as much as it can be, but I can't see any better way to handle it. We need to be able to "create" a device without showing it to the guest. And later, when we create a different device, and depending of driver support on the guest, we "finish" the creation of the primary device. Any good idea? Hm, the naive idea would be creating the device without attaching it to any bus. But I suppose qdev doesn't let you do that. Anyway, the part that I missed yesterday is that qdev_device_add() already skips creating the device if qdev_should_hide_device(), which explains how the inconsistency is created. (As an aside, it then returns NULL without setting an error to indicate success, which is an awkward interface, and sure enough, qmp_device_add() gets it wrong and deletes the QemuOpts again. So hotplugging the virtio-net standby device doesn't even seem to work?) Could we just save the configuration in the .hide_device callback (i.e. failover_hide_primary_device() in virtio-net) to a new field in VirtIONet and then use that when actually creating the device instead of accessing the command line state in the QemuOptsList? It seems that we can currently add two primary devices that are then both hidden. failover_add_primary() adds only one of them, leaving the other one hidden. Is this a bug and we should reject such a configuration or do we need to support keeping configurations for multiple primary devices in a single standby device? This would still be ugly because the configuration is only really validated when the primary device is actually added instead of immediately on -device/device_add, but at least it would keep the ugliness more local and wouldn't block the move away from QemuOpts (the config would just be stored as a QDict after my patches). I don't know if it can help the discussion, but I'm reformatting the failover code to move all the PCI stuff to pci files. And there is a lot of inconsistencies regarding the device_add and --device option so I've been in the end to add a list of of hidden devices rather than relying on the command line. See PATCH 8 of series "[RFC PATCH v2 0/8] virtio-net failover cleanup and new features" https://patchew.org/QEMU/20210820142002.152994-1-lviv...@redhat.com/ While it's certainly an improvement over the current state, we really should move away from QemuOpts and I think using global state for this I totally agree with that. is wrong anyway. So it feels like it's not the change we need here, but more a step sideways. Yes, I wan
Re: [PATCH 09/11] qdev: Avoid QemuOpts in QMP device_add
On 06/10/2021 10:21, Juan Quintela wrote: Kevin Wolf wrote: Am 05.10.2021 um 17:52 hat Damien Hedde geschrieben: Hi Usage - The primary device can be hotplugged or be part of the startup configuration -device virtio-net-pci,netdev=hostnet1,id=net1, mac=52:54:00:6f:55:cc,bus=root2,failover=on With the parameter failover=on the VIRTIO_NET_F_STANDBY feature will be enabled. -device vfio-pci,host=5e:00.2,id=hostdev0,bus=root1, failover_pair_id=net1 failover_pair_id references the id of the virtio-net standby device. This is only for pairing the devices within QEMU. The guest kernel module net_failover will match devices with identical MAC addresses. Hotplug --- Both primary and standby device can be hotplugged via the QEMU monitor. Note that if the virtio-net device is plugged first a warning will be issued that it couldn't find the primary device. So maybe this whole primary device lookup can happen during the -device CLI option creation loop. And we can indeed have un-created devices still in the list ? Yes, that's the only case for which I could imagine for an inconsistency between the qdev tree and QemuOpts, but failover_add_primary() is only called after feature negotiation with the guest driver, so we can be sure that the -device loop has completed long ago. And even if it hadn't completed yet, the paragraph also says that even hotplugging the device later is supported, so creating devices in the wrong order should still succeed. I hope that some of the people I added to CC have some more hints. Failover is ... interesting. You have two devices: primary and seconday. seconday is virtio-net, primary can be vfio and some other emulated devices. In the command line, devices can appear on any order, primary then secondary, secondary then primary, or only one of them. You can add (any of them) later in the toplevel. And now, what all this mess is about. We only enable the primary if the guest knows about failover. Otherwise we use only the virtio device (*). The important bit here is that we need to wait until the guest is booted, and the virtio-net driver is loaded, and then it tells us if it understands failover (or not). At that point we decide if we want to "really" create the primary. I know that it abuses device_add() as much as it can be, but I can't see any better way to handle it. We need to be able to "create" a device without showing it to the guest. And later, when we create a different device, and depending of driver support on the guest, we "finish" the creation of the primary device. Any good idea? I don't know if it can help the discussion, but I'm reformatting the failover code to move all the PCI stuff to pci files. And there is a lot of inconsistencies regarding the device_add and --device option so I've been in the end to add a list of of hidden devices rather than relying on the command line. See PATCH 8 of series "[RFC PATCH v2 0/8] virtio-net failover cleanup and new features" https://patchew.org/QEMU/20210820142002.152994-1-lviv...@redhat.com/ Thanks, Laurent
Re: [PATCH 6/7] linux-user: use GDateTime for formatting timestamp for core file
Le 05/05/2021 à 12:37, Daniel P. Berrangé a écrit :
> The GDateTime APIs provided by GLib avoid portability pitfalls, such
> as some platforms where 'struct timeval.tv_sec' field is still 'long'
> instead of 'time_t'. When combined with automatic cleanup, GDateTime
> often results in simpler code too.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> linux-user/elfload.c | 36 +---
> 1 file changed, 9 insertions(+), 27 deletions(-)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index c6731013fd..c38b7b4d37 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3386,7 +3386,6 @@ static size_t note_size(const struct memelfnote *);
> static void free_note_info(struct elf_note_info *);
> static int fill_note_info(struct elf_note_info *, long, const CPUArchState
> *);
> static void fill_thread_info(struct elf_note_info *, const CPUArchState *);
> -static int core_dump_filename(const TaskState *, char *, size_t);
>
> static int dump_write(int, const void *, size_t);
> static int write_note(struct memelfnote *, int);
> @@ -3685,32 +3684,16 @@ static void fill_auxv_note(struct memelfnote *note,
> const TaskState *ts)
> * for the name:
> * qemu__-_.core
> *
> - * Returns 0 in case of success, -1 otherwise (errno is set).
> + * Returns the filename
> */
> -static int core_dump_filename(const TaskState *ts, char *buf,
> - size_t bufsize)
> +static char *core_dump_filename(const TaskState *ts)
> {
> -char timestamp[64];
> -char *base_filename = NULL;
> -struct timeval tv;
> -struct tm tm;
> +g_autoptr(GDateTime) now = g_date_time_new_now_local();
> +g_autofree char *nowstr = g_date_time_format(now, "%Y%m%d-%H%M%S");
> +g_autofree char *base_filename = g_path_get_basename(ts->bprm->filename);
>
> -assert(bufsize >= PATH_MAX);
> -
> -if (gettimeofday(, NULL) < 0) {
> -(void) fprintf(stderr, "unable to get current timestamp: %s",
> - strerror(errno));
> -return (-1);
> -}
> -
> -base_filename = g_path_get_basename(ts->bprm->filename);
> -(void) strftime(timestamp, sizeof (timestamp), "%Y%m%d-%H%M%S",
> -localtime_r(_sec, ));
> -(void) snprintf(buf, bufsize, "qemu_%s_%s_%d.core",
> -base_filename, timestamp, (int)getpid());
> -g_free(base_filename);
> -
> -return (0);
> +return g_strdup_printf("qemu_%s_%s_%d.core",
> + base_filename, nowstr, (int)getpid());
> }
>
> static int dump_write(int fd, const void *ptr, size_t size)
> @@ -3938,7 +3921,7 @@ static int elf_core_dump(int signr, const CPUArchState
> *env)
> const CPUState *cpu = env_cpu((CPUArchState *)env);
> const TaskState *ts = (const TaskState *)cpu->opaque;
> struct vm_area_struct *vma = NULL;
> -char corefile[PATH_MAX];
> +g_autofree char *corefile = NULL;
> struct elf_note_info info;
> struct elfhdr elf;
> struct elf_phdr phdr;
> @@ -3955,8 +3938,7 @@ static int elf_core_dump(int signr, const CPUArchState
> *env)
> if (dumpsize.rlim_cur == 0)
> return 0;
>
> -if (core_dump_filename(ts, corefile, sizeof (corefile)) < 0)
> -return (-errno);
> +corefile = core_dump_filename(ts);
>
> if ((fd = open(corefile, O_WRONLY | O_CREAT,
> S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH)) < 0)
>
Applied to my linux-user-for-6.1 branch.
Thanks,
Laurent
Re: [PATCH 0/3] hw/virtio: Constify VirtIOFeature
Le 11/05/2021 à 12:41, Philippe Mathieu-Daudé a écrit : > Trivial patches to keep VirtIOFeature arrays read-only > (better safe than sorry). > > Philippe Mathieu-Daudé (3): > hw/virtio: Pass virtio_feature_get_config_size() a const argument > virtio-blk: Constify VirtIOFeature feature_sizes[] > virtio-net: Constify VirtIOFeature feature_sizes[] > > include/hw/virtio/virtio.h | 2 +- > hw/block/virtio-blk.c | 2 +- > hw/net/virtio-net.c| 2 +- > hw/virtio/virtio.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > Series applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH 6/7] linux-user: use GDateTime for formatting timestamp for core file
Le 05/05/2021 à 12:37, Daniel P. Berrangé a écrit : > The GDateTime APIs provided by GLib avoid portability pitfalls, such > as some platforms where 'struct timeval.tv_sec' field is still 'long' > instead of 'time_t'. When combined with automatic cleanup, GDateTime > often results in simpler code too. > > Signed-off-by: Daniel P. Berrangé > --- > linux-user/elfload.c | 36 +--- > 1 file changed, 9 insertions(+), 27 deletions(-) > Reviewed-by: Laurent Vivier
Re: [PATCH 2/3] hw/ide: Add Kconfig dependency MICRODRIVE -> PCMCIA
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit : > The Microdrive Compact Flash can be plugged on a PCMCIA bus. > Express the dependency using the 'depends on' Kconfig expression. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/ide/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/ide/Kconfig b/hw/ide/Kconfig > index 5d9106b1ac2..8e2c8934549 100644 > --- a/hw/ide/Kconfig > +++ b/hw/ide/Kconfig > @@ -41,6 +41,7 @@ config IDE_VIA > config MICRODRIVE > bool > select IDE_QDEV > +depends on PCMCIA > > config AHCI > bool > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH 3/3] hw/pcmcia: Do not register PCMCIA type if not required
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit :
> If the Kconfig 'PCMCIA' value is not selected, it is pointless
> to build the PCMCIA core components.
>
> (Currently only one machine of the ARM targets requires this).
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/pcmcia/meson.build | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/pcmcia/meson.build b/hw/pcmcia/meson.build
> index ab50bd325d6..51f2512b8ed 100644
> --- a/hw/pcmcia/meson.build
> +++ b/hw/pcmcia/meson.build
> @@ -1,2 +1,2 @@
> -softmmu_ss.add(files('pcmcia.c'))
> +softmmu_ss.add(when: 'CONFIG_PCMCIA', if_true: files('pcmcia.c'))
> softmmu_ss.add(when: 'CONFIG_PXA2XX', if_true: files('pxa2xx.c'))
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH 1/3] hw/arm/pxa2xx: Declare PCMCIA bus with Kconfig
Le 25/04/2021 à 00:20, Philippe Mathieu-Daudé a écrit : > The Intel XScale PXA chipsets provide a PCMCIA controller, > which expose a PCMCIA (IDE) bus. Express this dependency using > the Kconfig 'select' expression. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/arm/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig > index 8c37cf00da7..b887f6a5b17 100644 > --- a/hw/arm/Kconfig > +++ b/hw/arm/Kconfig > @@ -142,6 +142,7 @@ config PXA2XX > select SD > select SSI > select USB_OHCI > +select PCMCIA > > config GUMSTIX > bool > Applied to my trivial-patches branch. Thanks, Laurent
[PATCH v3 3/6] blockdev: with -drive if=virtio, use generic virtio-blk
Rather than checking if the machine is an s390x to use virtio-blk-ccw
instead of virtio-blk-pci, use the alias virtio-blk that is set to
the expected target.
This also enables the use of virtio-blk-device for targets without
PCI or CCW.
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Signed-off-by: Laurent Vivier
---
blockdev.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5cc7c7effe9f..64da5350e3ad 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -969,11 +969,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type,
QemuOpts *devopts;
devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-ccw", _abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", _abort);
-}
+qemu_opt_set(devopts, "driver", "virtio-blk", _abort);
qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
_abort);
}
--
2.30.2
[PATCH v3 5/6] iotests: test m68k with the virt machine
This allows to cover the virtio tests with a 32bit big-endian
virtio-mmio machine.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/testenv.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1f7..6d27712617a3 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -208,6 +208,7 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
('arm', 'virt'),
('aarch64', 'virt'),
('avr', 'mega2560'),
+('m68k', 'virt'),
('rx', 'gdbsim-r5f562n8'),
('tricore', 'tricore_testboard')
)
--
2.30.2
[PATCH v3 2/6] m68k: add the virtio devices aliases
Similarly to 5f629d943cb0 ("s390x: fix s390 virtio aliases"),
define the virtio aliases.
This allows to start machines with virtio devices without
knowledge of the implementation type.
For instance, we can use "-device virtio-scsi" on
m68k, s390x or PC, and the device will be respectively
"virtio-scsi-device", "virtio-scsi-ccw" or "virtio-scsi-pci".
This already exists for s390x and -ccw interfaces, add them
for m68k and MMIO (-device) interfaces.
Reviewed-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
Signed-off-by: Laurent Vivier
---
include/sysemu/arch_init.h | 1 +
softmmu/qdev-monitor.c | 12
2 files changed, 13 insertions(+)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 0c9070651664..16da27969627 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -42,5 +42,6 @@ int xen_available(void);
QEMU_ARCH_RISCV | QEMU_ARCH_SH4 | \
QEMU_ARCH_SPARC | QEMU_ARCH_XTENSA)
#define QEMU_ARCH_VIRTIO_CCW (QEMU_ARCH_S390X)
+#define QEMU_ARCH_VIRTIO_MMIO (QEMU_ARCH_M68K)
#endif
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 0b40c97c6e50..a9955b97a078 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -60,29 +60,41 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
+{ "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-balloon-device", "virtio-balloon", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-balloon-pci", "virtio-balloon", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-blk-device", "virtio-blk", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-device", "virtio-gpu", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-keyboard-device", "virtio-keyboard", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-keyboard-pci", "virtio-keyboard", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-mouse-device", "virtio-mouse", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-net-device", "virtio-net", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-net-pci", "virtio-net", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-rng-device", "virtio-rng", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-scsi-device", "virtio-scsi", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-serial-device", "virtio-serial", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
+{ "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
{ }
--
2.30.2
[PATCH v3 6/6] iotests: iothreads need ioeventfd
And ioeventfd are only available with virtio-scsi-pci or virtio-scsi-ccw,
use the alias but add a rule to require virtio-scsi-pci or virtio-scsi-ccw
for the tests that use iothreads.
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/127| 3 ++-
tests/qemu-iotests/256| 6 --
tests/qemu-iotests/common.rc | 13 +
tests/qemu-iotests/iotests.py | 5 +
4 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index 98e8e82a8210..32edc3b0685e 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file fuse
-_require_devices virtio-scsi scsi-hd
+_require_devices scsi-hd
+_require_one_device_of virtio-scsi-pci virtio-scsi-ccw
IMG_SIZE=64K
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index 8d82a1dd865f..13666813bd8f 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -24,6 +24,8 @@ import os
import iotests
from iotests import log
+iotests._verify_virtio_scsi_pci_or_ccw()
+
iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024
@@ -61,8 +63,8 @@ with iotests.FilePath('img0') as img0_path, \
log('--- Preparing images & VM ---\n')
vm.add_object('iothread,id=iothread0')
vm.add_object('iothread,id=iothread1')
-vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
-vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+vm.add_device('virtio-scsi,id=scsi0,iothread=iothread0')
+vm.add_device('virtio-scsi,id=scsi1,iothread=iothread1')
iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
vm.add_drive(img0_path, interface='none')
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 65cdba5723ba..7f49c9716db7 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -977,5 +977,18 @@ _require_devices()
done
}
+_require_one_device_of()
+{
+available=$($QEMU -M none -device help | \
+grep ^name | sed -e 's/^name "//' -e 's/".*$//')
+for device
+do
+if echo "$available" | grep -q "$device" ; then
+return
+fi
+done
+_notrun "$* not available"
+}
+
# make sure this script returns success
true
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1e9e6a066e90..5af01828951e 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1146,6 +1146,11 @@ def _verify_virtio_blk() -> None:
if 'virtio-blk' not in out:
notrun('Missing virtio-blk in QEMU binary')
+def _verify_virtio_scsi_pci_or_ccw() -> None:
+out = qemu_pipe('-M', 'none', '-device', 'help')
+if 'virtio-scsi-pci' not in out and 'virtio-scsi-ccw' not in out:
+notrun('Missing virtio-scsi-pci or virtio-scsi-ccw in QEMU binary')
+
def supports_quorum():
return 'quorum' in qemu_img_pipe('--help')
--
2.30.2
[PATCH v3 4/6] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"
Commit f1d5516ab583 introduces a test in some iotests to check if
the machine is a s390-ccw-virtio and to select virtio-*-ccw rather
than virtio-*-pci.
We don't need that because QEMU already provides aliases to use the correct
virtio interface according to the machine type.
This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-*
instead and remove get_virtio_scsi_device().
This also enables virtio-mmio devices (virtio-*-device)
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé
Reviewed-by: Cornelia Huck
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/040| 2 +-
tests/qemu-iotests/051| 12 +---
tests/qemu-iotests/051.out| 2 +-
tests/qemu-iotests/051.pc.out | 2 +-
tests/qemu-iotests/068| 4 +---
tests/qemu-iotests/093| 3 +--
tests/qemu-iotests/139| 9 ++---
tests/qemu-iotests/182| 13 ++---
tests/qemu-iotests/238| 4 +---
tests/qemu-iotests/240| 10 +-
tests/qemu-iotests/257| 4 ++--
tests/qemu-iotests/307| 4 +---
tests/qemu-iotests/iotests.py | 5 -
13 files changed, 19 insertions(+), 55 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 336ff7c4f2ab..ba7cb34ce8cf 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -89,7 +89,7 @@ class TestSingleDrive(ImageCommitTestCase):
qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
mid_img)
self.vm = iotests.VM().add_drive(test_img,
"node-name=top,backing.node-name=mid,backing.backing.node-name=base",
interface="none")
-self.vm.add_device(iotests.get_virtio_scsi_device())
+self.vm.add_device('virtio-scsi')
self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
self.vm.launch()
self.has_quit = False
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 7cbd1415ce7b..00382cc55e25 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -119,17 +119,7 @@ echo
echo === Device without drive ===
echo
-case "$QEMU_DEFAULT_MACHINE" in
- s390-ccw-virtio)
- virtio_scsi=virtio-scsi-ccw
- ;;
- *)
- virtio_scsi=virtio-scsi-pci
- ;;
-esac
-
-run_qemu -device $virtio_scsi -device scsi-hd |
-sed -e "s/$virtio_scsi/VIRTIO_SCSI/"
+run_qemu -device virtio-scsi -device scsi-hd
echo
echo === Overriding backing file ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb36d..437053c8395c 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node name
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index f570610f645f..c4e011369809 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node-name: 'fo
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 03e03508a6e2..54e49c8ffab1 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -49,11 +49,9 @@ IMG_SIZE=128K
case "$QEMU_DEFAULT_MACHINE" in
s390-ccw-virtio)
platform_parm="-no-shutdown"
- hba=virtio-scsi-ccw
;;
*)
platform_parm=""
- hba=virtio-scsi-pci
;;
esac
@@ -61,7 +59,7 @@ _qemu()
{
$QEMU $platform_parm -nographic -monitor stdio -serial none \
-drive if=none,id=drive0,file="$TEST_IMG",format="$IMGFMT" \
- -device $hba,id=hba0 \
+ -device virtio-scsi,id=hba0 \
-device scsi-hd,drive=drive0 \
"$@" |\
_filter_qemu | _filter_hmp
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 7745cb04b611..93274dc8cbde 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -371,8 +371,7 @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
class ThrottleTestRemovableMedia(iotests.QMPTestCase):
def setUp(self):
self.vm = iotests.VM()
-self.vm.add_device("{},id=virtio-scsi".format(
-iotests.get_virtio_scsi_device()))
+self.vm.add_device("{},id=virtio-scsi".format('virtio-scsi'))
self.vm.launch()
def tearDown(self):
diff --git a/tes
[PATCH v3 0/6] iotests: fix failures with non-PCI machines
Tests are executed using virtio-*-pci even on a non PCI machine. The problem can be easily fixed using the virtio aliases (virtio-*), to run virtio-*-ccw on s390x and virtio-*-device on m68k. A first attempt was tried with virtio-*-ccw by detecting the machine type, this series removes it to use the aliases that are a cleaner approach. v3: Fix typos and comments Add _require_one_device_of() v2: Fix typos and comments Don't define QEMU_ARCH_NO_PCI but QEMU_ARCH_VIRTIO_PCI, QEMU_ARCH_VIRTIO_CCW and QEMU_ARCH_VIRTIO_MMIO Update test 127 and 256 to run with ccw Update .out of test 051 Add a patch to update "-drive if=virtio" Laurent Vivier (6): qdev: define list of archs with virtio-pci or virtio-ccw m68k: add the virtio devices aliases blockdev: with -drive if=virtio, use generic virtio-blk iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" iotests: test m68k with the virt machine iotests: iothreads need ioeventfd include/sysemu/arch_init.h| 9 + blockdev.c| 6 +--- softmmu/qdev-monitor.c| 65 --- tests/qemu-iotests/040| 2 +- tests/qemu-iotests/051| 12 +-- tests/qemu-iotests/051.out| 2 +- tests/qemu-iotests/051.pc.out | 2 +- tests/qemu-iotests/068| 4 +-- tests/qemu-iotests/093| 3 +- tests/qemu-iotests/127| 3 +- tests/qemu-iotests/139| 9 ++--- tests/qemu-iotests/182| 13 ++- tests/qemu-iotests/238| 4 +-- tests/qemu-iotests/240| 10 +++--- tests/qemu-iotests/256| 6 ++-- tests/qemu-iotests/257| 4 +-- tests/qemu-iotests/307| 4 +-- tests/qemu-iotests/common.rc | 13 +++ tests/qemu-iotests/iotests.py | 10 +++--- tests/qemu-iotests/testenv.py | 1 + 20 files changed, 91 insertions(+), 91 deletions(-) -- 2.30.2
[PATCH v3 1/6] qdev: define list of archs with virtio-pci or virtio-ccw
This is used to define virtio-*-pci and virtio-*-ccw aliases
rather than substracting the CCW architecture from all the others.
Reviewed-by: Cornelia Huck
Signed-off-by: Laurent Vivier
---
include/sysemu/arch_init.h | 8 ++
softmmu/qdev-monitor.c | 53 ++
2 files changed, 33 insertions(+), 28 deletions(-)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 54f069d49126..0c9070651664 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -35,4 +35,12 @@ extern const uint32_t arch_type;
int kvm_available(void);
int xen_available(void);
+/* default virtio transport per architecture */
+#define QEMU_ARCH_VIRTIO_PCI (QEMU_ARCH_ALPHA | QEMU_ARCH_ARM | \
+ QEMU_ARCH_HPPA | QEMU_ARCH_I386 | \
+ QEMU_ARCH_MIPS | QEMU_ARCH_PPC | \
+ QEMU_ARCH_RISCV | QEMU_ARCH_SH4 | \
+ QEMU_ARCH_SPARC | QEMU_ARCH_XTENSA)
+#define QEMU_ARCH_VIRTIO_CCW (QEMU_ARCH_S390X)
+
#endif
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 8dc656becca9..0b40c97c6e50 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -60,34 +60,31 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
-{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
-{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X },
-{ "virtio-balloon-pci", "virtio-balloon",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
-{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_S390X },
-{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
-{ "virtio-input-host-pci", "virtio-input-host",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
-{ "virtio-keyboard-pci", "virtio-keyboard",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_S390X },
-{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
-{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_S390X },
-{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X },
-{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
-{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
-{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-balloon-pci", "virtio-balloon", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-keyboard-pci", "virtio-keyboard", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-net-pci", "virtio-net", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
+{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
{ }
};
--
2.30.2
Re: [PATCH v2 3/6] blockdev: with -drive if=virtio, use generic virtio-blk
Le 19/03/2021 à 15:46, Cornelia Huck a écrit :
> On Fri, 19 Mar 2021 14:25:34 +0100
> Laurent Vivier wrote:
>
>> Rather than checking if the machine is an s390x to use virtio-blk-ccw
>> instead of virtio-blk-pci, use the alias virtio-blk that is set to
>> the expected target.
>
> One side effect: if we add a new architecture and don't define the
> aliases for it, this function probably won't do the right thing; prior
> to the patch, it would simply default to virtio-blk-pci. Probably not a
> big deal, but we need to be careful to keep the alias defines up to
> date, which previously wasn't such a big deal.
But it will be easy to detect because we will have the error "unknown device:
virtio-blk".
It will be a good reminder to add the aliases...
Thanks,
Laurent
>
>>
>> This also enables the use of virtio-blk-device for targets without
>> PCI or CCW.
>>
>> Signed-off-by: Laurent Vivier
>> ---
>> blockdev.c | 6 +-
>> 1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/blockdev.c b/blockdev.c
>> index 5cc7c7effe9f..64da5350e3ad 100644
>> --- a/blockdev.c
>> +++ b/blockdev.c
>> @@ -969,11 +969,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
>> BlockInterfaceType block_default_type,
>> QemuOpts *devopts;
>> devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
>> _abort);
>> -if (arch_type == QEMU_ARCH_S390X) {
>> -qemu_opt_set(devopts, "driver", "virtio-blk-ccw", _abort);
>> -} else {
>> -qemu_opt_set(devopts, "driver", "virtio-blk-pci", _abort);
>> -}
>> +qemu_opt_set(devopts, "driver", "virtio-blk", _abort);
>> qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
>> _abort);
>> }
>
Re: [PATCH v2 6/6] iotests: iothreads need ioeventfd
Le 19/03/2021 à 15:23, Cornelia Huck a écrit :
> On Fri, 19 Mar 2021 14:51:59 +0100
> Laurent Vivier wrote:
>
>> Le 19/03/2021 à 14:36, Philippe Mathieu-Daudé a écrit :
>>> On 3/19/21 2:25 PM, Laurent Vivier wrote:
>>>> And ioeventfd are only available with virtio-scsi-pci or virtio-scsi-ccw,
>>>> use the alias but add a rule to require virtio-scsi-pci or virtio-scsi-ccw
>>>> for the tests that use iothreads.
>>>>
>>>> Signed-off-by: Laurent Vivier
>>>> ---
>>>> tests/qemu-iotests/127| 3 ++-
>>>> tests/qemu-iotests/256| 6 --
>>>> tests/qemu-iotests/iotests.py | 5 +
>>>> 3 files changed, 11 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
>>>> index 98e8e82a8210..abe24861100d 100755
>>>> --- a/tests/qemu-iotests/127
>>>> +++ b/tests/qemu-iotests/127
>>>> @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
>>>> _supported_fmt qcow2
>>>> _supported_proto file fuse
>>>>
>>>> -_require_devices virtio-scsi scsi-hd
>>>> +_require_devices scsi-hd
>>>> +_require_devices virtio-scsi-pci || _require_devices virtio-scsi-ccw
>>
>> Re-reading the code, I think this cannot work because we have an "exit" if
>> the test fails.
>
> We could try to make _require_devices accept alternatives, but that is
> probably overkill...
>
>>
>> The test is executed anyway because s390x provides virtio-scsi-ccw and
>> virtio-scsi-pci.
>
> ...because of this.
>
> Maybe just add a comment that we require pci or ccw because iothreads
> depend on ioventfd, but checking for pci is enough, as we have pci when
> we have ccw?
>
Well... bash is fun:
_require_one_device_of()
{
available=$($QEMU -M none -device help | \
grep ^name | sed -e 's/^name "//' -e 's/".*$//')
for device
do
if echo "$available" | grep -q "$device" ; then
return
fi
done
_notrun "$* not available"
}
and:
_require_one_device_of virtio-scsi-pci virtio-scsi-ccw
Thanks,
Laurent
Re: [PATCH v2 6/6] iotests: iothreads need ioeventfd
Le 19/03/2021 à 14:36, Philippe Mathieu-Daudé a écrit : > On 3/19/21 2:25 PM, Laurent Vivier wrote: >> And ioeventfd are only available with virtio-scsi-pci or virtio-scsi-ccw, >> use the alias but add a rule to require virtio-scsi-pci or virtio-scsi-ccw >> for the tests that use iothreads. >> >> Signed-off-by: Laurent Vivier >> --- >> tests/qemu-iotests/127| 3 ++- >> tests/qemu-iotests/256| 6 -- >> tests/qemu-iotests/iotests.py | 5 + >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127 >> index 98e8e82a8210..abe24861100d 100755 >> --- a/tests/qemu-iotests/127 >> +++ b/tests/qemu-iotests/127 >> @@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >> _supported_fmt qcow2 >> _supported_proto file fuse >> >> -_require_devices virtio-scsi scsi-hd >> +_require_devices scsi-hd >> +_require_devices virtio-scsi-pci || _require_devices virtio-scsi-ccw Re-reading the code, I think this cannot work because we have an "exit" if the test fails. The test is executed anyway because s390x provides virtio-scsi-ccw and virtio-scsi-pci. Thanks, Laurent
[PATCH v2 5/6] iotests: test m68k with the virt machine
This allows to cover the virtio tests with a 32bit big-endian
virtio-mmio machine.
Reviewed-by: Philippe Mathieu-Daudé
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/testenv.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1f7..6d27712617a3 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -208,6 +208,7 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
('arm', 'virt'),
('aarch64', 'virt'),
('avr', 'mega2560'),
+('m68k', 'virt'),
('rx', 'gdbsim-r5f562n8'),
('tricore', 'tricore_testboard')
)
--
2.30.2
[PATCH v2 3/6] blockdev: with -drive if=virtio, use generic virtio-blk
Rather than checking if the machine is an s390x to use virtio-blk-ccw
instead of virtio-blk-pci, use the alias virtio-blk that is set to
the expected target.
This also enables the use of virtio-blk-device for targets without
PCI or CCW.
Signed-off-by: Laurent Vivier
---
blockdev.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5cc7c7effe9f..64da5350e3ad 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -969,11 +969,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type,
QemuOpts *devopts;
devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-ccw", _abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", _abort);
-}
+qemu_opt_set(devopts, "driver", "virtio-blk", _abort);
qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
_abort);
}
--
2.30.2
[PATCH v2 4/6] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"
Commit f1d5516ab583 introduces a test in some iotests to check if
the machine is a s390-ccw-virtio and to select virtio-*-ccw rather
than virtio-*-pci.
We don't need that because QEMU already provides aliases to use the correct
virtio interface according to the machine type.
This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-*
instead. This also enables virtio-mmio devices (virtio-*-device)
Signed-off-by: Laurent Vivier
cc: Cornelia Huck
---
tests/qemu-iotests/040| 2 +-
tests/qemu-iotests/051| 12 +---
tests/qemu-iotests/051.out| 2 +-
tests/qemu-iotests/051.pc.out | 2 +-
tests/qemu-iotests/068| 4 +---
tests/qemu-iotests/093| 3 +--
tests/qemu-iotests/139| 9 ++---
tests/qemu-iotests/182| 13 ++---
tests/qemu-iotests/238| 4 +---
tests/qemu-iotests/240| 10 +-
tests/qemu-iotests/257| 4 ++--
tests/qemu-iotests/307| 4 +---
tests/qemu-iotests/iotests.py | 5 -
13 files changed, 19 insertions(+), 55 deletions(-)
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 336ff7c4f2ab..ba7cb34ce8cf 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -89,7 +89,7 @@ class TestSingleDrive(ImageCommitTestCase):
qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
mid_img)
self.vm = iotests.VM().add_drive(test_img,
"node-name=top,backing.node-name=mid,backing.backing.node-name=base",
interface="none")
-self.vm.add_device(iotests.get_virtio_scsi_device())
+self.vm.add_device('virtio-scsi')
self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
self.vm.launch()
self.has_quit = False
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 7cbd1415ce7b..00382cc55e25 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -119,17 +119,7 @@ echo
echo === Device without drive ===
echo
-case "$QEMU_DEFAULT_MACHINE" in
- s390-ccw-virtio)
- virtio_scsi=virtio-scsi-ccw
- ;;
- *)
- virtio_scsi=virtio-scsi-pci
- ;;
-esac
-
-run_qemu -device $virtio_scsi -device scsi-hd |
-sed -e "s/$virtio_scsi/VIRTIO_SCSI/"
+run_qemu -device virtio-scsi -device scsi-hd
echo
echo === Overriding backing file ===
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index de4771bcb36d..437053c8395c 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node name
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index f570610f645f..c4e011369809 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -72,7 +72,7 @@ QEMU_PROG: -drive file=TEST_DIR/t.qcow2,node-name=foo#12:
Invalid node-name: 'fo
=== Device without drive ===
-Testing: -device VIRTIO_SCSI -device scsi-hd
+Testing: -device virtio-scsi -device scsi-hd
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) QEMU_PROG: -device scsi-hd: drive property not set
diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 03e03508a6e2..54e49c8ffab1 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -49,11 +49,9 @@ IMG_SIZE=128K
case "$QEMU_DEFAULT_MACHINE" in
s390-ccw-virtio)
platform_parm="-no-shutdown"
- hba=virtio-scsi-ccw
;;
*)
platform_parm=""
- hba=virtio-scsi-pci
;;
esac
@@ -61,7 +59,7 @@ _qemu()
{
$QEMU $platform_parm -nographic -monitor stdio -serial none \
-drive if=none,id=drive0,file="$TEST_IMG",format="$IMGFMT" \
- -device $hba,id=hba0 \
+ -device virtio-scsi,id=hba0 \
-device scsi-hd,drive=drive0 \
"$@" |\
_filter_qemu | _filter_hmp
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 7745cb04b611..93274dc8cbde 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -371,8 +371,7 @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
class ThrottleTestRemovableMedia(iotests.QMPTestCase):
def setUp(self):
self.vm = iotests.VM()
-self.vm.add_device("{},id=virtio-scsi".format(
-iotests.get_virtio_scsi_device()))
+self.vm.add_device("{},id=virtio-scsi".format('virtio-scsi'))
self.vm.launch()
def tearDown(self):
diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index e79b3c21fdce..178b1ee230ca 100755
--- a/tests/qemu-iotests/
[PATCH v2 1/6] qdev: define list of archs with virtio-pci or virtio-ccw
This is used to define virtio-*-pci and virtio-*-ccw aliases
rather than substracting the CCW architecture from all the others.
Signed-off-by: Laurent Vivier
---
include/sysemu/arch_init.h | 7 +
softmmu/qdev-monitor.c | 53 ++
2 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 54f069d49126..7217a822a14b 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -35,4 +35,11 @@ extern const uint32_t arch_type;
int kvm_available(void);
int xen_available(void);
+#define QEMU_ARCH_VIRTIO_PCI (QEMU_ARCH_ALPHA | QEMU_ARCH_ARM | \
+ QEMU_ARCH_HPPA | QEMU_ARCH_I386 | \
+ QEMU_ARCH_MIPS | QEMU_ARCH_PPC | \
+ QEMU_ARCH_RISCV | QEMU_ARCH_SH4 | \
+ QEMU_ARCH_SPARC | QEMU_ARCH_XTENSA)
+#define QEMU_ARCH_VIRTIO_CCW (QEMU_ARCH_S390X)
+
#endif
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 8dc656becca9..0b40c97c6e50 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -60,34 +60,31 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
-{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
-{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X },
-{ "virtio-balloon-pci", "virtio-balloon",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
-{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_S390X },
-{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
-{ "virtio-input-host-pci", "virtio-input-host",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
-{ "virtio-keyboard-pci", "virtio-keyboard",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_S390X },
-{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
-{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_S390X },
-{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X },
-{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
-{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
-{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-balloon-pci", "virtio-balloon", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-keyboard-pci", "virtio-keyboard", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-net-pci", "virtio-net", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
+{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
+{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
{ }
};
--
2.30.2
[PATCH v2 2/6] m68k: add the virtio devices aliases
Similarly to 5f629d943cb0 ("s390x: fix s390 virtio aliases"),
define the virtio aliases.
This allows to start machines with virtio devices without
knowledge of the implementation type.
For instance, we can use "-device virtio-scsi" on
m68k, s390x or PC, and the device will be respectively
"virtio-scsi-device", "virtio-scsi-ccw" or "virtio-scsi-pci".
This already exists for s390x and -ccw interfaces, add them
for m68k and MMIO (-device) interfaces.
Signed-off-by: Laurent Vivier
---
include/sysemu/arch_init.h | 1 +
softmmu/qdev-monitor.c | 12
2 files changed, 13 insertions(+)
diff --git a/include/sysemu/arch_init.h b/include/sysemu/arch_init.h
index 7217a822a14b..c6e34124e8a3 100644
--- a/include/sysemu/arch_init.h
+++ b/include/sysemu/arch_init.h
@@ -41,5 +41,6 @@ int xen_available(void);
QEMU_ARCH_RISCV | QEMU_ARCH_SH4 | \
QEMU_ARCH_SPARC | QEMU_ARCH_XTENSA)
#define QEMU_ARCH_VIRTIO_CCW (QEMU_ARCH_S390X)
+#define QEMU_ARCH_VIRTIO_MMIO (QEMU_ARCH_M68K)
#endif
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 0b40c97c6e50..a9955b97a078 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -60,29 +60,41 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
+{ "virtio-9p-device", "virtio-9p", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-balloon-device", "virtio-balloon", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-balloon-pci", "virtio-balloon", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-blk-device", "virtio-blk", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-gpu-device", "virtio-gpu", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_VIRTIO_PCI },
{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-keyboard-device", "virtio-keyboard", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-keyboard-pci", "virtio-keyboard", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-mouse-device", "virtio-mouse", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-net-device", "virtio-net", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-net-pci", "virtio-net", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-rng-device", "virtio-rng", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-scsi-device", "virtio-scsi", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_VIRTIO_PCI },
+{ "virtio-serial-device", "virtio-serial", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_VIRTIO_PCI},
+{ "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_VIRTIO_MMIO },
{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_VIRTIO_CCW },
{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_VIRTIO_PCI },
{ }
--
2.30.2
[PATCH v2 6/6] iotests: iothreads need ioeventfd
And ioeventfd are only available with virtio-scsi-pci or virtio-scsi-ccw,
use the alias but add a rule to require virtio-scsi-pci or virtio-scsi-ccw
for the tests that use iothreads.
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/127| 3 ++-
tests/qemu-iotests/256| 6 --
tests/qemu-iotests/iotests.py | 5 +
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index 98e8e82a8210..abe24861100d 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -44,7 +44,8 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file fuse
-_require_devices virtio-scsi scsi-hd
+_require_devices scsi-hd
+_require_devices virtio-scsi-pci || _require_devices virtio-scsi-ccw
IMG_SIZE=64K
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index 8d82a1dd865f..13666813bd8f 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -24,6 +24,8 @@ import os
import iotests
from iotests import log
+iotests._verify_virtio_scsi_pci_or_ccw()
+
iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024
@@ -61,8 +63,8 @@ with iotests.FilePath('img0') as img0_path, \
log('--- Preparing images & VM ---\n')
vm.add_object('iothread,id=iothread0')
vm.add_object('iothread,id=iothread1')
-vm.add_device('virtio-scsi-pci,id=scsi0,iothread=iothread0')
-vm.add_device('virtio-scsi-pci,id=scsi1,iothread=iothread1')
+vm.add_device('virtio-scsi,id=scsi0,iothread=iothread0')
+vm.add_device('virtio-scsi,id=scsi1,iothread=iothread1')
iotests.qemu_img_create('-f', iotests.imgfmt, img0_path, str(size))
iotests.qemu_img_create('-f', iotests.imgfmt, img1_path, str(size))
vm.add_drive(img0_path, interface='none')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1e9e6a066e90..d3faf96005dd 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1146,6 +1146,11 @@ def _verify_virtio_blk() -> None:
if 'virtio-blk' not in out:
notrun('Missing virtio-blk in QEMU binary')
+def _verify_virtio_scsi_pci_or_ccw() -> None:
+out = qemu_pipe('-M', 'none', '-device', 'help')
+if 'virtio-scsi-pci' not in out and 'virtio-scsi-ccw' not in out:
+notrun('Missing virtio-scsi-pci and virtio-scsi-ccw in QEMU binary')
+
def supports_quorum():
return 'quorum' in qemu_img_pipe('--help')
--
2.30.2
[PATCH v2 0/6] iotests: fix failures with non-PCI machines
Tests are executed using virtio-*-pci even on a non PCI machine. The problem can be easily fixed using the virtio aliases (virtio-*), to run virtio-*-ccw on s390x and virtio-*-device on m68k. A first attempt was tried with virtio-*-ccw by detecting the machine type, this series removes it to use the aliases that are a cleaner approach. v2: Fix typos and comments Don't define QEMU_ARCH_NO_PCI but QEMU_ARCH_VIRTIO_PCI, QEMU_ARCH_VIRTIO_CCW and QEMU_ARCH_VIRTIO_MMIO Update test 127 and 256 to run with ccw Update .out of test 051 Add a patch to update "-drive if=virtio" Laurent Vivier (6): qdev: define list of archs with virtio-pci or virtio-ccw m68k: add the virtio devices aliases blockdev: with -drive if=virtio, use generic virtio-blk iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" iotests: test m68k with the virt machine iotests: iothreads need ioeventfd include/sysemu/arch_init.h| 8 + blockdev.c| 6 +--- softmmu/qdev-monitor.c| 65 --- tests/qemu-iotests/040| 2 +- tests/qemu-iotests/051| 12 +-- tests/qemu-iotests/051.out| 2 +- tests/qemu-iotests/051.pc.out | 2 +- tests/qemu-iotests/068| 4 +-- tests/qemu-iotests/093| 3 +- tests/qemu-iotests/127| 3 +- tests/qemu-iotests/139| 9 ++--- tests/qemu-iotests/182| 13 ++- tests/qemu-iotests/238| 4 +-- tests/qemu-iotests/240| 10 +++--- tests/qemu-iotests/256| 6 ++-- tests/qemu-iotests/257| 4 +-- tests/qemu-iotests/307| 4 +-- tests/qemu-iotests/iotests.py | 10 +++--- tests/qemu-iotests/testenv.py | 1 + 19 files changed, 77 insertions(+), 91 deletions(-) -- 2.30.2
Re: [PATCH 2/4] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"
Le 19/03/2021 à 12:43, Cornelia Huck a écrit : > On Thu, 18 Mar 2021 23:39:05 +0100 > Laurent Vivier wrote: > >> Commit f1d5516ab583 introduces a test in some iotests to check if >> the machine is a s390-ssw-virtio and to select virtio-*-ccw rather > > s/ssw/ccw/ > >> than virtio-*-pci. >> >> We don't need that because QEMU already provides aliases to use the correct >> virtio interface according to the machine type. > > Maybe add a comment that this also enables virtio-mmio? ok >> >> This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-* >> instead. >> >> Signed-off-by: Laurent Vivier >> cc: Cornelia Huck >> --- >> blockdev.c| 6 +- > > Hm, that also tweaks how -drive behaves, IIUC. Intended, I think; but > worth a note as well? > You're right. I think it needs a separate patch, in fact. Thanks, Laurent
Re: [PATCH 1/4] m68k: add the virtio devices aliases
Le 19/03/2021 à 12:36, Cornelia Huck a écrit :
> On Thu, 18 Mar 2021 23:39:04 +0100
> Laurent Vivier wrote:
>
>> Similarly to 5f629d943cb0 ("s390x: fix s390 virtio aliases"),
>> define the virtio aliases.
>>
>> This allows to start machines with virtio devices without
>> knowledge of the implementation type.
>>
>> For instance, we can use "-device virtio-scsi" on
>> m68k, s390x or PC, and the device will be
>> "virtio-scsi-device", "virtio-scsi-ccw" or "virtio-scsi-pci".
>>
>> This already exists for s390x and -ccw interfaces, adds them
>> for m68k and MMIO (-device) interfaces.
>>
>> Signed-off-by: Laurent Vivier
>> ---
>> softmmu/qdev-monitor.c | 46 +++---
>> 1 file changed, 30 insertions(+), 16 deletions(-)
>>
>> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
>> index 8dc656becca9..262d38b8c01e 100644
>> --- a/softmmu/qdev-monitor.c
>> +++ b/softmmu/qdev-monitor.c
>> @@ -42,6 +42,8 @@
>> #include "hw/qdev-properties.h"
>> #include "hw/clock.h"
>>
>> +#define QEMU_ARCH_NO_PCI (QEMU_ARCH_S390X | QEMU_ARCH_M68K)
>
> The name of the #define is a tad misleading (we do have virtio-pci
> devices on s390x, unlike in 2012, we just don't want the aliases to
> point to them.) Maybe QEMU_ARCH_NONPCI_DEFAULT?
I have changed this patch to define QEMU_ARCH_VIRTIO_PCI with the list of archs
with virtio-pci
devices, and QEMU_ARCH_VIRTIO_CCW and then QEMU_ARCH_VIRTIO_MMIO
>
>> +
>> /*
>> * Aliases were a bad idea from the start. Let's keep them
>> * from spreading further.
>
> Otherwise, LGTM.
>
Thanks,
Laurent
Re: [PATCH 4/4] iotests: iothreads need ioeventfd
Le 19/03/2021 à 12:27, Cornelia Huck a écrit : > On Fri, 19 Mar 2021 12:06:43 +0100 > Paolo Bonzini wrote: > >> On 18/03/21 23:39, Laurent Vivier wrote: >>> And ioeventfd are only available with virtio-scsi-pci, so don't use the >>> alias >>> and add a rule to require virtio-scsi-pci for the tests that use iothreads. >>> >>> Signed-off-by: Laurent Vivier >>> --- >>> tests/qemu-iotests/127| 4 ++-- >>> tests/qemu-iotests/256| 2 ++ >>> tests/qemu-iotests/iotests.py | 5 + >>> 3 files changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127 >>> index 98e8e82a8210..a3693533685a 100755 >>> --- a/tests/qemu-iotests/127 >>> +++ b/tests/qemu-iotests/127 >>> @@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15 >>> _supported_fmt qcow2 >>> _supported_proto file fuse >>> >>> -_require_devices virtio-scsi scsi-hd >>> +_require_devices virtio-scsi-pci scsi-hd >> >> Maybe >> >> _require_devices scsi-hd >> _require_devices virtio-scsi-pci || _require_devices virtio-scsi ccw >> >> ? >> >> Paolo >> > > Yes, ioeventfds are also available for ccw; I'd expect only mmio to be > the problem here. > OK, thanks. I update my patch with the changes from Paolo. Laurent
Re: [PULL 5/5] m68k: add Virtual M68k Machine
Le 19/03/2021 à 10:29, Paolo Bonzini a écrit : > On 19/03/21 10:20, Max Reitz wrote: >> On 19.03.21 07:32, Thomas Huth wrote: >>> On 18/03/2021 18.28, Max Reitz wrote: >>> [...] From that it follows that I don’t see much use in testing specific devices either. Say there’s a platform that provides both virtio-pci and virtio-mmio, the default (say virtio-pci) is fine for the iotests. I see little value in testing virtio-mmio as well. (Perhaps I’m short-sighted, though.) >>> >>> That's a fair point. But still, if someone compiled QEMU only with a target >>> that only provided >>> virtio-mmio, the iotests should not fail when running "make check". >>> To avoid that we continue playing whack-a-mole here in the future, maybe it >>> would be better to >>> restrict the iotests to the "main" targets only, e.g. modify check-block.sh >>> so that the tests >>> only run with x86, aarch64, s390x and ppc64 ? >> >> Right, that would certainly be the simplest solution. > > It would also make the patches that Laurent sent this morning unnecessary, > and avoid the use of > aliases in the tests (so that it's clear what is tested). > We don't test the virtio frontend, but the blockdev backend, so we don't care what we use here. Aliases simplify the code... Thanks, Laurent
Re: [PULL 5/5] m68k: add Virtual M68k Machine
Le 19/03/2021 à 10:20, Max Reitz a écrit : > On 19.03.21 07:32, Thomas Huth wrote: >> On 18/03/2021 18.28, Max Reitz wrote: >> [...] >>> From that it follows that I don’t see much use in testing specific devices >>> either. Say there’s >>> a platform that provides both virtio-pci and virtio-mmio, the default (say >>> virtio-pci) is fine >>> for the iotests. I see little value in testing virtio-mmio as well. >>> (Perhaps I’m short-sighted, >>> though.) >> >> That's a fair point. But still, if someone compiled QEMU only with a target >> that only provided >> virtio-mmio, the iotests should not fail when running "make check". >> To avoid that we continue playing whack-a-mole here in the future, maybe it >> would be better to >> restrict the iotests to the "main" targets only, e.g. modify check-block.sh >> so that the tests only >> run with x86, aarch64, s390x and ppc64 ? > > Right, that would certainly be the simplest solution. > The problem with that is we can't run the tests if target-list doesn't contain one of these targets. Thanks, Laurent
[PATCH 4/4] iotests: iothreads need ioeventfd
And ioeventfd are only available with virtio-scsi-pci, so don't use the alias
and add a rule to require virtio-scsi-pci for the tests that use iothreads.
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/127| 4 ++--
tests/qemu-iotests/256| 2 ++
tests/qemu-iotests/iotests.py | 5 +
3 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/127 b/tests/qemu-iotests/127
index 98e8e82a8210..a3693533685a 100755
--- a/tests/qemu-iotests/127
+++ b/tests/qemu-iotests/127
@@ -44,7 +44,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
_supported_fmt qcow2
_supported_proto file fuse
-_require_devices virtio-scsi scsi-hd
+_require_devices virtio-scsi-pci scsi-hd
IMG_SIZE=64K
@@ -62,7 +62,7 @@ $QEMU_IO -c 'write 0 42' "$TEST_IMG.overlay0" |
_filter_qemu_io
_launch_qemu \
-object iothread,id=iothr \
-blockdev
node-name=source,driver=$IMGFMT,file.driver=file,file.filename="$TEST_IMG.overlay0"
\
--device virtio-scsi,id=scsi-bus,iothread=iothr \
+-device virtio-scsi-pci,id=scsi-bus,iothread=iothr \
-device scsi-hd,bus=scsi-bus.0,drive=source
_send_qemu_cmd $QEMU_HANDLE \
diff --git a/tests/qemu-iotests/256 b/tests/qemu-iotests/256
index 8d82a1dd865f..eb3af0dea80c 100755
--- a/tests/qemu-iotests/256
+++ b/tests/qemu-iotests/256
@@ -24,6 +24,8 @@ import os
import iotests
from iotests import log
+iotests._verify_virtio_scsi_pci()
+
iotests.script_initialize(supported_fmts=['qcow2'])
size = 64 * 1024 * 1024
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 1e9e6a066e90..3404ed534bb5 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -1146,6 +1146,11 @@ def _verify_virtio_blk() -> None:
if 'virtio-blk' not in out:
notrun('Missing virtio-blk in QEMU binary')
+def _verify_virtio_scsi_pci() -> None:
+out = qemu_pipe('-M', 'none', '-device', 'help')
+if 'virtio-scsi-pci' not in out:
+notrun('Missing virtio-scsi-pci in QEMU binary')
+
def supports_quorum():
return 'quorum' in qemu_img_pipe('--help')
--
2.30.2
[PATCH 3/4] iotests: test m68k with the virt machine
This allows to cover the virtio tests with a 32bit big-endian
virtio-mmio machine.
Signed-off-by: Laurent Vivier
---
tests/qemu-iotests/testenv.py | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py
index 1fbec854c1f7..6d27712617a3 100644
--- a/tests/qemu-iotests/testenv.py
+++ b/tests/qemu-iotests/testenv.py
@@ -208,6 +208,7 @@ def __init__(self, imgfmt: str, imgproto: str, aiomode: str,
('arm', 'virt'),
('aarch64', 'virt'),
('avr', 'mega2560'),
+('m68k', 'virt'),
('rx', 'gdbsim-r5f562n8'),
('tricore', 'tricore_testboard')
)
--
2.30.2
[PATCH 2/4] iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182"
Commit f1d5516ab583 introduces a test in some iotests to check if
the machine is a s390-ssw-virtio and to select virtio-*-ccw rather
than virtio-*-pci.
We don't need that because QEMU already provides aliases to use the correct
virtio interface according to the machine type.
This patch removes all virtio-*-pci and virtio-*-ccw to use virtio-*
instead.
Signed-off-by: Laurent Vivier
cc: Cornelia Huck
---
blockdev.c| 6 +-
tests/qemu-iotests/040| 2 +-
tests/qemu-iotests/051| 12 +---
tests/qemu-iotests/068| 4 +---
tests/qemu-iotests/093| 3 +--
tests/qemu-iotests/139| 9 ++---
tests/qemu-iotests/182| 13 ++---
tests/qemu-iotests/238| 4 +---
tests/qemu-iotests/240| 10 +-
tests/qemu-iotests/257| 4 ++--
tests/qemu-iotests/307| 4 +---
tests/qemu-iotests/iotests.py | 5 -
12 files changed, 18 insertions(+), 58 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 5cc7c7effe9f..64da5350e3ad 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -969,11 +969,7 @@ DriveInfo *drive_new(QemuOpts *all_opts,
BlockInterfaceType block_default_type,
QemuOpts *devopts;
devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
_abort);
-if (arch_type == QEMU_ARCH_S390X) {
-qemu_opt_set(devopts, "driver", "virtio-blk-ccw", _abort);
-} else {
-qemu_opt_set(devopts, "driver", "virtio-blk-pci", _abort);
-}
+qemu_opt_set(devopts, "driver", "virtio-blk", _abort);
qemu_opt_set(devopts, "drive", qdict_get_str(bs_opts, "id"),
_abort);
}
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
index 336ff7c4f2ab..ba7cb34ce8cf 100755
--- a/tests/qemu-iotests/040
+++ b/tests/qemu-iotests/040
@@ -89,7 +89,7 @@ class TestSingleDrive(ImageCommitTestCase):
qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img)
qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288',
mid_img)
self.vm = iotests.VM().add_drive(test_img,
"node-name=top,backing.node-name=mid,backing.backing.node-name=base",
interface="none")
-self.vm.add_device(iotests.get_virtio_scsi_device())
+self.vm.add_device('virtio-scsi')
self.vm.add_device("scsi-hd,id=scsi0,drive=drive0")
self.vm.launch()
self.has_quit = False
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 7cbd1415ce7b..00382cc55e25 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -119,17 +119,7 @@ echo
echo === Device without drive ===
echo
-case "$QEMU_DEFAULT_MACHINE" in
- s390-ccw-virtio)
- virtio_scsi=virtio-scsi-ccw
- ;;
- *)
- virtio_scsi=virtio-scsi-pci
- ;;
-esac
-
-run_qemu -device $virtio_scsi -device scsi-hd |
-sed -e "s/$virtio_scsi/VIRTIO_SCSI/"
+run_qemu -device virtio-scsi -device scsi-hd
echo
echo === Overriding backing file ===
diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
index 03e03508a6e2..54e49c8ffab1 100755
--- a/tests/qemu-iotests/068
+++ b/tests/qemu-iotests/068
@@ -49,11 +49,9 @@ IMG_SIZE=128K
case "$QEMU_DEFAULT_MACHINE" in
s390-ccw-virtio)
platform_parm="-no-shutdown"
- hba=virtio-scsi-ccw
;;
*)
platform_parm=""
- hba=virtio-scsi-pci
;;
esac
@@ -61,7 +59,7 @@ _qemu()
{
$QEMU $platform_parm -nographic -monitor stdio -serial none \
-drive if=none,id=drive0,file="$TEST_IMG",format="$IMGFMT" \
- -device $hba,id=hba0 \
+ -device virtio-scsi,id=hba0 \
-device scsi-hd,drive=drive0 \
"$@" |\
_filter_qemu | _filter_hmp
diff --git a/tests/qemu-iotests/093 b/tests/qemu-iotests/093
index 7745cb04b611..93274dc8cbde 100755
--- a/tests/qemu-iotests/093
+++ b/tests/qemu-iotests/093
@@ -371,8 +371,7 @@ class ThrottleTestGroupNames(iotests.QMPTestCase):
class ThrottleTestRemovableMedia(iotests.QMPTestCase):
def setUp(self):
self.vm = iotests.VM()
-self.vm.add_device("{},id=virtio-scsi".format(
-iotests.get_virtio_scsi_device()))
+self.vm.add_device("{},id=virtio-scsi".format('virtio-scsi'))
self.vm.launch()
def tearDown(self):
diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
index e79b3c21fdce..178b1ee230ca 100755
--- a/tests/qemu-iotests/139
+++ b/tests/qemu-iotests/139
@@ -26,18 +26,13 @@ import time
base_img = os.path.join(iotests.test_dir, 'base.img')
new_img = os.path.join(iotests.test_dir, 'new.img')
-if iotests.qemu_default_machine == 's390-ccw-virtio':
-default_virtio_blk = 'virtio-blk-ccw'
-else:
-default_virtio_blk = '
[PATCH 0/4] iotests: fix failures with non-PCI machines
Tests are executed using virtio-*-pci even on a non PCI machine. The problem can be easily fixed using the virtio aliases (virtio-*), to run virtio-*-ccw on s390x and virtio-*-device on m68k. A first attempt was tried with virtio-*-ccw by detecting the machine type, this series removes it to use the aliases that are a cleaner approach. Laurent Vivier (4): m68k: add the virtio devices aliases iotests: Revert "iotests: use -ccw on s390x for 040, 139, and 182" iotests: test m68k with the virt machine iotests: iothreads need ioeventfd blockdev.c| 6 + softmmu/qdev-monitor.c| 46 +++ tests/qemu-iotests/040| 2 +- tests/qemu-iotests/051| 12 + tests/qemu-iotests/068| 4 +-- tests/qemu-iotests/093| 3 +-- tests/qemu-iotests/127| 4 +-- tests/qemu-iotests/139| 9 ++- tests/qemu-iotests/182| 13 ++ tests/qemu-iotests/238| 4 +-- tests/qemu-iotests/240| 10 tests/qemu-iotests/256| 2 ++ tests/qemu-iotests/257| 4 +-- tests/qemu-iotests/307| 4 +-- tests/qemu-iotests/iotests.py | 10 tests/qemu-iotests/testenv.py | 1 + 16 files changed, 58 insertions(+), 76 deletions(-) -- 2.30.2
[PATCH 1/4] m68k: add the virtio devices aliases
Similarly to 5f629d943cb0 ("s390x: fix s390 virtio aliases"),
define the virtio aliases.
This allows to start machines with virtio devices without
knowledge of the implementation type.
For instance, we can use "-device virtio-scsi" on
m68k, s390x or PC, and the device will be
"virtio-scsi-device", "virtio-scsi-ccw" or "virtio-scsi-pci".
This already exists for s390x and -ccw interfaces, adds them
for m68k and MMIO (-device) interfaces.
Signed-off-by: Laurent Vivier
---
softmmu/qdev-monitor.c | 46 +++---
1 file changed, 30 insertions(+), 16 deletions(-)
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 8dc656becca9..262d38b8c01e 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -42,6 +42,8 @@
#include "hw/qdev-properties.h"
#include "hw/clock.h"
+#define QEMU_ARCH_NO_PCI (QEMU_ARCH_S390X | QEMU_ARCH_M68K)
+
/*
* Aliases were a bad idea from the start. Let's keep them
* from spreading further.
@@ -60,34 +62,46 @@ static const QDevAlias qdev_alias_table[] = {
{ "ES1370", "es1370" }, /* -soundhw name */
{ "ich9-ahci", "ahci" },
{ "lsi53c895a", "lsi" },
+{ "virtio-9p-device", "virtio-9p", QEMU_ARCH_M68K },
{ "virtio-9p-ccw", "virtio-9p", QEMU_ARCH_S390X },
-{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-9p-pci", "virtio-9p", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-balloon-device", "virtio-balloon", QEMU_ARCH_M68K },
{ "virtio-balloon-ccw", "virtio-balloon", QEMU_ARCH_S390X },
-{ "virtio-balloon-pci", "virtio-balloon",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-balloon-pci", "virtio-balloon", QEMU_ARCH_ALL &
+ ~QEMU_ARCH_NO_PCI },
+{ "virtio-blk-device", "virtio-blk", QEMU_ARCH_M68K },
{ "virtio-blk-ccw", "virtio-blk", QEMU_ARCH_S390X },
-{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-blk-pci", "virtio-blk", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
{ "virtio-gpu-ccw", "virtio-gpu", QEMU_ARCH_S390X },
-{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-gpu-device", "virtio-gpu", QEMU_ARCH_M68K },
+{ "virtio-gpu-pci", "virtio-gpu", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-input-host-device", "virtio-input-host", QEMU_ARCH_M68K },
{ "virtio-input-host-ccw", "virtio-input-host", QEMU_ARCH_S390X },
-{ "virtio-input-host-pci", "virtio-input-host",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
-{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-input-host-pci", "virtio-input-host", QEMU_ARCH_ALL &
+ ~QEMU_ARCH_NO_PCI },
+{ "virtio-iommu-pci", "virtio-iommu", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-keyboard-device", "virtio-keyboard", QEMU_ARCH_M68K },
{ "virtio-keyboard-ccw", "virtio-keyboard", QEMU_ARCH_S390X },
-{ "virtio-keyboard-pci", "virtio-keyboard",
-QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-keyboard-pci", "virtio-keyboard", QEMU_ARCH_ALL &
+ ~QEMU_ARCH_NO_PCI },
+{ "virtio-mouse-device", "virtio-mouse", QEMU_ARCH_M68K },
{ "virtio-mouse-ccw", "virtio-mouse", QEMU_ARCH_S390X },
-{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-mouse-pci", "virtio-mouse", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-net-device", "virtio-net", QEMU_ARCH_M68K },
{ "virtio-net-ccw", "virtio-net", QEMU_ARCH_S390X },
-{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-net-pci", "virtio-net", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-rng-device", "virtio-rng", QEMU_ARCH_M68K },
{ "virtio-rng-ccw", "virtio-rng", QEMU_ARCH_S390X },
-{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-rng-pci", "virtio-rng", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-scsi-device", "virtio-scsi", QEMU_ARCH_M68K },
{ "virtio-scsi-ccw", "virtio-scsi", QEMU_ARCH_S390X },
-{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-scsi-pci", "virtio-scsi", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI },
+{ "virtio-serial-device", "virtio-serial", QEMU_ARCH_M68K },
{ "virtio-serial-ccw", "virtio-serial", QEMU_ARCH_S390X },
-{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-serial-pci", "virtio-serial", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI},
+{ "virtio-tablet-device", "virtio-tablet", QEMU_ARCH_M68K },
{ "virtio-tablet-ccw", "virtio-tablet", QEMU_ARCH_S390X },
-{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_S390X },
+{ "virtio-tablet-pci", "virtio-tablet", QEMU_ARCH_ALL & ~QEMU_ARCH_NO_PCI
},
{ }
};
--
2.30.2
Re: [PATCH 2/2] sysemu: Let VMChangeStateHandler take boolean 'running' argument
Le 11/01/2021 à 16:20, Philippe Mathieu-Daudé a écrit :
> The 'running' argument from VMChangeStateHandler does not require
> other value than 0 / 1. Make it a plain boolean.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/sysemu/runstate.h | 10 --
> target/arm/kvm_arm.h| 2 +-
> target/ppc/cpu-qom.h| 2 +-
> accel/xen/xen-all.c | 2 +-
> audio/audio.c | 2 +-
> block/block-backend.c | 2 +-
> gdbstub.c | 2 +-
> hw/block/pflash_cfi01.c | 2 +-
> hw/block/virtio-blk.c | 2 +-
> hw/display/qxl.c| 2 +-
> hw/i386/kvm/clock.c | 2 +-
> hw/i386/kvm/i8254.c | 2 +-
> hw/i386/kvmvapic.c | 2 +-
> hw/i386/xen/xen-hvm.c | 2 +-
> hw/ide/core.c | 2 +-
> hw/intc/arm_gicv3_its_kvm.c | 2 +-
> hw/intc/arm_gicv3_kvm.c | 2 +-
> hw/intc/spapr_xive_kvm.c| 2 +-
> hw/misc/mac_via.c | 2 +-
> hw/net/e1000e_core.c| 2 +-
> hw/nvram/spapr_nvram.c | 2 +-
> hw/ppc/ppc.c| 2 +-
> hw/ppc/ppc_booke.c | 2 +-
> hw/s390x/tod-kvm.c | 2 +-
> hw/scsi/scsi-bus.c | 2 +-
> hw/usb/hcd-ehci.c | 2 +-
> hw/usb/host-libusb.c| 2 +-
> hw/usb/redirect.c | 2 +-
> hw/vfio/migration.c | 2 +-
> hw/virtio/virtio-rng.c | 2 +-
> hw/virtio/virtio.c | 2 +-
> net/net.c | 2 +-
> softmmu/memory.c| 2 +-
> softmmu/runstate.c | 2 +-
> target/arm/kvm.c| 2 +-
> target/i386/kvm/kvm.c | 2 +-
> target/i386/sev.c | 2 +-
> target/i386/whpx/whpx-all.c | 2 +-
> target/mips/kvm.c | 4 ++--
> ui/gtk.c| 2 +-
> ui/spice-core.c | 2 +-
> 41 files changed, 49 insertions(+), 43 deletions(-)
>
> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index 3ab35a039a0..a5356915734 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -10,7 +10,7 @@ bool runstate_is_running(void);
> bool runstate_needs_reset(void);
> bool runstate_store(char *str, size_t size);
>
> -typedef void VMChangeStateHandler(void *opaque, int running, RunState state);
> +typedef void VMChangeStateHandler(void *opaque, bool running, RunState
> state);
>
> VMChangeStateEntry *qemu_add_vm_change_state_handler(VMChangeStateHandler
> *cb,
> void *opaque);
> @@ -20,7 +20,13 @@ VMChangeStateEntry
> *qdev_add_vm_change_state_handler(DeviceState *dev,
> VMChangeStateHandler
> *cb,
> void *opaque);
> void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
> -void vm_state_notify(int running, RunState state);
> +/**
> + * vm_state_notify: Notify the state of the VM
> + *
> + * @running: whether the VM is running or not.
> + * @state: the #RunState of the VM.
> + */
> +void vm_state_notify(bool running, RunState state);
>
> static inline bool shutdown_caused_by_guest(ShutdownCause cause)
> {
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index eb81b7059eb..68ec970c4f4 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -352,7 +352,7 @@ void kvm_arm_get_virtual_time(CPUState *cs);
> */
> void kvm_arm_put_virtual_time(CPUState *cs);
>
> -void kvm_arm_vm_state_change(void *opaque, int running, RunState state);
> +void kvm_arm_vm_state_change(void *opaque, bool running, RunState state);
>
> int kvm_arm_vgic_probe(void);
>
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 63b9e8632ca..118baf8d41f 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -218,7 +218,7 @@ extern const VMStateDescription vmstate_ppc_timebase;
> .offset = vmstate_offset_value(_state, _field, PPCTimebase), \
> }
>
> -void cpu_ppc_clock_vm_state_change(void *opaque, int running,
> +void cpu_ppc_clock_vm_state_change(void *opaque, bool running,
> RunState state);
> #endif
>
> diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
> index 878a4089d97..3756aca27be 100644
> --- a/accel/xen/xen-all.c
> +++ b/accel/xen/xen-all.c
> @@ -122,7 +122,7 @@ static void xenstore_record_dm_state(struct xs_handle
> *xs, const char *state)
> }
>
>
> -static void xen_change_state_handler(void *opaque, int running,
> +static void xen_change_state_handler(void *opaque, bool running,
> RunState state)
> {
> if (running) {
> diff --git a/audio/audio.c b/audio/audio.c
> index b48471bb3f6..f2d56e7e57d 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -1549,7 +1549,7 @@ static int audio_driver_init(AudioState *s, struct
> audio_driver *drv,
> }
> }
>
> -static void audio_vm_change_state_handler (void *opaque, int running,
>
Re: [PATCH 1/2] sysemu/runstate: Let runstate_is_running() return bool
Le 11/01/2021 à 16:20, Philippe Mathieu-Daudé a écrit :
> runstate_check() returns a boolean. runstate_is_running()
> returns what runstate_check() returns, also a boolean.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/sysemu/runstate.h | 2 +-
> softmmu/runstate.c| 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/include/sysemu/runstate.h b/include/sysemu/runstate.h
> index e557f470d42..3ab35a039a0 100644
> --- a/include/sysemu/runstate.h
> +++ b/include/sysemu/runstate.h
> @@ -6,7 +6,7 @@
>
> bool runstate_check(RunState state);
> void runstate_set(RunState new_state);
> -int runstate_is_running(void);
> +bool runstate_is_running(void);
> bool runstate_needs_reset(void);
> bool runstate_store(char *str, size_t size);
>
> diff --git a/softmmu/runstate.c b/softmmu/runstate.c
> index 636aab0addb..c7a67147d17 100644
> --- a/softmmu/runstate.c
> +++ b/softmmu/runstate.c
> @@ -217,7 +217,7 @@ void runstate_set(RunState new_state)
> current_run_state = new_state;
> }
>
> -int runstate_is_running(void)
> +bool runstate_is_running(void)
> {
> return runstate_check(RUN_STATE_RUNNING);
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 1/5] ui: Replace the word 'whitelist'
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit :
> Follow the inclusive terminology from the "Conscious Language in your
> Open Source Projects" guidelines [*] and replace the words "whitelist"
> appropriately.
>
> [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md
>
> Reviewed-by: Gerd Hoffmann
> Reviewed-by: Alex Bennée
> Reviewed-by: Thomas Huth
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> ui/console.c | 2 +-
> ui/vnc-auth-sasl.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/ui/console.c b/ui/console.c
> index c5d11bc7017..5a8311ced20 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -1708,7 +1708,7 @@ bool dpy_gfx_check_format(QemuConsole *con,
> return false;
> }
> } else {
> -/* default is to whitelist native 32 bpp only */
> +/* default is to allow native 32 bpp only */
> if (format != qemu_default_pixman_format(32, true)) {
> return false;
> }
> diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
> index f67111a3662..df7dc08e9fc 100644
> --- a/ui/vnc-auth-sasl.c
> +++ b/ui/vnc-auth-sasl.c
> @@ -288,7 +288,7 @@ static int protocol_client_auth_sasl_step(VncState *vs,
> uint8_t *data, size_t le
> goto authreject;
> }
>
> -/* Check username whitelist ACL */
> +/* Check the username access control list */
> if (vnc_auth_sasl_check_access(vs) < 0) {
> goto authreject;
> }
> @@ -409,7 +409,7 @@ static int protocol_client_auth_sasl_start(VncState *vs,
> uint8_t *data, size_t l
> goto authreject;
> }
>
> -/* Check username whitelist ACL */
> +/* Check the username access control list */
> if (vnc_auth_sasl_check_access(vs) < 0) {
> goto authreject;
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 3/5] seccomp: Replace the word 'blacklist'
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit :
> Follow the inclusive terminology from the "Conscious Language in your
> Open Source Projects" guidelines [*] and replace the word "blacklist"
> appropriately.
>
> [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md
>
> Reviewed-by: Daniel P. Berrangé
> Acked-by: Eduardo Otubo
> Reviewed-by: Alex Bennée
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> v3: Reworded comment (thuth)
> ---
> softmmu/qemu-seccomp.c | 16
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/softmmu/qemu-seccomp.c b/softmmu/qemu-seccomp.c
> index 377ef6937ca..9c29d9cf007 100644
> --- a/softmmu/qemu-seccomp.c
> +++ b/softmmu/qemu-seccomp.c
> @@ -45,8 +45,8 @@ const struct scmp_arg_cmp sched_setscheduler_arg[] = {
> { .arg = 1, .op = SCMP_CMP_NE, .datum_a = SCHED_IDLE }
> };
>
> -static const struct QemuSeccompSyscall blacklist[] = {
> -/* default set of syscalls to blacklist */
> +static const struct QemuSeccompSyscall denylist[] = {
> +/* default set of syscalls that should get blocked */
> { SCMP_SYS(reboot), QEMU_SECCOMP_SET_DEFAULT },
> { SCMP_SYS(swapon), QEMU_SECCOMP_SET_DEFAULT },
> { SCMP_SYS(swapoff),QEMU_SECCOMP_SET_DEFAULT },
> @@ -175,18 +175,18 @@ static int seccomp_start(uint32_t seccomp_opts, Error
> **errp)
> goto seccomp_return;
> }
>
> -for (i = 0; i < ARRAY_SIZE(blacklist); i++) {
> +for (i = 0; i < ARRAY_SIZE(denylist); i++) {
> uint32_t action;
> -if (!(seccomp_opts & blacklist[i].set)) {
> +if (!(seccomp_opts & denylist[i].set)) {
> continue;
> }
>
> -action = qemu_seccomp_get_action(blacklist[i].set);
> -rc = seccomp_rule_add_array(ctx, action, blacklist[i].num,
> -blacklist[i].narg, blacklist[i].arg_cmp);
> +action = qemu_seccomp_get_action(denylist[i].set);
> +rc = seccomp_rule_add_array(ctx, action, denylist[i].num,
> +denylist[i].narg, denylist[i].arg_cmp);
> if (rc < 0) {
> error_setg_errno(errp, -rc,
> - "failed to add seccomp blacklist rules");
> + "failed to add seccomp denylist rules");
> goto seccomp_return;
> }
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 5/5] tests/fp/fp-test: Replace the word 'blacklist'
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit :
> Follow the inclusive terminology from the "Conscious Language in your
> Open Source Projects" guidelines [*] and replace the word "blacklist"
> appropriately.
>
> [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md
>
> Acked-by: Alex Bennée
> Reviewed-by: Daniel P. Berrangé
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tests/fp/fp-test.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/fp/fp-test.c b/tests/fp/fp-test.c
> index 06ffebd6db1..5a4cad8c8b2 100644
> --- a/tests/fp/fp-test.c
> +++ b/tests/fp/fp-test.c
> @@ -123,7 +123,7 @@ static void not_implemented(void)
> fprintf(stderr, "Not implemented.\n");
> }
>
> -static bool blacklisted(unsigned op, int rmode)
> +static bool is_allowed(unsigned op, int rmode)
> {
> /* odd has not been implemented for any 80-bit ops */
> if (rmode == softfloat_round_odd) {
> @@ -161,10 +161,10 @@ static bool blacklisted(unsigned op, int rmode)
> case F32_TO_EXTF80:
> case F64_TO_EXTF80:
> case F128_TO_EXTF80:
> -return true;
> +return false;
> }
> }
> -return false;
> +return true;
> }
>
> static void do_testfloat(int op, int rmode, bool exact)
> @@ -194,7 +194,7 @@ static void do_testfloat(int op, int rmode, bool exact)
> verCases_writeFunctionName(stderr);
> fputs("\n", stderr);
>
> -if (blacklisted(op, rmode)) {
> +if (!is_allowed(op, rmode)) {
> not_implemented();
> return;
> }
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3 2/5] scripts/tracetool: Replace the word 'whitelist'
Le 03/03/2021 à 19:46, Philippe Mathieu-Daudé a écrit :
> Follow the inclusive terminology from the "Conscious Language in your
> Open Source Projects" guidelines [*] and replace the words "whitelist"
> appropriately.
>
> [*] https://github.com/conscious-lang/conscious-lang-docs/blob/main/faq.md
>
> Reviewed-by: Daniel P. Berrangé
> Reviewed-by: Stefan Hajnoczi
> Reviewed-by: Alex Bennée
> Reviewed-by: Thomas Huth
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> scripts/tracetool/__init__.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
> index 96b1cd69a52..5bc94d95cfc 100644
> --- a/scripts/tracetool/__init__.py
> +++ b/scripts/tracetool/__init__.py
> @@ -100,7 +100,7 @@ def validate_type(name):
> if bit == "const":
> continue
> if bit not in ALLOWED_TYPES:
> -raise ValueError("Argument type '%s' is not in whitelist. "
> +raise ValueError("Argument type '%s' is not allowed. "
> "Only standard C types and fixed size integer "
> "types should be used. struct, union, and "
> "other complex pointer types should be "
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v2] MAINTAINERS: Fix the location of tools manuals
Le 09/03/2021 à 20:48, Thomas Huth a écrit :
> On 09/03/2021 18.41, Wainer dos Santos Moschetta wrote:
>> Hi,
>>
>> Any issue that prevent this of being queued?
>
> Maybe it's just not clear who should take the patch ... CC:-ing qemu-trivial
> and qemu-block now,
> since I think it could go through the trivial or block tree.
>
>> On 2/4/21 10:59 AM, Philippe Mathieu-Daudé wrote:
>>> On 2/4/21 2:54 PM, Wainer dos Santos Moschetta wrote:
The qemu-img.rst, qemu-nbd.rst, virtfs-proxy-helper.rst,
qemu-trace-stap.rst,
and virtiofsd.rst manuals were moved to docs/tools, so this update
MAINTAINERS
accordingly.
Fixes: a08b4a9fe6c ("docs: Move tools documentation to tools manual")
Signed-off-by: Wainer dos Santos Moschetta
---
v1: was "MAINTAINERS: Fix the location of virtiofsd.rst"
v2: Fixed the location of all files [philmd]
MAINTAINERS | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 00626941f1..174425a941 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1829,7 +1829,7 @@ S: Odd Fixes
F: hw/9pfs/
X: hw/9pfs/xen-9p*
F: fsdev/
-F: docs/interop/virtfs-proxy-helper.rst
+F: docs/tools/virtfs-proxy-helper.rst
>
> FWIW:
> Reviewed-by: Thomas Huth
>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH] hw/scsi/megasas: Remove pointless parenthesis
Le 11/10/2020 à 21:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/scsi/megasas.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index e24c12d7eed..d57402c9b09 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2384,8 +2384,8 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
> **errp)
> if (!s->sas_addr) {
> s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
> IEEE_COMPANY_LOCALLY_ASSIGNED) << 36;
> -s->sas_addr |= (pci_dev_bus_num(dev) << 16);
> -s->sas_addr |= (PCI_SLOT(dev->devfn) << 8);
> +s->sas_addr |= pci_dev_bus_num(dev) << 16;
> +s->sas_addr |= PCI_SLOT(dev->devfn) << 8;
> s->sas_addr |= PCI_FUNC(dev->devfn);
> }
> if (!s->hba_serial) {
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH] hw/scsi/megasas: Remove pointless parenthesis
Le 11/10/2020 à 21:50, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/scsi/megasas.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c
> index e24c12d7eed..d57402c9b09 100644
> --- a/hw/scsi/megasas.c
> +++ b/hw/scsi/megasas.c
> @@ -2384,8 +2384,8 @@ static void megasas_scsi_realize(PCIDevice *dev, Error
> **errp)
> if (!s->sas_addr) {
> s->sas_addr = ((NAA_LOCALLY_ASSIGNED_ID << 24) |
> IEEE_COMPANY_LOCALLY_ASSIGNED) << 36;
> -s->sas_addr |= (pci_dev_bus_num(dev) << 16);
> -s->sas_addr |= (PCI_SLOT(dev->devfn) << 8);
> +s->sas_addr |= pci_dev_bus_num(dev) << 16;
> +s->sas_addr |= PCI_SLOT(dev->devfn) << 8;
> s->sas_addr |= PCI_FUNC(dev->devfn);
> }
> if (!s->hba_serial) {
>
Reviewed-by: Laurent Vivier
Re: [PATCH] hw/ide/ahci: Replace fprintf() by qemu_log_mask(GUEST_ERROR)
Le 12/01/2021 à 12:29, Philippe Mathieu-Daudé a écrit :
> Replace fprintf() calls by qemu_log_mask(LOG_GUEST_ERROR).
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> hw/ide/ahci.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 4b675b9cfd8..6d50482b8d1 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -465,8 +465,9 @@ static void ahci_mem_write(void *opaque, hwaddr addr,
>
> /* Only aligned reads are allowed on AHCI */
> if (addr & 3) {
> -fprintf(stderr, "ahci: Mis-aligned write to addr 0x"
> -TARGET_FMT_plx "\n", addr);
> +qemu_log_mask(LOG_GUEST_ERROR,
> + "ahci: Mis-aligned write to addr 0x%03" HWADDR_PRIX
> "\n",
> + addr);
> return;
> }
>
> @@ -,7 +1112,8 @@ static void process_ncq_command(AHCIState *s, int port,
> uint8_t *cmd_fis,
> g_assert(is_ncq(ncq_fis->command));
> if (ncq_tfs->used) {
> /* error - already in use */
> -fprintf(stderr, "%s: tag %d already used\n", __func__, tag);
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: tag %d already used\n",
> + __func__, tag);
> return;
> }
>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PULL for-5.2 2/2] scripts/tracetool: silence SystemTap dtrace(1) long long warnings
On 11/11/2020 16:56, Stefan Hajnoczi wrote:
> SystemTap's dtrace(1) prints the following warning when it encounters
> long long arguments:
>
> Warning: /usr/bin/dtrace:trace/trace-dtrace-hw_virtio.dtrace:76: syntax
> error near:
> probe vhost_vdpa_dev_start
>
> Warning: Proceeding as if --no-pyparsing was given.
>
> Use the uint64_t and int64_t types, respectively. This works with all
> host CPU 32- and 64-bit data models (ILP32, LP64, and LLP64) that QEMU
> supports.
>
> Reported-by: Markus Armbruster
> Signed-off-by: Stefan Hajnoczi
> Reviewed-by: Daniel P. Berrangé
> Reviewed-by: Philippe Mathieu-Daudé
> Message-id: 20201020094043.159935-1-stefa...@redhat.com
> Suggested-by: Daniel P. Berrangé
> Signed-off-by: Stefan Hajnoczi
> ---
> scripts/tracetool/format/d.py | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/scripts/tracetool/format/d.py b/scripts/tracetool/format/d.py
> index 353722f89c..ebfb714200 100644
> --- a/scripts/tracetool/format/d.py
> +++ b/scripts/tracetool/format/d.py
> @@ -57,6 +57,12 @@ def generate(events, backend, group):
> # Avoid it by changing probe type to signed char *
> beforehand.
> if type_ == 'int8_t *':
> type_ = 'signed char *'
> +
> +# SystemTap dtrace(1) emits a warning when long long is used
> +type_ = type_.replace('unsigned long long', 'uint64_t')
> +type_ = type_.replace('signed long long', 'int64_t')
> +type_ = type_.replace('long long', 'int64_t')
> +
> if name in RESERVED_WORDS:
> name += '_'
> args.append(type_ + ' ' + name)
>
This patch fixes the warning with "d" format, but we have the same kind of
problem with
log-stap format:
$ sudo stap -e 'probe begin{printf ("BEGIN")}' -I .
parse error: invalid or missing conversion specifier
saw: operator ',' at ./qemu-system-x86_64-log.stp:15118:101
source: printf("%d@%d vhost_vdpa_set_log_base dev: %p base: 0x%x
size: %llu
refcnt: %d fd: %d log: %p\n", pid(), gettimeofday_ns(), dev, base, size,
refcnt, fd, log)
^
1 parse error.
WARNING: tapset "./qemu-system-x86_64-log.stp" has errors, and will be skipped
BEGIN
This happens because of the "%llu" in the format string.
I'm wondering if we need to fix all the stap based format or simply replace the
"unsigned
long long" by "uint64_t" in hw/virtio/trace-events?
Thanks,
Laurent
Re: [PATCH v2] hw/block/nand: Decommission the NAND museum
Le 13/12/2020 à 20:01, Peter Maydell a écrit :
> On Sun, 13 Dec 2020 at 17:21, Laurent Vivier wrote:
>>
>> Le 16/10/2020 à 18:52, Philippe Mathieu-Daudé a écrit :
>>> Cc'ing qemu-trivial@ since this patch is reviewed.
>>>
>>> On 10/15/20 8:12 PM, Philippe Mathieu-Daudé wrote:
>>>> ping^2...
>>>>
>>>> On 10/1/20 7:31 PM, Philippe Mathieu-Daudé wrote:
>>>>> ping qemu-block or qemu-arm?
>>>>>
>>>>> On 9/15/20 7:16 PM, Philippe Mathieu-Daudé wrote:
>>>>>> This is the QEMU equivalent of this Linux commit (but 7 years later):
>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7025a43a9da2
>>>>>>
>>>>>> The MTD subsystem has its own small museum of ancient NANDs
>>>>>> in a form of the CONFIG_MTD_NAND_MUSEUM_IDS configuration option.
>>>>>> The museum contains stone age NANDs with 256 bytes pages, as well
>>>>>> as iron age NANDs with 512 bytes per page and up to 8MiB page size.
>>>>>>
>>>>>> It is with great sorrow that I inform you that the museum is being
>>>>>> decommissioned. The MTD subsystem is out of budget for Kconfig
>>>>>> options and already has too many of them, and there is a general
>>>>>> kernel trend to simplify the configuration menu.
>>>>>>
>>>>>> We remove the stone age exhibits along with closing the museum,
>>>>>> but some of the iron age ones are transferred to the regular NAND
>>>>>> depot. Namely, only those which have unique device IDs are
>>>>>> transferred, and the ones which have conflicting device IDs are
>>>>>> removed.
>>>>>>
>>>>>> The machine using this device are:
>>>>>> - axis-dev88
>>>>>> - tosa (via tc6393xb_init)
>>>>>> - spitz based (akita, borzoi, terrier)
>>>>>>
>>>>>> Reviewed-by: Richard Henderson
>>>>>> Signed-off-by: Philippe Mathieu-Daudé
>>>>>> ---
>>>>>> Peter, as 4 of the 5 machines are ARM-based, can this go via your tree?
>>>>>> ---
>>>>>> hw/block/nand.c | 13 ++---
>>>>>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>>>>>
>>>>>> diff --git a/hw/block/nand.c b/hw/block/nand.c
>>>>>> index 5c8112ed5a4..5f01ba2bc44 100644
>>>>>> --- a/hw/block/nand.c
>>>>>> +++ b/hw/block/nand.c
>>>>>> @@ -138,7 +138,7 @@ static void mem_and(uint8_t *dest, const uint8_t
>>>>>> *src, size_t n)
>>>>>> # define ADDR_SHIFT16
>>>>>> # include "nand.c"
>>>>>> -/* Information based on Linux drivers/mtd/nand/nand_ids.c */
>>>>>> +/* Information based on Linux drivers/mtd/nand/raw/nand_ids.c */
>>>>>> static const struct {
>>>>>> int size;
>>>>>> int width;
>>>>>> @@ -154,15 +154,14 @@ static const struct {
>>>>>> [0xe8] = { 1,8,8, 4, 0 },
>>>>>> [0xec] = { 1,8,8, 4, 0 },
>>>>>> [0xea] = { 2,8,8, 4, 0 },
>>>>>> -[0xd5] = { 4,8,9, 4, 0 },
>>>>>> [0xe3] = { 4,8,9, 4, 0 },
>>>>>> [0xe5] = { 4,8,9, 4, 0 },
>>>>>> -[0xd6] = { 8,8,9, 4, 0 },
>>>>>> -[0x39] = { 8,8,9, 4, 0 },
>>>>>> -[0xe6] = { 8,8,9, 4, 0 },
>>>>>> -[0x49] = { 8,16,9, 4, NAND_BUSWIDTH_16 },
>>>>>> -[0x59] = { 8,16,9, 4, NAND_BUSWIDTH_16 },
>>>>>> +[0x6b] = { 4,8,9, 4, 0 },
>>>>>> +[0xe3] = { 4,8,9, 4, 0 },
>>>>>> +[0xe5] = { 4,8,9, 4, 0 },
>>>>>> +[0xd6] = { 8,8,9, 4, 0 },
>>>>>> +[0xe6] = { 8,8,9, 4, 0 },
>>>>>> [0x33] = { 16,8,9, 5, 0 },
>>>>>> [0x73] = { 16,8,9, 5, 0 },
>>>>>>
>>>>>
>>>>
>>>
>>
>> Applied to my trivial-patches branch.
>
> Er, I made review comments on this one; it needs a respin.
Yes, removed from my queue.
Thanks,
Laurent
Re: [PATCH v2] hw/block/nand: Decommission the NAND museum
Le 16/10/2020 à 18:52, Philippe Mathieu-Daudé a écrit :
> Cc'ing qemu-trivial@ since this patch is reviewed.
>
> On 10/15/20 8:12 PM, Philippe Mathieu-Daudé wrote:
>> ping^2...
>>
>> On 10/1/20 7:31 PM, Philippe Mathieu-Daudé wrote:
>>> ping qemu-block or qemu-arm?
>>>
>>> On 9/15/20 7:16 PM, Philippe Mathieu-Daudé wrote:
This is the QEMU equivalent of this Linux commit (but 7 years later):
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f7025a43a9da2
The MTD subsystem has its own small museum of ancient NANDs
in a form of the CONFIG_MTD_NAND_MUSEUM_IDS configuration option.
The museum contains stone age NANDs with 256 bytes pages, as well
as iron age NANDs with 512 bytes per page and up to 8MiB page size.
It is with great sorrow that I inform you that the museum is being
decommissioned. The MTD subsystem is out of budget for Kconfig
options and already has too many of them, and there is a general
kernel trend to simplify the configuration menu.
We remove the stone age exhibits along with closing the museum,
but some of the iron age ones are transferred to the regular NAND
depot. Namely, only those which have unique device IDs are
transferred, and the ones which have conflicting device IDs are
removed.
The machine using this device are:
- axis-dev88
- tosa (via tc6393xb_init)
- spitz based (akita, borzoi, terrier)
Reviewed-by: Richard Henderson
Signed-off-by: Philippe Mathieu-Daudé
---
Peter, as 4 of the 5 machines are ARM-based, can this go via your tree?
---
hw/block/nand.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/block/nand.c b/hw/block/nand.c
index 5c8112ed5a4..5f01ba2bc44 100644
--- a/hw/block/nand.c
+++ b/hw/block/nand.c
@@ -138,7 +138,7 @@ static void mem_and(uint8_t *dest, const uint8_t *src,
size_t n)
# define ADDR_SHIFT 16
# include "nand.c"
-/* Information based on Linux drivers/mtd/nand/nand_ids.c */
+/* Information based on Linux drivers/mtd/nand/raw/nand_ids.c */
static const struct {
int size;
int width;
@@ -154,15 +154,14 @@ static const struct {
[0xe8] = { 1, 8, 8, 4, 0 },
[0xec] = { 1, 8, 8, 4, 0 },
[0xea] = { 2, 8, 8, 4, 0 },
- [0xd5] = { 4, 8, 9, 4, 0 },
[0xe3] = { 4, 8, 9, 4, 0 },
[0xe5] = { 4, 8, 9, 4, 0 },
- [0xd6] = { 8, 8, 9, 4, 0 },
- [0x39] = { 8, 8, 9, 4, 0 },
- [0xe6] = { 8, 8, 9, 4, 0 },
- [0x49] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
- [0x59] = { 8, 16, 9, 4, NAND_BUSWIDTH_16 },
+ [0x6b] = { 4, 8, 9, 4, 0 },
+ [0xe3] = { 4, 8, 9, 4, 0 },
+ [0xe5] = { 4, 8, 9, 4, 0 },
+ [0xd6] = { 8, 8, 9, 4, 0 },
+ [0xe6] = { 8, 8, 9, 4, 0 },
[0x33] = { 16, 8, 9, 5, 0 },
[0x73] = { 16, 8, 9, 5, 0 },
>>>
>>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH 1/4] qdev: Fix two typos
Le 24/10/2020 à 07:34, Thomas Huth a écrit :
> On 19/10/2020 18.36, Maxim Levitsky wrote:
>> Signed-off-by: Maxim Levitsky
>> ---
>> include/hw/qdev-core.h | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 868973319e..3761186804 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -163,8 +163,8 @@ struct NamedClockList {
>> /**
>> * DeviceState:
>> * @realized: Indicates whether the device has been fully constructed.
>> - *When accessed outsize big qemu lock, must be accessed with
>> - *atomic_load_acquire()
>> + *When accessed outside big qemu lock, must be accessed with
>> + *qatomic_load_acquire()
>> * @reset: ResettableState for the device; handled by Resettable interface.
>> *
>> * This structure should not be accessed directly. We declare it here
>>
>
> Reviewed-by: Thomas Huth
>
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH v3] migration/block-dirty-bitmap: fix uninitialized variable warning
Le 20/10/2020 à 09:36, Chenqun (kuhn) a écrit : > Ping! > > Hello, > > Maybe this patch, some people have any other suggestions? Or, maybe > missed to queue? Hi, As we've seen by reviewing previous versions, this change is not really trivial, so I will not take it via the trivial queue. Perhaps via block I/O queue (Stefan?), Dirty Bitmaps (John?) or Migration (Juan?). Thanks, Laurent > Thanks, > Chen Qun > >> -Original Message- >> From: Vladimir Sementsov-Ogievskiy [mailto:vsement...@virtuozzo.com] >> Sent: Wednesday, October 14, 2020 11:56 PM >> To: Chenqun (kuhn) ; qemu-de...@nongnu.org; >> qemu-triv...@nongnu.org >> Cc: mre...@redhat.com; stefa...@redhat.com; f...@euphon.net; >> ebl...@redhat.com; js...@redhat.com; quint...@redhat.com; >> dgilb...@redhat.com; Zhanghailiang ; >> ganqixin ; qemu-block@nongnu.org; Euler Robot >> ; Laurent Vivier ; Li Qiang >> >> Subject: Re: [PATCH v3] migration/block-dirty-bitmap: fix uninitialized >> variable >> warning >> >> 14.10.2020 14:44, Chen Qun wrote: >>> A default value is provided for the variable 'bitmap_name' to avoid compiler >> warning. >>> >>> The compiler show warning: >>> migration/block-dirty-bitmap.c:1090:13: warning: ‘bitmap_name’ >>> may be used uninitialized in this function [-Wmaybe-uninitialized] >>> g_strlcpy(s->bitmap_name, bitmap_name, >> sizeof(s->bitmap_name)); >>> >> ^~ >>> >>> Reported-by: Euler Robot >>> Signed-off-by: Chen Qun >> >> Reviewed-by: Vladimir Sementsov-Ogievskiy >> >> -- >> Best regards, >> Vladimir
Re: [PATCH] migration/block-dirty-bitmap: fix uninitialized variable warning
Le 13/10/2020 à 03:34, Li Qiang a écrit :
> Laurent Vivier 于2020年10月12日周一 下午11:33写道：
>>
>> Le 10/10/2020 à 13:07, Chen Qun a écrit :
>>> This if statement judgment is redundant and it will cause a warning:
>>>
>>> migration/block-dirty-bitmap.c:1090:13: warning: ‘bitmap_name’ may be used
>>> uninitialized in this function [-Wmaybe-uninitialized]
>>> g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
>>> ^~
>>>
>>> Reported-by: Euler Robot
>>> Signed-off-by: Chen Qun
>>> ---
>>> migration/block-dirty-bitmap.c | 2 --
>>> 1 file changed, 2 deletions(-)
>>>
>>> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
>>> index 5bef793ac0..e09ea4f22b 100644
>>> --- a/migration/block-dirty-bitmap.c
>>> +++ b/migration/block-dirty-bitmap.c
>>> @@ -1084,9 +1084,7 @@ static int dirty_bitmap_load_header(QEMUFile *f,
>>> DBMLoadState *s,
>>> } else {
>>> bitmap_name = s->bitmap_alias;
>>> }
>>> -}
>>>
>>> -if (!s->cancelled) {
>>> g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
>>> s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
>>>
>>>
>>
>> I don't think it's correct as "cancel_incoming_locked(s)" can change the
>> value of "s->cancelled".
>>
>
> Hi Laurent,
>
> You're right. So I think this can simply assign 'bitmap_name' to NULL
> to make compiler happy.
Yes, and adding a comment before the second "if (!s->cancelled) {" to
explain the value can be changed by "cancel_incoming_locked(s)" would
avoid to have this kind of patch posted regularly to the ML.
Thanks,
Laurent
Re: [PATCH] migration/block-dirty-bitmap: fix uninitialized variable warning
Le 10/10/2020 à 13:07, Chen Qun a écrit :
> This if statement judgment is redundant and it will cause a warning:
>
> migration/block-dirty-bitmap.c:1090:13: warning: ‘bitmap_name’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
> ^~
>
> Reported-by: Euler Robot
> Signed-off-by: Chen Qun
> ---
> migration/block-dirty-bitmap.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
> index 5bef793ac0..e09ea4f22b 100644
> --- a/migration/block-dirty-bitmap.c
> +++ b/migration/block-dirty-bitmap.c
> @@ -1084,9 +1084,7 @@ static int dirty_bitmap_load_header(QEMUFile *f,
> DBMLoadState *s,
> } else {
> bitmap_name = s->bitmap_alias;
> }
> -}
>
> -if (!s->cancelled) {
> g_strlcpy(s->bitmap_name, bitmap_name, sizeof(s->bitmap_name));
> s->bitmap = bdrv_find_dirty_bitmap(s->bs, s->bitmap_name);
>
>
I don't think it's correct as "cancel_incoming_locked(s)" can change the
value of "s->cancelled".
Thanks,
Laurent
Re: [PATCH] block/blkdebug: fix memory leak
Le 09/10/2020 à 21:09, Elena Afanasova a écrit :
> Spotted by PVS-Studio
>
> Signed-off-by: Elena Afanasova
> ---
> block/blkdebug.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index eecbf3e5c4..54da719dd1 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -215,6 +215,7 @@ static int add_rule(void *opaque, QemuOpts *opts, Error
> **errp)
> BLKDEBUG_IO_TYPE__MAX, _error);
> if (local_error) {
> error_propagate(errp, local_error);
> +g_free(rule);
> return -1;
> }
> if (iotype != BLKDEBUG_IO_TYPE__MAX) {
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH 1/4] vmdk: fix maybe uninitialized warnings
Le 05/10/2020 à 08:26, Christian Borntraeger a écrit :
> On 30.09.20 18:36, Fam Zheng wrote:
>> On Wed, 2020-09-30 at 17:58 +0200, Christian Borntraeger wrote:
>>> Fedora 32 gcc 10 seems to give false positives:
>>>
>>> Compiling C object libblock.fa.p/block_vmdk.c.o
>>> ../block/vmdk.c: In function ‘vmdk_parse_extents’:
>>> ../block/vmdk.c:587:5: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> 587 | g_free(extent->l1_table);
>>> | ^~~~
>>> ../block/vmdk.c:754:17: note: ‘extent’ was declared here
>>> 754 | VmdkExtent *extent;
>>> | ^~
>>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> 620 | ret = vmdk_init_tables(bs, extent, errp);
>>> | ^~
>>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>> 598 | VmdkExtent *extent;
>>> | ^~
>>> ../block/vmdk.c:1178:39: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> 1178 | extent->flat_start_offset = flat_offset << 9;
>>> | ~~^~
>>> ../block/vmdk.c: In function ‘vmdk_open_vmdk4’:
>>> ../block/vmdk.c:581:22: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> 581 | extent->l2_cache =
>>> | ~^
>>> 582 | g_malloc(extent->entry_size * extent->l2_size *
>>> L2_CACHE_SIZE);
>>> | ~
>>> ~
>>> ../block/vmdk.c:872:17: note: ‘extent’ was declared here
>>> 872 | VmdkExtent *extent;
>>> | ^~
>>> ../block/vmdk.c: In function ‘vmdk_open’:
>>> ../block/vmdk.c:620:11: error: ‘extent’ may be used uninitialized in
>>> this function [-Werror=maybe-uninitialized]
>>> 620 | ret = vmdk_init_tables(bs, extent, errp);
>>> | ^~
>>> ../block/vmdk.c:598:17: note: ‘extent’ was declared here
>>> 598 | VmdkExtent *extent;
>>> | ^~
>>> cc1: all warnings being treated as errors
>>> make: *** [Makefile.ninja:884: libblock.fa.p/block_vmdk.c.o] Error 1
>>>
>>> fix them by assigning a default value.
>>>
>>> Signed-off-by: Christian Borntraeger
>>> ---
>>> block/vmdk.c | 8
>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/block/vmdk.c b/block/vmdk.c
>>> index 8ec62c7ab798..a00dc00eb47a 100644
>>> --- a/block/vmdk.c
>>> +++ b/block/vmdk.c
>>> @@ -595,7 +595,7 @@ static int vmdk_open_vmfs_sparse(BlockDriverState
>>> *bs,
>>> int ret;
>>> uint32_t magic;
>>> VMDK3Header header;
>>> -VmdkExtent *extent;
>>> +VmdkExtent *extent = NULL;
>>>
>>> ret = bdrv_pread(file, sizeof(magic), , sizeof(header));
>>> if (ret < 0) {
>>> @@ -751,7 +751,7 @@ static int vmdk_open_se_sparse(BlockDriverState
>>> *bs,
>>> int ret;
>>> VMDKSESparseConstHeader const_header;
>>> VMDKSESparseVolatileHeader volatile_header;
>>> -VmdkExtent *extent;
>>> +VmdkExtent *extent = NULL;
>>>
>>> ret = bdrv_apply_auto_read_only(bs,
>>> "No write support for seSparse images available", errp);
>>> @@ -869,7 +869,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
>>> uint32_t magic;
>>> uint32_t l1_size, l1_entry_sectors;
>>> VMDK4Header header;
>>> -VmdkExtent *extent;
>>> +VmdkExtent *extent = NULL;
>>> BDRVVmdkState *s = bs->opaque;
>>> int64_t l1_backup_offset = 0;
>>> bool compressed;
>>> @@ -1088,7 +1088,7 @@ static int vmdk_parse_extents(const char *desc,
>>> BlockDriverState *bs,
>>> BdrvChild *extent_file;
>>> BdrvChildRole extent_role;
>>> BDRVVmdkState *s = bs->opaque;
>>> -VmdkExtent *extent;
>>> +VmdkExtent *extent = NULL;
>>> char extent_opt_prefix[32];
>>> Error *local_err = NULL;
>>>
>>
>> Looks trivial, and correct.
>>
>> Reviewed-by: Fam Zheng
>
>
> Will this go via the block or trivial tree (cced).
>
Applied to my trivial-patches branch.
Thanks,
Laurent
Re: [PATCH] hw/block/nvme: Simplify timestamp sum
Le 02/10/2020 à 09:57, Philippe Mathieu-Daudé a écrit : > As the 'timestamp' variable is declared as a 48-bit bitfield, > we do not need to wrap the sum result. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/block/nvme.c | 7 +-- > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/hw/block/nvme.c b/hw/block/nvme.c > index 63078f6009..44fa5b9076 100644 > --- a/hw/block/nvme.c > +++ b/hw/block/nvme.c > @@ -1280,12 +1280,7 @@ static inline uint64_t nvme_get_timestamp(const > NvmeCtrl *n) > > union nvme_timestamp ts; > ts.all = 0; > - > -/* > - * If the sum of the Timestamp value set by the host and the elapsed > - * time exceeds 2^48, the value returned should be reduced modulo 2^48. > - */ > -ts.timestamp = (n->host_timestamp + elapsed_time) & 0x; > +ts.timestamp = n->host_timestamp + elapsed_time; > > /* If the host timestamp is non-zero, set the timestamp origin */ > ts.origin = n->host_timestamp ? 0x01 : 0x00; > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH v10 13/26] meson: remove empty else and duplicated gio deps
Le 15/09/2020 à 19:12, Yonggang Luo a écrit : > Signed-off-by: Yonggang Luo > Reviewed-by: Daniel P. Berrangé > --- > meson.build | 6 -- > 1 file changed, 6 deletions(-) > > diff --git a/meson.build b/meson.build > index 690723b470..23cb1b8742 100644 > --- a/meson.build > +++ b/meson.build > @@ -317,7 +317,6 @@ opengl = not_found > if 'CONFIG_OPENGL' in config_host >opengl = declare_dependency(compile_args: > config_host['OPENGL_CFLAGS'].split(), >link_args: config_host['OPENGL_LIBS'].split()) > -else > endif > gtk = not_found > if 'CONFIG_GTK' in config_host > @@ -344,11 +343,6 @@ if 'CONFIG_ICONV' in config_host >iconv = declare_dependency(compile_args: > config_host['ICONV_CFLAGS'].split(), > link_args: config_host['ICONV_LIBS'].split()) > endif > -gio = not_found > -if 'CONFIG_GIO' in config_host > - gio = declare_dependency(compile_args: config_host['GIO_CFLAGS'].split(), > - link_args: config_host['GIO_LIBS'].split()) > -endif > vnc = not_found > png = not_found > jpeg = not_found > Applied to my trivial-patches branch. Thanks, Laurent
Re: [PATCH v2 1/2] util/hexdump: Convert to take a void pointer argument
Le 22/08/2020 à 20:09, Philippe Mathieu-Daudé a écrit :
> Most uses of qemu_hexdump() do not take an array of char
> as input, forcing use of cast. Since we can use this
> helper to dump any kind of buffer, use a pointer to void
> argument instead.
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> Since v1:
> - renamed argument 'bufptr' (Peter Maydell)
> ---
> include/qemu-common.h| 3 ++-
> hw/dma/xlnx_dpdma.c | 2 +-
> hw/net/fsl_etsec/etsec.c | 2 +-
> hw/sd/sd.c | 2 +-
> hw/usb/redirect.c| 2 +-
> net/colo-compare.c | 12 ++--
> net/net.c| 2 +-
> util/hexdump.c | 4 +++-
> 8 files changed, 16 insertions(+), 13 deletions(-)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index bb9496bd80f..6834883822f 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -138,7 +138,8 @@ int os_parse_cmd_args(int index, const char *optarg);
> * Hexdump a buffer to a file. An optional string prefix is added to every
> line
> */
>
> -void qemu_hexdump(const char *buf, FILE *fp, const char *prefix, size_t
> size);
> +void qemu_hexdump(const void *bufptr, FILE *fp,
> + const char *prefix, size_t size);
>
> /*
> * helper to parse debug environment variables
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index b40c897de2c..d75a8069426 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -388,7 +388,7 @@ static void xlnx_dpdma_dump_descriptor(DPDMADescriptor
> *desc)
> {
> if (DEBUG_DPDMA) {
> qemu_log("DUMP DESCRIPTOR:\n");
> -qemu_hexdump((char *)desc, stdout, "", sizeof(DPDMADescriptor));
> +qemu_hexdump(desc, stdout, "", sizeof(DPDMADescriptor));
> }
> }
>
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index 7035cf4eb97..c817a28decd 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -357,7 +357,7 @@ static ssize_t etsec_receive(NetClientState *nc,
>
> #if defined(HEX_DUMP)
> fprintf(stderr, "%s receive size:%zd\n", nc->name, size);
> -qemu_hexdump((void *)buf, stderr, "", size);
> +qemu_hexdump(buf, stderr, "", size);
> #endif
> /* Flush is unnecessary as are already in receiving path */
> etsec->need_flush = false;
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index fad9cf1ee7a..190e4cf1232 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1781,7 +1781,7 @@ send_response:
> }
>
> #ifdef DEBUG_SD
> -qemu_hexdump((const char *)response, stderr, "Response", rsplen);
> +qemu_hexdump(response, stderr, "Response", rsplen);
> #endif
>
> return rsplen;
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 417a60a2e68..09edb0d81c0 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -240,7 +240,7 @@ static void usbredir_log_data(USBRedirDevice *dev, const
> char *desc,
> if (dev->debug < usbredirparser_debug_data) {
> return;
> }
> -qemu_hexdump((char *)data, stderr, desc, len);
> +qemu_hexdump(data, stderr, desc, len);
> }
>
> /*
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 2c20de1537d..550272b3baa 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -494,9 +494,9 @@ sec:
> g_queue_push_head(>secondary_list, spkt);
>
> if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump((char *)ppkt->data, stderr,
> +qemu_hexdump(ppkt->data, stderr,
> "colo-compare ppkt", ppkt->size);
> -qemu_hexdump((char *)spkt->data, stderr,
> +qemu_hexdump(spkt->data, stderr,
> "colo-compare spkt", spkt->size);
> }
>
> @@ -535,9 +535,9 @@ static int colo_packet_compare_udp(Packet *spkt, Packet
> *ppkt)
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
> if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
> +qemu_hexdump(ppkt->data, stderr, "colo-compare pri pkt",
> ppkt->size);
> -qemu_hexdump((char *)spkt->data, stderr, "colo-compare sec pkt",
> +qemu_hexdump(spkt->data, stderr, "colo-compare sec pkt",
> spkt->size);
> }
> return -1;
> @@ -578,9 +578,9 @@ static int colo_packet_compare_icmp(Packet *spkt, Packet
> *ppkt)
> trace_colo_compare_icmp_miscompare("Secondary pkt size",
> spkt->size);
> if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump((char *)ppkt->data, stderr, "colo-compare pri pkt",
> +qemu_hexdump(ppkt->data, stderr, "colo-compare pri pkt",
>
Re: [PATCH v2 0/2] util/hexdump: Cleanup qemu_hexdump()
Le 11/09/2020 à 16:35, Philippe Mathieu-Daudé a écrit : > Hi Laurent, > > On 8/22/20 8:09 PM, Philippe Mathieu-Daudé wrote: >> - Pass const void* buffer >> - Reorder arguments >> >> Supersedes: <20200822150457.1322519-1-f4...@amsat.org> >> >> Philippe Mathieu-Daudé (2): >> util/hexdump: Convert to take a void pointer argument >> util/hexdump: Reorder qemu_hexdump() arguments > > This series is fully reviewed, can it go via > your qemu-trivial tree? Done. Thanks, Laurent
Re: [PATCH v2 2/2] util/hexdump: Reorder qemu_hexdump() arguments
Le 22/08/2020 à 20:09, Philippe Mathieu-Daudé a écrit :
> qemu_hexdump()'s pointer to the buffer and length of the
> buffer are closely related arguments but are widely separated
> in the argument list order (also, the format of
> function prototypes is usually to have the FILE* argument
> coming first).
>
> Reorder the arguments as "fp, prefix, buf, size" which is
> more logical.
>
> Suggested-by: Peter Maydell
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> include/qemu-common.h| 4 ++--
> hw/dma/xlnx_dpdma.c | 2 +-
> hw/net/fsl_etsec/etsec.c | 2 +-
> hw/net/fsl_etsec/rings.c | 2 +-
> hw/sd/sd.c | 2 +-
> hw/usb/redirect.c| 2 +-
> net/colo-compare.c | 24
> net/net.c| 2 +-
> util/hexdump.c | 4 ++--
> util/iov.c | 2 +-
> 10 files changed, 23 insertions(+), 23 deletions(-)
>
> diff --git a/include/qemu-common.h b/include/qemu-common.h
> index 6834883822f..9cfd62669bf 100644
> --- a/include/qemu-common.h
> +++ b/include/qemu-common.h
> @@ -138,8 +138,8 @@ int os_parse_cmd_args(int index, const char *optarg);
> * Hexdump a buffer to a file. An optional string prefix is added to every
> line
> */
>
> -void qemu_hexdump(const void *bufptr, FILE *fp,
> - const char *prefix, size_t size);
> +void qemu_hexdump(FILE *fp, const char *prefix,
> + const void *bufptr, size_t size);
>
> /*
> * helper to parse debug environment variables
> diff --git a/hw/dma/xlnx_dpdma.c b/hw/dma/xlnx_dpdma.c
> index d75a8069426..967548abd31 100644
> --- a/hw/dma/xlnx_dpdma.c
> +++ b/hw/dma/xlnx_dpdma.c
> @@ -388,7 +388,7 @@ static void xlnx_dpdma_dump_descriptor(DPDMADescriptor
> *desc)
> {
> if (DEBUG_DPDMA) {
> qemu_log("DUMP DESCRIPTOR:\n");
> -qemu_hexdump(desc, stdout, "", sizeof(DPDMADescriptor));
> +qemu_hexdump(stdout, "", desc, sizeof(DPDMADescriptor));
> }
> }
>
> diff --git a/hw/net/fsl_etsec/etsec.c b/hw/net/fsl_etsec/etsec.c
> index c817a28decd..c5edb25dc9f 100644
> --- a/hw/net/fsl_etsec/etsec.c
> +++ b/hw/net/fsl_etsec/etsec.c
> @@ -357,7 +357,7 @@ static ssize_t etsec_receive(NetClientState *nc,
>
> #if defined(HEX_DUMP)
> fprintf(stderr, "%s receive size:%zd\n", nc->name, size);
> -qemu_hexdump(buf, stderr, "", size);
> +qemu_hexdump(stderr, "", buf, size);
> #endif
> /* Flush is unnecessary as are already in receiving path */
> etsec->need_flush = false;
> diff --git a/hw/net/fsl_etsec/rings.c b/hw/net/fsl_etsec/rings.c
> index 337a55fc957..628648a9c37 100644
> --- a/hw/net/fsl_etsec/rings.c
> +++ b/hw/net/fsl_etsec/rings.c
> @@ -269,7 +269,7 @@ static void process_tx_bd(eTSEC *etsec,
>
> #if defined(HEX_DUMP)
> qemu_log("eTSEC Send packet size:%d\n", etsec->tx_buffer_len);
> -qemu_hexdump(etsec->tx_buffer, stderr, "", etsec->tx_buffer_len);
> +qemu_hexdump(stderr, "", etsec->tx_buffer, etsec->tx_buffer_len);
> #endif /* ETSEC_RING_DEBUG */
>
> if (etsec->first_bd.flags & BD_TX_TOEUN) {
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 190e4cf1232..6508939b1f4 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1781,7 +1781,7 @@ send_response:
> }
>
> #ifdef DEBUG_SD
> -qemu_hexdump(response, stderr, "Response", rsplen);
> +qemu_hexdump(stderr, "Response", response, rsplen);
> #endif
>
> return rsplen;
> diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
> index 09edb0d81c0..48f38d33912 100644
> --- a/hw/usb/redirect.c
> +++ b/hw/usb/redirect.c
> @@ -240,7 +240,7 @@ static void usbredir_log_data(USBRedirDevice *dev, const
> char *desc,
> if (dev->debug < usbredirparser_debug_data) {
> return;
> }
> -qemu_hexdump(data, stderr, desc, len);
> +qemu_hexdump(stderr, desc, data, len);
> }
>
> /*
> diff --git a/net/colo-compare.c b/net/colo-compare.c
> index 550272b3baa..4a5ed642e9a 100644
> --- a/net/colo-compare.c
> +++ b/net/colo-compare.c
> @@ -494,10 +494,10 @@ sec:
> g_queue_push_head(>secondary_list, spkt);
>
> if (trace_event_get_state_backends(TRACE_COLO_COMPARE_MISCOMPARE)) {
> -qemu_hexdump(ppkt->data, stderr,
> -"colo-compare ppkt", ppkt->size);
> -qemu_hexdump(spkt->data, stderr,
> -"colo-compare spkt", spkt->size);
> +qemu_hexdump(stderr, "colo-compare ppkt",
> + ppkt->data, ppkt->size);
> +qemu_hexdump(stderr, "colo-compare spkt",
> + spkt->data, spkt->size);
> }
>
> colo_compare_inconsistency_notify(s);
> @@ -535,10 +535,10 @@ static int colo_packet_compare_udp(Packet *spkt, Packet
> *ppkt)
> trace_colo_compare_udp_miscompare("primary pkt size", ppkt->size);
> trace_colo_compare_udp_miscompare("Secondary pkt size", spkt->size);
> if
Re: [PATCH 0/7] block: Use definitions instead of magic values
Le 14/08/2020 à 10:28, Philippe Mathieu-Daudé a écrit : > Trivial block patches: > - Fix a typo > - Replace '1 << 30' by '1 * GiB' in null-co > - Replace 512 by BDRV_SECTOR_SIZE when appropriate. > > Philippe Mathieu-Daudé (7): > block/null: Make more explicit the driver default size is 1GiB > hw/ide/core: Trivial typo fix > hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SIZE > hw/ide/ahci: Replace magic '512' value by BDRV_SECTOR_SIZE > hw/ide/atapi: Replace magic '512' value by BDRV_SECTOR_SIZE > hw/ide/pci: Replace magic '512' value by BDRV_SECTOR_SIZE > hw/scsi/scsi-disk: Replace magic '512' value by BDRV_SECTOR_SIZE > > block/null.c| 4 +++- > hw/ide/ahci.c | 5 +++-- > hw/ide/atapi.c | 8 > hw/ide/core.c | 25 + > hw/ide/pci.c| 2 +- > hw/scsi/scsi-disk.c | 44 +++- > 6 files changed, 47 insertions(+), 41 deletions(-) > Applied to my trivial-patches branch. Except the following ones that have comment from Kevin: [PATCH 1/7] block/null: Make more explicit the driver default size is 1GiB [PATCH 3/7] hw/ide/core: Replace magic '512' value by BDRV_SECTOR_SIZE Thanks, Laurent
