Re: [PATCH 12/12] backends/tpm: Use qemu_hexdump_line() to avoid sprintf()
On 4/10/24 12:06, Philippe Mathieu-Daudé wrote:
sprintf() is deprecated on Darwin since macOS 13.0 / XCode 14.1,
resulting in painful developper experience.
Use qemu_hexdump_line() to avoid sprintf() calls, silencing:
backends/tpm/tpm_util.c:357:14: warning: 'sprintf' is deprecated:
This function is provided for compatibility reasons only.
Due to security concerns inherent in the design of sprintf(3),
it is highly recommended that you use snprintf(3) instead.
[-Wdeprecated-declarations]
p += sprintf(p, "%.2X ", buffer[i]);
^ >
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Berger
---
backends/tpm/tpm_util.c | 24
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 1856589c3b..0747af2d1c 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "qemu/error-report.h"
+#include "qemu/cutils.h"
#include "qapi/error.h"
#include "qapi/visitor.h"
#include "tpm_int.h"
@@ -336,27 +337,18 @@ void tpm_sized_buffer_reset(TPMSizedBuffer *tsb)
void tpm_util_show_buffer(const unsigned char *buffer,
size_t buffer_size, const char *string)
{
-size_t len, i;
-char *line_buffer, *p;
+size_t len;
+char *line, *lineup;
if (!trace_event_get_state_backends(TRACE_TPM_UTIL_SHOW_BUFFER)) {
return;
}
len = MIN(tpm_cmd_get_size(buffer), buffer_size);
-/*
- * allocate enough room for 3 chars per buffer entry plus a
- * newline after every 16 chars and a final null terminator.
- */
-line_buffer = g_malloc(len * 3 + (len / 16) + 1);
+line = qemu_hexdump_line(buffer, 0, len, false);
+lineup = g_ascii_strup(line, -1);
+trace_tpm_util_show_buffer(string, len, lineup);
-for (i = 0, p = line_buffer; i < len; i++) {
-if (i && !(i % 16)) {
-p += sprintf(p, "\n");
-}
-p += sprintf(p, "%.2X ", buffer[i]);
-}
-trace_tpm_util_show_buffer(string, len, line_buffer);
-
-g_free(line_buffer);
+g_free(line);
+g_free(lineup);
}
Re: [PATCH v2 10/13] migration: Improve example and documentation of vmstate_register()
On 10/20/23 05:07, Juan Quintela wrote: Signed-off-by: Juan Quintela Reviewed-by: Stefan Berger --- docs/devel/migration.rst | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index c3e1400c0c..bfd8710c95 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -165,13 +165,17 @@ An example (from hw/input/pckbd.c) } }; -We are declaring the state with name "pckbd". -The ``version_id`` is 3, and the fields are 4 uint8_t in a KBDState structure. -We registered this with: +We are declaring the state with name "pckbd". The ``version_id`` is +3, and there are 4 uint8_t fields in the KBDState structure. We +registered this ``VMSTATEDescription`` with one of the following +functions. The first one will generate a device ``instance_id`` +different for each registration. Use the second one if you already +have an id that is different for each instance of the device: .. code:: c -vmstate_register(NULL, 0, _kbd, s); +vmstate_register_any(NULL, _kbd, s); +vmstate_register(NULL, instance_id, _kbd, s); For devices that are ``qdev`` based, we can register the device in the class init function:
Re: [PATCH 10/13] migration: Improve example and documentation of vmstate_register()
On 10/19/23 15:08, Juan Quintela wrote: Signed-off-by: Juan Quintela --- docs/devel/migration.rst | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst index c3e1400c0c..a9fde75862 100644 --- a/docs/devel/migration.rst +++ b/docs/devel/migration.rst @@ -165,13 +165,17 @@ An example (from hw/input/pckbd.c) } }; -We are declaring the state with name "pckbd". -The ``version_id`` is 3, and the fields are 4 uint8_t in a KBDState structure. -We registered this with: +We are declaring the state with name "pckbd". The ``version_id`` is +3, and the fields are 4 uint8_t in a KBDState structure. We and there are 4 uint8_t fields in the KBDState structure. +registered this with one of those. The first one will generate a I am not sure what this means 'We registered this with one of those'. What is 'one of those'? Maybe you mean: We register the KBDState with one of the following functions. +device ``instance_id`` different for each registration. Use the +second one if you already have an id different for each instance of +the device: ... have an id that is is different for each ... .. code:: c -vmstate_register(NULL, 0, _kbd, s); +vmstate_register_any(NULL, _kbd, s); +vmstate_register(NULL, instance_id, _kbd, s); For devices that are ``qdev`` based, we can register the device in the class init function:
Re: [PATCH 13/13] migration: Use vmstate_register_any() for vmware_vga
On 10/19/23 15:08, Juan Quintela wrote: I have no idea if we can have more than one vmware_vga device, so play it safe. Signed-off-by: Juan Quintela Reviewed-by: Stefan Berger --- hw/display/vmware_vga.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 09591fbd39..7490d43881 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -1264,7 +1264,7 @@ static void vmsvga_init(DeviceState *dev, struct vmsvga_state_s *s, vga_common_init(>vga, OBJECT(dev), _fatal); vga_init(>vga, OBJECT(dev), address_space, io, true); -vmstate_register(NULL, 0, _vga_common, >vga); +vmstate_register_any(NULL, _vga_common, >vga); And the first one registered with 'any' will again have instance_id = 0 assigned. So there's no side effect to be expected with any of these device, I suppose. s->new_depth = 32; }
Re: [PATCH 12/13] migration: Use vmstate_register_any() for eeprom93xx
On 10/19/23 15:08, Juan Quintela wrote:
We can have more than one eeprom93xx.
For instance:
e100_nic_realize() -> eeprom93xx_new()
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
hw/nvram/eeprom93xx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/nvram/eeprom93xx.c b/hw/nvram/eeprom93xx.c
index 1081e2cc0d..57d63638d7 100644
--- a/hw/nvram/eeprom93xx.c
+++ b/hw/nvram/eeprom93xx.c
@@ -321,7 +321,7 @@ eeprom_t *eeprom93xx_new(DeviceState *dev, uint16_t nwords)
/* Output DO is tristate, read results in 1. */
eeprom->eedo = 1;
logout("eeprom = 0x%p, nwords = %u\n", eeprom, nwords);
-vmstate_register(VMSTATE_IF(dev), 0, _eeprom, eeprom);
+vmstate_register_any(VMSTATE_IF(dev), _eeprom, eeprom);
return eeprom;
}
Re: [PATCH 11/13] migration: Use vmstate_register_any() for audio
On 10/19/23 15:08, Juan Quintela wrote:
We can have more than one audio card.
void audio_init_audiodevs(void)
{
AudiodevListEntry *e;
QSIMPLEQ_FOREACH(e, , next) {
audio_init(e->dev, _fatal);
}
}
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
audio/audio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/audio/audio.c b/audio/audio.c
index e9815d6812..f91e05b72c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1781,7 +1781,7 @@ static AudioState *audio_init(Audiodev *dev, Error **errp)
QTAILQ_INSERT_TAIL(_states, s, list);
QLIST_INIT (>card_head);
-vmstate_register (NULL, 0, _audio, s);
+vmstate_register_any(NULL, _audio, s);
return s;
out:
Re: [PATCH 06/13] migration: Use VMSTATE_INSTANCE_ID_ANY for s390 devices
On 10/19/23 15:08, Juan Quintela wrote:
Just with make check I can see that we can have more than one of this
devices, so use ANY.
ok 5 /s390x/device/introspect/abstract-interfaces
...
Broken pipe
../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried
to terminate QEMU process but encountered exit status 1 (expected 0)
Aborted (core dumped)
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
hw/s390x/s390-skeys.c| 3 ++-
hw/s390x/s390-stattrib.c | 3 ++-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c
index 5024faf411..ef089e1967 100644
--- a/hw/s390x/s390-skeys.c
+++ b/hw/s390x/s390-skeys.c
@@ -22,6 +22,7 @@
#include "sysemu/kvm.h"
#include "migration/qemu-file-types.h"
#include "migration/register.h"
+#include "migration/vmstate.h"
#define S390_SKEYS_BUFFER_SIZE (128 * KiB) /* Room for 128k storage keys */
#define S390_SKEYS_SAVE_FLAG_EOS 0x01
@@ -457,7 +458,7 @@ static inline void s390_skeys_set_migration_enabled(Object
*obj, bool value,
ss->migration_enabled = value;
if (ss->migration_enabled) {
-register_savevm_live(TYPE_S390_SKEYS, 0, 1,
+register_savevm_live(TYPE_S390_SKEYS, VMSTATE_INSTANCE_ID_ANY, 1,
_s390_storage_keys, ss);
} else {
unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss);
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index 220e845d12..055d382c3c 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -13,6 +13,7 @@
#include "qemu/units.h"
#include "migration/qemu-file.h"
#include "migration/register.h"
+#include "migration/vmstate.h"
#include "hw/s390x/storage-attributes.h"
#include "qemu/error-report.h"
#include "exec/ram_addr.h"
@@ -380,7 +381,7 @@ static void s390_stattrib_instance_init(Object *obj)
{
S390StAttribState *sas = S390_STATTRIB(obj);
-register_savevm_live(TYPE_S390_STATTRIB, 0, 0,
+register_savevm_live(TYPE_S390_STATTRIB, VMSTATE_INSTANCE_ID_ANY, 0,
_s390_stattrib_handlers, sas);
object_property_add_bool(obj, "migration-enabled",
Re: [PATCH 05/13] migration: Use VMSTATE_INSTANCE_ID_ANY for slirp
On 10/19/23 15:08, Juan Quintela wrote:
Each user network conection create a new slirp instance. We register
more than one slirp instance for number 0.
qemu-system-x86_64: -netdev user,id=hs1: savevm_state_handler_insert: Detected
duplicate SaveStateEntry: id=slirp, instance_id=0x0
Broken pipe
../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried
to terminate QEMU process but encountered exit status 1 (expected 0)
Aborted (core dumped)
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
net/slirp.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index c33b3e02e7..25b49c4526 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -46,6 +46,7 @@
#include "qapi/qmp/qdict.h"
#include "util.h"
#include "migration/register.h"
+#include "migration/vmstate.h"
#include "migration/qemu-file-types.h"
static int get_str_sep(char *buf, int buf_size, const char **pp, int sep)
@@ -659,8 +660,8 @@ static int net_slirp_init(NetClientState *peer, const char
*model,
* specific version?
*/
g_assert(slirp_state_version() == 4);
-register_savevm_live("slirp", 0, slirp_state_version(),
- _slirp_state, s->slirp);
+register_savevm_live("slirp", VMSTATE_INSTANCE_ID_ANY,
+ slirp_state_version(), _slirp_state, s->slirp);
s->poll_notifier.notify = net_slirp_poll_notify;
main_loop_poll_add_notifier(>poll_notifier);
Re: [PATCH 04/13] migration: Use vmstate_register_any() for ipmi-bt*
On 10/19/23 15:08, Juan Quintela wrote:
Otherwise device-introspection-test fails.
$ ./tests/qtest/device-introspect-test
...
Broken pipe
../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried
to terminate QEMU process but encountered exit status 1 (expected 0)
Aborted (core dumped)
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
hw/ipmi/ipmi_bmc_extern.c | 2 +-
hw/ipmi/ipmi_bmc_sim.c| 2 +-
hw/ipmi/isa_ipmi_bt.c | 2 +-
hw/ipmi/isa_ipmi_kcs.c| 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/ipmi/ipmi_bmc_extern.c b/hw/ipmi/ipmi_bmc_extern.c
index e232d35ba2..324a2c8835 100644
--- a/hw/ipmi/ipmi_bmc_extern.c
+++ b/hw/ipmi/ipmi_bmc_extern.c
@@ -504,7 +504,7 @@ static void ipmi_bmc_extern_init(Object *obj)
IPMIBmcExtern *ibe = IPMI_BMC_EXTERN(obj);
ibe->extern_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, extern_timeout, ibe);
-vmstate_register(NULL, 0, _ipmi_bmc_extern, ibe);
+vmstate_register_any(NULL, _ipmi_bmc_extern, ibe);
}
static void ipmi_bmc_extern_finalize(Object *obj)
diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 905e091094..404db5d5bc 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -2188,7 +2188,7 @@ static void ipmi_sim_realize(DeviceState *dev, Error
**errp)
ibs->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, ipmi_timeout, ibs);
-vmstate_register(NULL, 0, _ipmi_sim, ibs);
+vmstate_register_any(NULL, _ipmi_sim, ibs);
}
static Property ipmi_sim_properties[] = {
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index a83e7243d6..afb76b548a 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -125,7 +125,7 @@ static void isa_ipmi_bt_init(Object *obj)
ipmi_bmc_find_and_link(obj, (Object **) >bt.bmc);
-vmstate_register(NULL, 0, _ISAIPMIBTDevice, iib);
+vmstate_register_any(NULL, _ISAIPMIBTDevice, iib);
}
static void *isa_ipmi_bt_get_backend_data(IPMIInterface *ii)
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index b2ed70b9da..5ab63b2fcf 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -132,7 +132,7 @@ static void isa_ipmi_kcs_init(Object *obj)
* IPMI device, so receive it, but transmit a different
* version.
*/
-vmstate_register(NULL, 0, _ISAIPMIKCSDevice, iik);
+vmstate_register_any(NULL, _ISAIPMIKCSDevice, iik);
}
static void *isa_ipmi_kcs_get_backend_data(IPMIInterface *ii)
Re: [PATCH 03/13] migration: Use vmstate_register_any() for isa-ide
On 10/19/23 15:08, Juan Quintela wrote: Otherwise qom-test fails. ok 4 /i386/qom/x-remote qemu-system-i386: savevm_state_handler_insert: Detected duplicate SaveStateEntry: id=isa-ide, instance_id=0x0 Broken pipe ../../../../../mnt/code/qemu/full/tests/qtest/libqtest.c:195: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) Aborted (core dumped) $ Signed-off-by: Juan Quintela Reviewed-by: Stefan Berger --- hw/ide/isa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ide/isa.c b/hw/ide/isa.c index 95053e026f..ea60c08116 100644 --- a/hw/ide/isa.c +++ b/hw/ide/isa.c @@ -73,7 +73,7 @@ static void isa_ide_realizefn(DeviceState *dev, Error **errp) ide_bus_init(>bus, sizeof(s->bus), dev, 0, 2); ide_init_ioport(>bus, isadev, s->iobase, s->iobase2); ide_bus_init_output_irq(>bus, isa_get_irq(isadev, s->irqnum)); -vmstate_register(VMSTATE_IF(dev), 0, _ide_isa, s); +vmstate_register_any(VMSTATE_IF(dev), _ide_isa, s); ide_bus_register_restart_cb(>bus); }
Re: [PATCH 02/13] migration: Use vmstate_register_any()
On 10/19/23 15:08, Juan Quintela wrote:
This are the easiest cases, where we were already using
VMSTATE_INSTANCE_ID_ANY.
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
backends/dbus-vmstate.c | 3 +--
backends/tpm/tpm_emulator.c | 3 +--
hw/i2c/core.c | 2 +-
hw/input/adb.c | 2 +-
hw/input/ads7846.c | 2 +-
hw/input/stellaris_input.c | 3 +--
hw/net/eepro100.c | 3 +--
hw/pci/pci.c| 2 +-
hw/ppc/spapr_nvdimm.c | 3 +--
hw/timer/arm_timer.c| 2 +-
hw/virtio/virtio-mem.c | 4 ++--
11 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index 57369ec0f2..a9d8cb0acd 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -426,8 +426,7 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp)
return;
}
-if (vmstate_register(VMSTATE_IF(self), VMSTATE_INSTANCE_ID_ANY,
- _vmstate, self) < 0) {
+if (vmstate_register_any(VMSTATE_IF(self), _vmstate, self) < 0) {
error_setg(errp, "Failed to register vmstate");
}
}
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c
index 402a2d6312..8920b75251 100644
--- a/backends/tpm/tpm_emulator.c
+++ b/backends/tpm/tpm_emulator.c
@@ -978,8 +978,7 @@ static void tpm_emulator_inst_init(Object *obj)
qemu_add_vm_change_state_handler(tpm_emulator_vm_state_change,
tpm_emu);
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
- _tpm_emulator, obj);
+vmstate_register_any(NULL, _tpm_emulator, obj);
}
/*
diff --git a/hw/i2c/core.c b/hw/i2c/core.c
index bed594fe59..879a1d45cb 100644
--- a/hw/i2c/core.c
+++ b/hw/i2c/core.c
@@ -64,7 +64,7 @@ I2CBus *i2c_init_bus(DeviceState *parent, const char *name)
bus = I2C_BUS(qbus_new(TYPE_I2C_BUS, parent, name));
QLIST_INIT(>current_devs);
QSIMPLEQ_INIT(>pending_masters);
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, _i2c_bus, bus);
+vmstate_register_any(NULL, _i2c_bus, bus);
return bus;
}
diff --git a/hw/input/adb.c b/hw/input/adb.c
index 214ae6f42b..8aed0da2cd 100644
--- a/hw/input/adb.c
+++ b/hw/input/adb.c
@@ -247,7 +247,7 @@ static void adb_bus_realize(BusState *qbus, Error **errp)
adb_bus->autopoll_timer = timer_new_ms(QEMU_CLOCK_VIRTUAL, adb_autopoll,
adb_bus);
-vmstate_register(NULL, -1, _adb_bus, adb_bus);
+vmstate_register_any(NULL, _adb_bus, adb_bus);
}
static void adb_bus_unrealize(BusState *qbus)
diff --git a/hw/input/ads7846.c b/hw/input/ads7846.c
index dc0998ac79..91116c6bdb 100644
--- a/hw/input/ads7846.c
+++ b/hw/input/ads7846.c
@@ -158,7 +158,7 @@ static void ads7846_realize(SSIPeripheral *d, Error **errp)
ads7846_int_update(s);
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, _ads7846, s);
+vmstate_register_any(NULL, _ads7846, s);
}
static void ads7846_class_init(ObjectClass *klass, void *data)
diff --git a/hw/input/stellaris_input.c b/hw/input/stellaris_input.c
index e6ee5e11f1..a58721c8cd 100644
--- a/hw/input/stellaris_input.c
+++ b/hw/input/stellaris_input.c
@@ -88,6 +88,5 @@ void stellaris_gamepad_init(int n, qemu_irq *irq, const int
*keycode)
}
s->num_buttons = n;
qemu_add_kbd_event_handler(stellaris_gamepad_put_key, s);
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY,
- _stellaris_gamepad, s);
+vmstate_register_any(NULL, _stellaris_gamepad, s);
}
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index dc07984ae9..94ce9e18ff 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -1883,8 +1883,7 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error
**errp)
s->vmstate = g_memdup(_eepro100, sizeof(vmstate_eepro100));
s->vmstate->name = qemu_get_queue(s->nic)->model;
-vmstate_register(VMSTATE_IF(_dev->qdev), VMSTATE_INSTANCE_ID_ANY,
- s->vmstate, s);
+vmstate_register_any(VMSTATE_IF(_dev->qdev), s->vmstate, s);
}
static void eepro100_instance_init(Object *obj)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b0d21bf43a..294c3c38ea 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -147,7 +147,7 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
bus->machine_done.notify = pcibus_machine_done;
qemu_add_machine_init_done_notifier(>machine_done);
-vmstate_register(NULL, VMSTATE_INSTANCE_ID_ANY, _pcibus, bus);
+vmstate_register_any(NULL, _pcibus, bus);
}
static void pcie_bus_realize(BusState *qbus, Error **errp)
diff --git a/hw/ppc/spapr_nvdimm.c b/hw/ppc/spapr_nvdimm.c
index b2f009c816..ad7afe7544 100644
--- a/hw/ppc/spapr_nvdimm.c
+++ b/hw/ppc/spapr_nvdimm.c
@@ -876,8 +876,7 @@ static void spapr_nvdimm_realize(NVDIMMDevice *dimm, E
Re: [PATCH 01/13] migration: Create vmstate_register_any()
On 10/19/23 15:08, Juan Quintela wrote:
We have lots of cases where we are using an instance_id==0 when we
should be using VMSTATE_INSTANCE_ID_ANY (-1). Basically everything
that can have more than one needs to have a proper instance_id or -1
and the system will take one for it.
vmstate_register_any(): We register with -1.
Signed-off-by: Juan Quintela
Reviewed-by: Stefan Berger
---
include/migration/vmstate.h | 17 +
1 file changed, 17 insertions(+)
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 1a31fb7293..9ca7e9cc48 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1230,6 +1230,23 @@ static inline int vmstate_register(VMStateIf *obj, int
instance_id,
opaque, -1, 0, NULL);
}
+/**
+ * vmstate_register_any() - legacy function to register state
+ * serialisation description and let the function choose the id
+ *
+ * New code shouldn't be using this function as QOM-ified devices have
+ * dc->vmsd to store the serialisation description.
+ *
+ * Returns: 0 on success, -1 on failure
+ */
+static inline int vmstate_register_any(VMStateIf *obj,
+ const VMStateDescription *vmsd,
+ void *opaque)
+{
+return vmstate_register_with_alias_id(obj, VMSTATE_INSTANCE_ID_ANY, vmsd,
+ opaque, -1, 0, NULL);
+}
+
void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
void *opaque);
Re: [PATCH 15/21] qapi: Inline QERR_MISSING_PARAMETER definition (constant parameter)
On 10/4/23 13:31, Philippe Mathieu-Daudé wrote:
Address the comment added in commit 4629ed1e98
("qerror: Finally unused, clean up"), from 2015:
/*
* These macros will go away, please don't use
* in new code, and do not add new ones!
*/
Mechanical transformation using the following
coccinelle semantic patches:
@match@
expression errp;
constant param;
@@
error_setg(errp, QERR_MISSING_PARAMETER, param);
@script:python strformat depends on match@
param << match.param;
fixedfmt; // new var
@@
if param[0] == '"': # Format skipping '"',
fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"'
else: # or use definition.
fixedfmt = f'"Parameter " {param} " is missing"'
coccinelle.fixedfmt = cocci.make_ident(fixedfmt)
@replace@
expression match.errp;
constant match.param;
identifier strformat.fixedfmt;
@@
-error_setg(errp, QERR_MISSING_PARAMETER, param);
+error_setg(errp, fixedfmt);
and:
@match@
constant param;
@@
error_report(QERR_MISSING_PARAMETER, param);
@script:python strformat depends on match@
param << match.param;
fixedfmt; // new var
@@
fixedfmt = f'"Parameter \'{param[1:-1]}\' is missing"'
coccinelle.fixedfmt = cocci.make_ident(fixedfmt)
@replace@
constant match.param;
identifier strformat.fixedfmt;
@@
-error_report(QERR_MISSING_PARAMETER, param);
+error_report(fixedfmt);
Signed-off-by: Philippe Mathieu-Daud
Reviewed-by: Stefan Berger
---
backends/dbus-vmstate.c| 2 +-
block/gluster.c| 21 +++--
block/monitor/block-hmp-cmds.c | 6 +++---
dump/dump.c| 4 ++--
hw/usb/redirect.c | 2 +-
softmmu/qdev-monitor.c | 2 +-
softmmu/tpm.c | 4 ++--
softmmu/vl.c | 4 ++--
ui/input-barrier.c | 2 +-
ui/ui-qmp-cmds.c | 2 +-
10 files changed, 25 insertions(+), 24 deletions(-)
diff --git a/backends/dbus-vmstate.c b/backends/dbus-vmstate.c
index 57369ec0f2..e781ded17c 100644
--- a/backends/dbus-vmstate.c
+++ b/backends/dbus-vmstate.c
@@ -413,7 +413,7 @@ dbus_vmstate_complete(UserCreatable *uc, Error **errp)
}
if (!self->dbus_addr) {
-error_setg(errp, QERR_MISSING_PARAMETER, "addr");
+error_setg(errp, "Parameter 'addr' is missing");
return;
}
diff --git a/block/gluster.c b/block/gluster.c
index ad5fadbe79..8d97d698c3 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -530,20 +530,20 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster
*gconf,
num_servers = qdict_array_entries(options, GLUSTER_OPT_SERVER_PATTERN);
if (num_servers < 1) {
-error_setg(_err, QERR_MISSING_PARAMETER, "server");
+error_setg(_err, "Parameter 'server' is missing");
goto out;
}
ptr = qemu_opt_get(opts, GLUSTER_OPT_VOLUME);
if (!ptr) {
-error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_VOLUME);
+error_setg(_err, "Parameter " GLUSTER_OPT_VOLUME " is missing");
goto out;
}
gconf->volume = g_strdup(ptr);
ptr = qemu_opt_get(opts, GLUSTER_OPT_PATH);
if (!ptr) {
-error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_PATH);
+error_setg(_err, "Parameter " GLUSTER_OPT_PATH " is missing");
goto out;
}
gconf->path = g_strdup(ptr);
@@ -562,7 +562,8 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster
*gconf,
ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE);
if (!ptr) {
-error_setg(_err, QERR_MISSING_PARAMETER, GLUSTER_OPT_TYPE);
+error_setg(_err,
+ "Parameter " GLUSTER_OPT_TYPE " is missing");
error_append_hint(_err, GERR_INDEX_HINT, i);
goto out;
@@ -592,16 +593,16 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster
*gconf,
ptr = qemu_opt_get(opts, GLUSTER_OPT_HOST);
if (!ptr) {
-error_setg(_err, QERR_MISSING_PARAMETER,
- GLUSTER_OPT_HOST);
+error_setg(_err,
+ "Parameter " GLUSTER_OPT_HOST " is missing");
error_append_hint(_err, GERR_INDEX_HINT, i);
goto out;
}
gsconf->u.inet.host = g_strdup(ptr);
ptr = qemu_opt_get(opts, GLUSTER_OPT_PORT);
if (!ptr) {
-error_setg(_err, QERR_MISSING_PARAMETER,
- GLUSTER_OPT_PORT);
+error_setg(_err,
+
Re: [PATCH 12/21] qapi: Inline and remove QERR_INVALID_PARAMETER_VALUE definition
On 10/4/23 13:31, Philippe Mathieu-Daudé wrote:
Address the comment added in commit 4629ed1e98
("qerror: Finally unused, clean up"), from 2015:
/*
* These macros will go away, please don't use
* in new code, and do not add new ones!
*/
Manually modify the error_report() call in softmmu/tpm.c,
then use sed to mechanically transform the rest. Finally
remove the definition in include/qapi/qmp/qerror.h.
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Berger
---
include/qapi/qmp/qerror.h | 3 ---
qapi/opts-visitor.c | 4 ++--
qapi/qapi-visit-core.c| 4 ++--
softmmu/tpm.c | 3 +--
4 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index b723830eff..ac727d1c2d 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -17,9 +17,6 @@
* add new ones!
*/
-#define QERR_INVALID_PARAMETER_VALUE \
-"Parameter '%s' expects %s"
-
#define QERR_IO_ERROR \
"An IO error has occurred"
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 0393704a73..844db583f4 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -441,7 +441,7 @@ opts_type_int64(Visitor *v, const char *name, int64_t *obj,
Error **errp)
}
}
}
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+error_setg(errp, "Parameter '%s' expects %s", opt->name,
(ov->list_mode == LM_NONE) ? "an int64 value" :
"an int64 value or range");
return false;
@@ -494,7 +494,7 @@ opts_type_uint64(Visitor *v, const char *name, uint64_t
*obj, Error **errp)
}
}
}
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+error_setg(errp, "Parameter '%s' expects %s", opt->name,
(ov->list_mode == LM_NONE) ? "a uint64 value" :
"a uint64 value or range");
return false;
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 6c13510a2b..01793d6e74 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -194,7 +194,7 @@ static bool visit_type_uintN(Visitor *v, uint64_t *obj,
const char *name,
}
if (value > max) {
assert(v->type == VISITOR_INPUT);
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+error_setg(errp, "Parameter '%s' expects %s",
name ? name : "null", type);
return false;
}
@@ -262,7 +262,7 @@ static bool visit_type_intN(Visitor *v, int64_t *obj, const
char *name,
}
if (value < min || value > max) {
assert(v->type == VISITOR_INPUT);
-error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+error_setg(errp, "Parameter '%s' expects %s",
name ? name : "null", type);
return false;
}
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index 578563f05a..8437c4efc3 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -120,8 +120,7 @@ static int tpm_init_tpmdev(void *dummy, QemuOpts *opts,
Error **errp)
i = qapi_enum_parse(_lookup, value, -1, NULL);
be = i >= 0 ? tpm_be_find_by_type(i) : NULL;
if (be == NULL) {
-error_report(QERR_INVALID_PARAMETER_VALUE,
- "type", "a TPM backend type");
+error_report("Parameter 'type' expects a TPM backend type");
tpm_display_backend_drivers();
return 1;
}
Re: [PATCH v3 15/16] sysemu/tpm: Clean up global variable shadowing
On 10/4/23 08:00, Philippe Mathieu-Daudé wrote:
Fix:
softmmu/tpm.c:178:59: error: declaration shadows a variable in the global
scope [-Werror,-Wshadow]
int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/getopt.h:77:14:
note: previous declaration is here
extern char *optarg;/* getopt(3) external variables */
^
Signed-off-by: Philippe Mathieu-Daudé
Reviewed-by: Stefan Berger
---
include/sysemu/tpm.h | 2 +-
softmmu/tpm.c| 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 66e3b45f30..1ee568b3b6 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -17,7 +17,7 @@
#ifdef CONFIG_TPM
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg);
+int tpm_config_parse(QemuOptsList *opts_list, const char *optstr);
int tpm_init(void);
void tpm_cleanup(void);
diff --git a/softmmu/tpm.c b/softmmu/tpm.c
index 578563f05a..7164ea7ff1 100644
--- a/softmmu/tpm.c
+++ b/softmmu/tpm.c
@@ -175,15 +175,15 @@ int tpm_init(void)
* Parse the TPM configuration options.
* To display all available TPM backends the user may use '-tpmdev help'
*/
-int tpm_config_parse(QemuOptsList *opts_list, const char *optarg)
+int tpm_config_parse(QemuOptsList *opts_list, const char *optstr)
{
QemuOpts *opts;
-if (!strcmp(optarg, "help")) {
+if (!strcmp(optstr, "help")) {
tpm_display_backend_drivers();
return -1;
}
-opts = qemu_opts_parse_noisily(opts_list, optarg, true);
+opts = qemu_opts_parse_noisily(opts_list, optstr, true);
if (!opts) {
return -1;
}
Re: [PATCH v3 13/16] slirp: open-code qemu_socket_(un)select()
On 3/6/23 09:03, Marc-André Lureau wrote:
On Mon, Mar 6, 2023 at 5:59 PM Stefan Berger mailto:stef...@linux.ibm.com>> wrote:
On 2/21/23 07:47, marcandre.lur...@redhat.com
<mailto:marcandre.lur...@redhat.com> wrote:
> From: Marc-André Lureau mailto:marcandre.lur...@redhat.com>>
>
> We are about to make the QEMU socket API use file-descriptor space only,
> but libslirp gives us SOCKET as fd, still.
>
> Signed-off-by: Marc-André Lureau mailto:marcandre.lur...@redhat.com>>
> ---
> net/slirp.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/net/slirp.c b/net/slirp.c
> index a7c35778a6..c33b3e02e7 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd,
void *opaque)
Shouldn't this int fd rather be a SOCKET s instead? Or do you get compiler
warnings then?
Yes, you would get compiler warnings, because the "int fd" argument is from the
slirp API, whether it is posix or win32.
Right, this is shared code.
> #ifdef WIN32
> AioContext *ctxt = qemu_get_aio_context();
>
> - qemu_socket_select(fd, event_notifier_get_handle(>notifier),
> + if (WSAEventSelect(fd, event_notifier_get_handle(>notifier),
> FD_READ | FD_ACCEPT | FD_CLOSE |
> - FD_CONNECT | FD_WRITE | FD_OOB, NULL);
> + FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
> + error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
> + }
> #endif
> }
> > static void net_slirp_unregister_poll_fd(int fd, void *opaque)
Same here.
> {
> #ifdef WIN32
> - qemu_socket_unselect(fd, NULL);
> + if (WSAEventSelect(fd, NULL, 0) != 0) {
> + error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
> + }
> #endif
> }
>
Re: [PATCH v3 16/16] win32: replace closesocket() with close() wrapper
On 2/21/23 07:48, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Use a close() wrapper instead, so that we don't need to worry about
closesocket() vs close() anymore, let's hope.
Signed-off-by: Marc-André Lureau
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 7836fb0be3..29a667ae3d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -370,39 +370,39 @@ int qemu_bind_wrap(int sockfd, const struct sockaddr
*addr,
}
-#undef closesocket
-int qemu_closesocket_wrap(int fd)
+#undef close
+int qemu_close_wrap(int fd)
{
int ret;
DWORD flags = 0;
-SOCKET s = _get_osfhandle(fd);
+SOCKET s = INVALID_SOCKET;
-if (s == INVALID_SOCKET) {
-return -1;
-}
+if (fd_is_socket(fd)) {
+s = _get_osfhandle(fd);
-/*
- * If we were to just call _close on the descriptor, it would close the
- * HANDLE, but it wouldn't free any of the resources associated to the
- * SOCKET, and we can't call _close after calling closesocket, because
- * closesocket has already closed the HANDLE, and _close would attempt to
- * close the HANDLE again, resulting in a double free. We can however
- * protect the HANDLE from actually being closed long enough to close the
- * file descriptor, then close the socket itself.
- */
-if (!GetHandleInformation((HANDLE)s, )) {
-errno = EACCES;
-return -1;
-}
+/*
+ * If we were to just call _close on the descriptor, it would close the
+ * HANDLE, but it wouldn't free any of the resources associated to the
+ * SOCKET, and we can't call _close after calling closesocket, because
+ * closesocket has already closed the HANDLE, and _close would attempt
to
+ * close the HANDLE again, resulting in a double free. We can however
+ * protect the HANDLE from actually being closed long enough to close
the
+ * file descriptor, then close the socket itself.
+ */
+if (!GetHandleInformation((HANDLE)s, )) {
+errno = EACCES;
+return -1;
+}
-if (!SetHandleInformation((HANDLE)s, HANDLE_FLAG_PROTECT_FROM_CLOSE,
HANDLE_FLAG_PROTECT_FROM_CLOSE)) {
-errno = EACCES;
-return -1;
+if (!SetHandleInformation((HANDLE)s, HANDLE_FLAG_PROTECT_FROM_CLOSE,
HANDLE_FLAG_PROTECT_FROM_CLOSE)) {
+errno = EACCES;
+return -1;
+}
}
ret = close(fd);
-if (!SetHandleInformation((HANDLE)s, flags, flags)) {
+if (s != INVALID_SOCKET && !SetHandleInformation((HANDLE)s, flags, flags))
{
errno = EACCES;
return -1;
}
@@ -411,13 +411,15 @@ int qemu_closesocket_wrap(int fd)
* close() returns EBADF since we PROTECT_FROM_CLOSE the underlying
handle,
* but the FD is actually freed
*/
-if (ret < 0 && errno != EBADF) {
+if (ret < 0 && (s == INVALID_SOCKET || errno != EBADF)) {
return ret;
}
-ret = closesocket(s);
-if (ret < 0) {
-errno = socket_error();
+if (s != INVALID_SOCKET) {
+ret = closesocket(s);
+if (ret < 0) {
+errno = socket_error();
+}
}
if (fs_is_socket()) {{
...
close()
...
closesocket()
...
} else {
...
close()
...
}
would avoid the threading and make this function look a bit simpler.
Reviewed-by: Stefan Berger
Re: [PATCH v3 13/16] slirp: open-code qemu_socket_(un)select()
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
We are about to make the QEMU socket API use file-descriptor space only,
but libslirp gives us SOCKET as fd, still.
Signed-off-by: Marc-André Lureau
---
net/slirp.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index a7c35778a6..c33b3e02e7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void
*opaque)
Shouldn't this int fd rather be a SOCKET s instead? Or do you get compiler
warnings then?
#ifdef WIN32
AioContext *ctxt = qemu_get_aio_context();
-qemu_socket_select(fd, event_notifier_get_handle(>notifier),
+if (WSAEventSelect(fd, event_notifier_get_handle(>notifier),
FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+ FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
+error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
> static void net_slirp_unregister_poll_fd(int fd, void *opaque)
Same here.
{
#ifdef WIN32
-qemu_socket_unselect(fd, NULL);
+if (WSAEventSelect(fd, NULL, 0) != 0) {
+error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
Re: [PATCH v3 14/16] win32: avoid mixing SOCKET and file descriptor space
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau Until now, a win32 SOCKET handle is often cast to an int file descriptor, as this is what other OS use for sockets. When necessary, QEMU eventually queries whether it's a socket with the help of fd_is_socket(). However, there is no guarantee of conflict between the fd and SOCKET space. Such conflict would have surprising consequences, we shouldn't mix them. Also, it is often forgotten that SOCKET must be closed with closesocket(), and not close(). Instead, let's make the win32 socket wrapper functions return and take a file descriptor, and let util/ wrappers do the fd/SOCKET conversion as necessary. A bit of adaptation is necessary in io/ as well. Unfortunately, we can't drop closesocket() usage, despite _open_osfhandle() documentation claiming transfer of ownership, testing shows bad behaviour if you forget to call closesocket(). Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 13/16] slirp: open-code qemu_socket_(un)select()
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
We are about to make the QEMU socket API use file-descriptor space only,
but libslirp gives us SOCKET as fd, still.
Signed-off-by: Marc-André Lureau
---
net/slirp.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/slirp.c b/net/slirp.c
index a7c35778a6..c33b3e02e7 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -251,16 +251,20 @@ static void net_slirp_register_poll_fd(int fd, void
*opaque)
#ifdef WIN32
AioContext *ctxt = qemu_get_aio_context();
-qemu_socket_select(fd, event_notifier_get_handle(>notifier),
+if (WSAEventSelect(fd, event_notifier_get_handle(>notifier),
FD_READ | FD_ACCEPT | FD_CLOSE |
- FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+ FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
+error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
#ifdef WIN32
-qemu_socket_unselect(fd, NULL);
+if (WSAEventSelect(fd, NULL, 0) != 0) {
+error_setg_win32(_warn, WSAGetLastError(), "failed to
WSAEventSelect()");
+}
#endif
}
Reviewed-by: Stefan Berger
Re: [PATCH v3 14/16] win32: avoid mixing SOCKET and file descriptor space
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Until now, a win32 SOCKET handle is often cast to an int file
descriptor, as this is what other OS use for sockets. When necessary,
QEMU eventually queries whether it's a socket with the help of
fd_is_socket(). However, there is no guarantee of conflict between the
fd and SOCKET space. Such conflict would have surprising consequences,
we shouldn't mix them.
Also, it is often forgotten that SOCKET must be closed with
closesocket(), and not close().
Instead, let's make the win32 socket wrapper functions return and take a
file descriptor, and let util/ wrappers do the fd/SOCKET conversion as
necessary. A bit of adaptation is necessary in io/ as well.
Unfortunately, we can't drop closesocket() usage, despite
_open_osfhandle() documentation claiming transfer of ownership, testing
shows bad behaviour if you forget to call closesocket().
Signed-off-by: Marc-André Lureau
---
include/sysemu/os-win32.h | 4 +-
io/channel-watch.c| 6 +-
util/aio-win32.c | 9 +-
util/oslib-win32.c| 219 --
4 files changed, 197 insertions(+), 41 deletions(-)
#undef connect
@@ -308,7 +315,13 @@ int qemu_connect_wrap(int sockfd, const struct sockaddr
*addr,
socklen_t addrlen)
{
int ret;
-ret = connect(sockfd, addr, addrlen);
+SOCKET s = _get_osfhandle(sockfd);
+
+if (s == INVALID_SOCKET) {
+return -1;
+}
+
+ret = connect(s, addr, addrlen);
Previously you passed int sockfd and now you convert this sockfd to a SOCKET s
and you can pass this as an alternative? Or was sockfd before this patch a
SOCKET??
Stefan
Re: [PATCH v3 12/16] slirp: unregister the win32 SOCKET
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Presumably, this is what should happen when the SOCKET is to be removed.
(it probably worked until now because closesocket() does it implicitly,
but we never now how the slirp library could use the SOCKET later)
Signed-off-by: Marc-André Lureau
---
net/slirp.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/slirp.c b/net/slirp.c
index 0730a935ba..a7c35778a6 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -259,7 +259,9 @@ static void net_slirp_register_poll_fd(int fd, void *opaque)
static void net_slirp_unregister_poll_fd(int fd, void *opaque)
{
-/* no qemu_fd_unregister */
+#ifdef WIN32
The majority of code seems to use _WIN32. Not sure what is 'right'.
Reviewed-by: Stefan Berger
+qemu_socket_unselect(fd, NULL);
+#endif
}
static void net_slirp_notify(void *opaque)
Re: [PATCH v3 11/16] main-loop: remove qemu_fd_register(), win32/slirp/socket specific
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Open-code the socket registration where it's needed, to avoid
artificially used or unclear generic interface.
Furthermore, the following patches are going to make socket handling use
FD-only inside QEMU, but we need to handle win32 SOCKET from libslirp.
Signed-off-by: Marc-André Lureau
---
include/qemu/main-loop.h | 2 --
net/slirp.c | 8 +++-
util/main-loop.c | 11 ---
3 files changed, 7 insertions(+), 14 deletions(-)
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index c25f390696..b3e54e00bc 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -387,8 +387,6 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
/* internal interfaces */
-void qemu_fd_register(int fd);
-
#define qemu_bh_new(cb, opaque) \
qemu_bh_new_full((cb), (opaque), (stringify(cb)))
QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name);
diff --git a/net/slirp.c b/net/slirp.c
index 2ee3f1a0d7..0730a935ba 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -248,7 +248,13 @@ static void net_slirp_timer_mod(void *timer, int64_t
expire_timer,
static void net_slirp_register_poll_fd(int fd, void *opaque)
{
-qemu_fd_register(fd);
+#ifdef WIN32
_WIN32 ?
With this fixed:
Reviewed-by: Stefan Berger
Re: [PATCH v3 09/16] aio/win32: aio_set_fd_handler() only supports SOCKET
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau Let's check if the argument is actually a SOCKET, else report an error and return. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 08/16] aio: make aio_set_fd_poll() static to aio-posix.c
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 07/16] win32/socket: introduce qemu_socket_unselect() helper
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau A more explicit version of qemu_socket_select() with no events. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 06/16] win32/socket: introduce qemu_socket_select() helper
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau This is a wrapper for WSAEventSelect, with Error handling. By default, it will produce a warning, so callers don't have to be modified now, and yet we can spot potential mis-use. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 05/16] error: add global _warn destination
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau This can help debugging issues or develop, when error handling is introduced. Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH v3 04/16] tests: add test-error-report
On 2/21/23 07:47, marcandre.lur...@redhat.com wrote: From: Marc-André Lureau Signed-off-by: Marc-André Lureau Reviewed-by: Stefan Berger
Re: [PATCH 12/32] block: Factor out hmp_change_medium(), and move to block/monitor/
On 1/24/23 07:19, Markus Armbruster wrote: Signed-off-by: Markus Armbruster Reviewed-by: Stefan Berger
Re: [PATCH 01/32] monitor: Drop unnecessary includes
On 1/24/23 07:19, Markus Armbruster wrote: Signed-off-by: Markus Armbruster Reviewed-by: Stefan Berger
Re: [PATCH 20/32] tpm: Move HMP commands from monitor/ to softmmu/
On 1/24/23 07:19, Markus Armbruster wrote:
This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" to "TPM".
Signed-off-by: Markus Armbruster
Reviewed-by: Stefan Berger
---
MAINTAINERS| 2 +-
monitor/hmp-cmds.c | 54 ---
softmmu/tpm-hmp-cmds.c | 65 ++
softmmu/meson.build| 1 +
4 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 softmmu/tpm-hmp-cmds.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3bd4d101d3..dab4def753 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3067,7 +3067,7 @@ T: git https://github.com/stefanha/qemu.git tracing
TPM
M: Stefan Berger
S: Maintained
-F: softmmu/tpm.c
+F: softmmu/tpm*
F: hw/tpm/*
F: include/hw/acpi/tpm.h
F: include/sysemu/tpm*
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 6b1d5358f7..81f63fa8ec 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -22,7 +22,6 @@
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-run-state.h"
#include "qapi/qapi-commands-stats.h"
-#include "qapi/qapi-commands-tpm.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "qemu/cutils.h"
@@ -126,59 +125,6 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
hmp_info_pic_foreach, mon);
}
-void hmp_info_tpm(Monitor *mon, const QDict *qdict)
-{
-#ifdef CONFIG_TPM
-TPMInfoList *info_list, *info;
-Error *err = NULL;
-unsigned int c = 0;
-TPMPassthroughOptions *tpo;
-TPMEmulatorOptions *teo;
-
-info_list = qmp_query_tpm();
-if (err) {
-monitor_printf(mon, "TPM device not supported\n");
-error_free(err);
-return;
-}
-
-if (info_list) {
-monitor_printf(mon, "TPM device:\n");
-}
-
-for (info = info_list; info; info = info->next) {
-TPMInfo *ti = info->value;
-monitor_printf(mon, " tpm%d: model=%s\n",
- c, TpmModel_str(ti->model));
-
-monitor_printf(mon, " \\ %s: type=%s",
- ti->id, TpmType_str(ti->options->type));
-
-switch (ti->options->type) {
-case TPM_TYPE_PASSTHROUGH:
-tpo = ti->options->u.passthrough.data;
-monitor_printf(mon, "%s%s%s%s",
- tpo->path ? ",path=" : "",
- tpo->path ?: "",
- tpo->cancel_path ? ",cancel-path=" : "",
- tpo->cancel_path ?: "");
-break;
-case TPM_TYPE_EMULATOR:
-teo = ti->options->u.emulator.data;
-monitor_printf(mon, ",chardev=%s", teo->chardev);
-break;
-case TPM_TYPE__MAX:
-break;
-}
-monitor_printf(mon, "\n");
-c++;
-}
-qapi_free_TPMInfoList(info_list);
-#else
-monitor_printf(mon, "TPM device not supported\n");
-#endif /* CONFIG_TPM */
-}
-
void hmp_quit(Monitor *mon, const QDict *qdict)
{
monitor_suspend(mon);
diff --git a/softmmu/tpm-hmp-cmds.c b/softmmu/tpm-hmp-cmds.c
new file mode 100644
index 00..9ed6ad6c4d
--- /dev/null
+++ b/softmmu/tpm-hmp-cmds.c
@@ -0,0 +1,65 @@
+/*
+ * HMP commands related to TPM
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-tpm.h"
+#include "monitor/monitor.h"
+#include "monitor/hmp.h"
+#include "qapi/error.h"
+
+void hmp_info_tpm(Monitor *mon, const QDict *qdict)
+{
+#ifdef CONFIG_TPM
+TPMInfoList *info_list, *info;
+Error *err = NULL;
+unsigned int c = 0;
+TPMPassthroughOptions *tpo;
+TPMEmulatorOptions *teo;
+
+info_list = qmp_query_tpm();
+if (err) {
+monitor_printf(mon, "TPM device not supported\n");
+error_free(err);
+return;
+}
+
+if (info_list) {
+monitor_printf(mon, "TPM device:\n");
+}
+
+for (info = info_list; info; info = info->next) {
+TPMInfo *ti = info->value;
+monitor_printf(mon, " tpm%d: model=%s\n",
+ c, TpmModel_str(ti->model));
+
+monitor_printf(mon, " \\ %s: type=%s",
+ ti->id, TpmType_str(ti->options->type));
+
+switch (ti->options->type) {
+case TPM_TYPE_PASSTHROUGH:
+tpo = ti->options->u.passthrough.data;
+monitor_printf(mon, "%s%s%s%s",
+ tpo->path ? ",path=" : "",
+
Re: [RFC 1/1] hw: tpmtisspi: add SPI support to QEMU TPM implementation
On 8/3/22 04:52, Cédric Le Goater wrote:
On 8/3/22 04:32, Iris Chen wrote:
From: Iris Chen
+++ b/hw/tpm/tpm_tis_spi.c
@@ -0,0 +1,311 @@
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "hw/acpi/tpm.h"
+#include "tpm_prop.h"
+#include "tpm_tis.h"
+#include "qom/object.h"
+#include "hw/ssi/ssi.h"
+#include "hw/ssi/spi_gpio.h"
+
+#define TPM_TIS_SPI_ADDR_BYTES 3
+#define SPI_WRITE 0
+
+typedef enum {
+ TIS_SPI_PKT_STATE_DEACTIVATED = 0,
+ TIS_SPI_PKT_STATE_START,
+ TIS_SPI_PKT_STATE_ADDRESS,
+ TIS_SPI_PKT_STATE_DATA_WR,
+ TIS_SPI_PKT_STATE_DATA_RD,
+ TIS_SPI_PKT_STATE_DONE,
+} TpmTisSpiPktState;
+
+union TpmTisRWSizeByte {
+ uint8_t byte;
+ struct {
+ uint8_t data_expected_size:6;
+ uint8_t resv:1;
+ uint8_t rwflag:1;
+ };
I think it would be better to define a mask for the number of bytes and
a flag for read/write rather than using bitfields. It should better for
portability.
Stefan
Re: [RFC 1/1] hw: tpmtisspi: add SPI support to QEMU TPM implementation
On 8/3/22 04:52, Cédric Le Goater wrote:
On 8/3/22 04:32, Iris Chen wrote:
From: Iris Chen
A commit log telling us about this new device would be good to have.
Signed-off-by: Iris Chen
---
configs/devices/arm-softmmu/default.mak | 1 +
hw/arm/Kconfig | 5 +
hw/tpm/Kconfig | 5 +
hw/tpm/meson.build | 1 +
hw/tpm/tpm_tis_spi.c | 311
include/sysemu/tpm.h | 3 +
6 files changed, 326 insertions(+)
create mode 100644 hw/tpm/tpm_tis_spi.c
diff --git a/configs/devices/arm-softmmu/default.mak
b/configs/devices/arm-softmmu/default.mak
index 6985a25377..80d2841568 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -42,3 +42,4 @@ CONFIG_FSL_IMX6UL=y
CONFIG_SEMIHOSTING=y
CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
CONFIG_ALLWINNER_H3=y
+CONFIG_FBOBMC_AST=y
I don't think this extra config is useful for now
diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 15fa79afd3..193decaec1 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -458,6 +458,11 @@ config ASPEED_SOC
select PMBUS
select MAX31785
+config FBOBMC_AST
+ bool
+ select ASPEED_SOC
+ select TPM_TIS_SPI
+
config MPS2
bool
imply I2C_DEVICES
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..370a43f045 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -8,6 +8,11 @@ config TPM_TIS_SYSBUS
depends on TPM
select TPM_TIS
+config TPM_TIS_SPI
+ bool
+ depends on TPM
+ select TPM_TIS
+
config TPM_TIS
bool
depends on TPM
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 1c68d81d6a..1a057f4e36 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/meson.build
@@ -2,6 +2,7 @@ softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true:
files('tpm_tis_common.c'))
softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true:
files('tpm_tis_isa.c'))
softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true:
files('tpm_tis_sysbus.c'))
softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
+softmmu_ss.add(when: 'CONFIG_TPM_TIS_SPI', if_true:
files('tpm_tis_spi.c'))
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true:
files('tpm_ppi.c'))
specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true:
files('tpm_ppi.c'))
diff --git a/hw/tpm/tpm_tis_spi.c b/hw/tpm/tpm_tis_spi.c
new file mode 100644
index 00..c98ddcfddb
--- /dev/null
+++ b/hw/tpm/tpm_tis_spi.c
@@ -0,0 +1,311 @@
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "hw/acpi/tpm.h"
+#include "tpm_prop.h"
+#include "tpm_tis.h"
+#include "qom/object.h"
+#include "hw/ssi/ssi.h"
+#include "hw/ssi/spi_gpio.h"
+
+#define TPM_TIS_SPI_ADDR_BYTES 3
+#define SPI_WRITE 0
+
+typedef enum {
+ TIS_SPI_PKT_STATE_DEACTIVATED = 0,
+ TIS_SPI_PKT_STATE_START,
+ TIS_SPI_PKT_STATE_ADDRESS,
+ TIS_SPI_PKT_STATE_DATA_WR,
+ TIS_SPI_PKT_STATE_DATA_RD,
+ TIS_SPI_PKT_STATE_DONE,
+} TpmTisSpiPktState;
+
+union TpmTisRWSizeByte {
+ uint8_t byte;
+ struct {
+ uint8_t data_expected_size:6;
+ uint8_t resv:1;
+ uint8_t rwflag:1;
+ };
+};
+
+union TpmTisSpiHwAddr {
+ hwaddr addr;
+ uint8_t bytes[sizeof(hwaddr)];
+};
+
+union TpmTisSpiData {
+ uint32_t data;
+ uint8_t bytes[64];
+};
+
+struct TpmTisSpiState {
+ /*< private >*/
+ SSIPeripheral parent_obj;
+
+ /*< public >*/
+ TPMState tpm_state; /* not a QOM object */
+ TpmTisSpiPktState tpm_tis_spi_state;
+
+ union TpmTisRWSizeByte first_byte;
+ union TpmTisSpiHwAddr addr;
+ union TpmTisSpiData data;
Are these device registers ? I am not sure the unions are very useful.
+ uint32_t data_size;
+ uint8_t data_idx;
+ uint8_t addr_idx; >> +};
I suppose that these registers will also have to be stored as part of
the device state (for suspend/resume).
+/*
+ * Pre-reading logic for transfer:
+ * This is to fix the transaction between reading and writing.
+ * The first byte is arbitrarily inserted so we need to
+ * shift the all the output bytes (timeline) one byte right.
-> shift all the output bytes (timeline) one byte to the right
+
+static void tpm_tis_spi_realizefn(SSIPeripheral *ss, Error **errp)
+{
+ TpmTisSpiState *sbdev = TPM_TIS_SPI(ss);
+
+ if (!tpm_find()) {
+ error_setg(errp, "at most one TPM device is permitted");
+ return;
+ }
+
+ if (!sbdev->tpm_state.be_driver) {
+ error_setg(errp, "'tpmdev' property is required");
+ return;
+ }
+
+ DeviceState *spi_gpio = qdev_find_recursive(sysbus_get_default(),
+ TYPE_SPI_GPIO);
+ qdev_connect_gpio_out_named(spi_gpio,
+ "SPI_CS_out", 0,
+ qdev_get_gpio_in_named(DEVICE(ss),
+ SSI_GPIO_CS,
Re: [PATCH v2 05/26] tests: move libqtest.h back under qtest/
On 4/26/22 05:26, marcandre.lur...@redhat.com wrote:
From: Marc-André Lureau
Since commit a2ce7dbd917 ("meson: convert tests/qtest to meson"),
libqtest.h is under libqos/ directory, while libqtest.c is still in
qtest/. Move back to its original location to avoid mixing with libqos/.
Suggested-by: Thomas Huth
Signed-off-by: Marc-André Lureau
Reviewed-by: Thomas Huth
Reviewed-by: Stefan Berger
Re: [PATCH 1/4] tests/qtest: Remove TPM tests
On 1/15/21 1:40 PM, Stefan Berger wrote:
On 1/15/21 11:06 AM, Philippe Mathieu-Daudé wrote:
On 1/15/21 4:53 PM, Stefan Berger wrote:
On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote:
Subject is incorrect, this is not a removal of the tests, but
removal of their execution. The tests are still in the repository.
This is more of a disablement.
How do you compile / run them to have the LeakSanitizer checks?
I used:
../configure --cc=clang --enable-sanitizers && make check-qtest
$ clang -v
clang version 10.0.1 (Fedora 10.0.1-3.fc32)
This was previously covered by patchew CI. I just figured
patchew is running without the LeakSanitizer since commit
6f89ec7442e ("docker: test-debug: disable LeakSanitizer"):
docker: test-debug: disable LeakSanitizer
There are just too many leaks in device-introspect-test (especially
for
the plethora of arm and aarch64 boards) to make LeakSanitizer useful;
disable it for now.
I only get short stack traces:
Indirect leak of 852840 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f8b57 in calloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23fb57)
#1 0x14f0963069b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0)
#2 0x561a8c4c2508 in json_parser_parse
/home/stefanb/tmp/qemu-tip/build/../qobject/json-parser.c:580:14
#3 0x561a8c4a99aa in json_message_process_token
/home/stefanb/tmp/qemu-tip/build/../qobject/json-streamer.c:92:12
#4 0x561a8c4b6cfb in json_lexer_feed_char
/home/stefanb/tmp/qemu-tip/build/../qobject/json-lexer.c:313:13
Indirect leak of 6624 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f8b57 in calloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23fb57)
#1 0x14f0963069b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0)
Indirect leak of 1449 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f899f in malloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23f99f)
#1 0x14f096306958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958)
How can I see more of those?
I now added -fno-omit-frame-pointer to configure (should it not be
there?) and it now shows some useful stacktraces.
diff --git a/configure b/configure
index 155dda124c..ed86b5ca32 100755
--- a/configure
+++ b/configure
@@ -5308,7 +5308,7 @@ if test "$gprof" = "yes" ; then
fi
if test "$have_asan" = "yes"; then
- QEMU_CFLAGS="-fsanitize=address $QEMU_CFLAGS"
+ QEMU_CFLAGS="-fsanitize=address -fno-omit-frame-pointer $QEMU_CFLAGS"
QEMU_LDFLAGS="-fsanitize=address $QEMU_LDFLAGS"
if test "$have_asan_iface_h" = "no" ; then
echo "ASAN build enabled, but ASAN header missing." \
diff --git a/tests/qtest/tpm-util.c b/tests/qtest/tpm-util.c
This is my TPM related fix. Maybe it resolve the issue for you also?
index 5a33a6ef0f..b70cc32d60 100644
--- a/tests/qtest/tpm-util.c
+++ b/tests/qtest/tpm-util.c
@@ -250,7 +250,7 @@ void tpm_util_wait_for_migration_complete(QTestState
*who)
status = qdict_get_str(rsp_return, "status");
completed = strcmp(status, "completed") == 0;
g_assert_cmpstr(status, !=, "failed");
- qobject_unref(rsp_return);
+ qobject_unref(rsp);
if (completed) {
return;
}
Now I see ppc64 related leaks:
Direct leak of 200 byte(s) in 1 object(s) allocated from:
#0 0x14c9b743c837 in __interceptor_calloc (/lib64/libasan.so.6+0xb0837)
#1 0x14c9b6e8b9b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0)
#2 0x55c5e7130a1a in qemu_init_vcpu ../softmmu/cpus.c:618
#3 0x55c5e68b30c0 in ppc_cpu_realize
../target/ppc/translate_init.c.inc:10146
#4 0x55c5e7539c08 in device_set_realized ../hw/core/qdev.c:761
#5 0x55c5e714aa38 in property_set_bool ../qom/object.c:2255
#6 0x55c5e7145d52 in object_property_set ../qom/object.c:1400
#7 0x55c5e714f99f in object_property_set_qobject
../qom/qom-qobject.c:28
#8 0x55c5e71465f4 in object_property_set_bool ../qom/object.c:1470
#9 0x55c5e666ae21 in spapr_realize_vcpu ../hw/ppc/spapr_cpu_core.c:254
#10 0x55c5e666ae21 in spapr_cpu_core_realize
../hw/ppc/spapr_cpu_core.c:337
#11 0x55c5e7539c08 in device_set_realized ../hw/core/qdev.c:761
#12 0x55c5e714aa38 in property_set_bool ../qom/object.c:2255
#13 0x55c5e7145d52 in object_property_set ../qom/object.c:1400
#14 0x55c5e714f99f in object_property_set_qobject
../qom/qom-qobject.c:28
#15 0x55c5e71465f4 in object_property_set_bool ../qom/object.c:1470
#16 0x55c5e5c7553c in qdev_device_add ../softmmu/qdev-monitor.c:665
#17 0x55c5e6fd4cc4 in device_init_func ../softmmu/vl.c:1201
#18 0x55c5e78fc7bb in qemu_opts_foreach ../util/qemu-option.c:1147
#19 0x55c5e6fc8912 in qemu_create_cli_devices ../softmmu/vl.c:2488
#20 0x55c5e6fc8912 in qmp_x_exit_preconfig ../softmmu/vl.c:2527
Re: [PATCH 1/4] tests/qtest: Remove TPM tests
On 1/15/21 11:06 AM, Philippe Mathieu-Daudé wrote:
On 1/15/21 4:53 PM, Stefan Berger wrote:
On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote:
Subject is incorrect, this is not a removal of the tests, but
removal of their execution. The tests are still in the repository.
This is more of a disablement.
How do you compile / run them to have the LeakSanitizer checks?
I used:
../configure --cc=clang --enable-sanitizers && make check-qtest
$ clang -v
clang version 10.0.1 (Fedora 10.0.1-3.fc32)
This was previously covered by patchew CI. I just figured
patchew is running without the LeakSanitizer since commit
6f89ec7442e ("docker: test-debug: disable LeakSanitizer"):
docker: test-debug: disable LeakSanitizer
There are just too many leaks in device-introspect-test (especially for
the plethora of arm and aarch64 boards) to make LeakSanitizer useful;
disable it for now.
I only get short stack traces:
Indirect leak of 852840 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f8b57 in calloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23fb57)
#1 0x14f0963069b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0)
#2 0x561a8c4c2508 in json_parser_parse
/home/stefanb/tmp/qemu-tip/build/../qobject/json-parser.c:580:14
#3 0x561a8c4a99aa in json_message_process_token
/home/stefanb/tmp/qemu-tip/build/../qobject/json-streamer.c:92:12
#4 0x561a8c4b6cfb in json_lexer_feed_char
/home/stefanb/tmp/qemu-tip/build/../qobject/json-lexer.c:313:13
Indirect leak of 6624 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f8b57 in calloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23fb57)
#1 0x14f0963069b0 in g_malloc0 (/lib64/libglib-2.0.so.0+0x589b0)
Indirect leak of 1449 byte(s) in 207 object(s) allocated from:
#0 0x561a8c2f899f in malloc
(/home/stefanb/tmp/qemu-tip/build/tests/qtest/tpm-crb-swtpm-test+0x23f99f)
#1 0x14f096306958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958)
How can I see more of those?
Stefan
Re: [PATCH 1/4] tests/qtest: Remove TPM tests
On 1/15/21 10:52 AM, Philippe Mathieu-Daudé wrote: Subject is incorrect, this is not a removal of the tests, but removal of their execution. The tests are still in the repository. This is more of a disablement. How do you compile / run them to have the LeakSanitizer checks?
Re: [PATCH v2 36/44] qdev: Rename qdev_get_prop_ptr() to object_field_prop_ptr()
On 11/4/20 11:00 AM, Eduardo Habkost wrote:
The function will be moved to common QOM code, as it is not
specific to TYPE_DEVICE anymore.
Signed-off-by: Eduardo Habkost
Reviewed-by: Stefan Berger
---
Changes v1 -> v2:
* Rename to object_field_prop_ptr() instead of object_static_prop_ptr()
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: qemu-de...@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
backends/tpm/tpm_util.c | 6 ++--
hw/block/xen-block.c | 4 +--
hw/core/qdev-properties-system.c | 50 +-
hw/core/qdev-properties.c| 60
hw/s390x/css.c | 4 +--
hw/s390x/s390-pci-bus.c | 4 +--
hw/vfio/pci-quirks.c | 4 +--
8 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 7f8d5fc206..2bec65c8e5 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -223,7 +223,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-void *qdev_get_prop_ptr(Object *obj, Property *prop);
+void *object_field_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(Object *obj,
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 0b07cf55ea..bb1ab34a75 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -35,7 +35,7 @@
static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
+TPMBackend **be = object_field_prop_ptr(obj, opaque);
char *p;
p = g_strdup(*be ? (*be)->id : "");
@@ -47,7 +47,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend *s, **be = object_field_prop_ptr(obj, prop);
char *str;
if (!visit_type_str(v, name, , errp)) {
@@ -67,7 +67,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void release_tpm(Object *obj, const char *name, void *opaque)
{
Property *prop = opaque;
-TPMBackend **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend **be = object_field_prop_ptr(obj, prop);
if (*be) {
tpm_backend_reset(*be);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index bd1aef63a7..718d886e5c 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -336,7 +336,7 @@ static void xen_block_get_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
char *str;
switch (vdev->type) {
@@ -396,7 +396,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_field_prop_ptr(obj, prop);
char *str, *p;
const char *end;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 96a0bc5109..8781b856d3 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -62,7 +62,7 @@ static void get_drive(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_field_prop_ptr(obj, prop);
const char *value;
char *p;
@@ -88,7 +88,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_field_prop_ptr(obj, prop);
char *str;
BlockBackend *blk;
bool blk_created = false;
@@ -181,7 +181,7 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-BlockBackend **ptr = qdev_get_prop_ptr(obj, p
Re: [PATCH v2 22/44] qdev: Move dev->realized check to qdev_property_set()
On 11/4/20 10:59 AM, Eduardo Habkost wrote:
Every single qdev property setter function manually checks
dev->realized. We can just check dev->realized inside
qdev_property_set() instead.
The check is being added as a separate function
(qdev_prop_allow_set()) because it will become a callback later.
Signed-off-by: Eduardo Habkost
Reviewed-by: Stefan Berger
---
Changes v1 -> v2:
* Removed unused variable at xen_block_set_vdev()
* Redone patch after changes in the previous patches in the
series
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-de...@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
---
backends/tpm/tpm_util.c | 6 --
hw/block/xen-block.c | 6 --
hw/core/qdev-properties-system.c | 70 --
hw/core/qdev-properties.c| 100 ++-
hw/s390x/css.c | 6 --
hw/s390x/s390-pci-bus.c | 6 --
hw/vfio/pci-quirks.c | 6 --
target/sparc/cpu.c | 6 --
8 files changed, 18 insertions(+), 188 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index dba2f6b04a..0b07cf55ea 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -46,16 +46,10 @@ static void get_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 905e4acd97..bd1aef63a7 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -395,17 +395,11 @@ static int vbd_name_to_disk(const char *name, const char
**endp,
static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str, *p;
const char *end;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 202abd0e4b..0d3e57bba0 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -94,11 +94,6 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
bool blk_created = false;
int ret;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -230,17 +225,11 @@ static void get_chr(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
CharBackend *be = qdev_get_prop_ptr(obj, prop);
Chardev *s;
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -311,18 +300,12 @@ static void get_mac(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
MACAddr *mac = qdev_get_prop_ptr(obj, prop);
int i, pos;
char *str;
const char *p;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -390,7 +373,6 @@ static void get_netdev(Object *obj, Visitor *v, const char
*name,
static void set_netdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
NICPeers *peers_ptr = qdev_get_prop_ptr(obj, prop);
NetClientState **ncs = peers_ptr->ncs;
@@ -398,11 +380,6
Re: [PATCH 09/36] qdev: Make qdev_get_prop_ptr() get Object* arg
On 10/29/20 6:02 PM, Eduardo Habkost wrote:
Make the code more generic and not specific to TYPE_DEVICE.
Signed-off-by: Eduardo Habkost
Reviewed-by: Stefan Berger
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: qemu-de...@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
backends/tpm/tpm_util.c | 8 ++--
hw/block/xen-block.c | 6 +--
hw/core/qdev-properties-system.c | 57 +-
hw/core/qdev-properties.c| 82 +---
hw/s390x/css.c | 5 +-
hw/s390x/s390-pci-bus.c | 4 +-
hw/vfio/pci-quirks.c | 5 +-
8 files changed, 68 insertions(+), 101 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0ea822e6a7..0b92cfc761 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -302,7 +302,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-void *qdev_get_prop_ptr(DeviceState *dev, Property *prop);
+void *qdev_get_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(DeviceState *dev,
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index b58d298c1a..e91c21dd4a 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -35,8 +35,7 @@
static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
-TPMBackend **be = qdev_get_prop_ptr(dev, opaque);
+TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
char *p;
p = g_strdup(*be ? (*be)->id : "");
@@ -49,7 +48,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-TPMBackend *s, **be = qdev_get_prop_ptr(dev, prop);
+TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
char *str;
if (dev->realized) {
@@ -73,9 +72,8 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void release_tpm(Object *obj, const char *name, void *opaque)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-TPMBackend **be = qdev_get_prop_ptr(dev, prop);
+TPMBackend **be = qdev_get_prop_ptr(obj, prop);
if (*be) {
tpm_backend_reset(*be);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 8a7a3f5452..1ba9981c08 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -335,9 +335,8 @@ static char *disk_to_vbd_name(unsigned int disk)
static void xen_block_get_vdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(dev, prop);
+XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str;
switch (vdev->type) {
@@ -396,9 +395,8 @@ static int vbd_name_to_disk(const char *name, const char
**endp,
static void xen_block_set_vdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(dev, prop);
+XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
char *str, *p;
const char *end;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index d0fb063a49..c8c73c371b 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -59,9 +59,8 @@ static bool check_prop_still_unset(DeviceState *dev, const
char *name,
static void get_drive(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(dev, prop);
+void **ptr = qdev_get_prop_ptr(obj, prop);
const char *value;
char *p;
@@ -87,7 +86,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(dev, prop);
+void **ptr = qdev_get_prop_ptr(obj, prop);
char *str;
BlockBackend *blk;
bool blk_created = false;
@@ -185,7 +184,7 @@ static void release_drive(
Re: [PATCH 14/36] qdev: Move dev->realized check to qdev_property_set()
On 10/29/20 6:02 PM, Eduardo Habkost wrote:
Every single qdev property setter function manually checks
dev->realized. We can just check dev->realized inside
qdev_property_set() instead.
The check is being added as a separate function
(qdev_prop_allow_set()) because it will become a callback later.
Signed-off-by: Eduardo Habkost
Reviewed-by: Stefan Berger
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Thomas Huth
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: Mark Cave-Ayland
Cc: Artyom Tarasenko
Cc: qemu-de...@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
---
backends/tpm/tpm_util.c | 6 --
hw/block/xen-block.c | 5 --
hw/core/qdev-properties-system.c | 64 ---
hw/core/qdev-properties.c| 106 ++-
hw/s390x/css.c | 6 --
hw/s390x/s390-pci-bus.c | 6 --
hw/vfio/pci-quirks.c | 6 --
target/sparc/cpu.c | 6 --
8 files changed, 18 insertions(+), 187 deletions(-)
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index e91c21dd4a..042cacfcca 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -46,16 +46,10 @@ static void get_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index 1ba9981c08..bd1aef63a7 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -400,11 +400,6 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
const char *name,
char *str, *p;
const char *end;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index fca1b694ca..60a45f5620 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -92,11 +92,6 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
bool blk_created = false;
int ret;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -228,17 +223,11 @@ static void get_chr(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_chr(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
CharBackend *be = qdev_get_prop_ptr(obj, prop);
Chardev *s;
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -309,18 +298,12 @@ static void get_mac(Object *obj, Visitor *v, const char
*name, void *opaque,
static void set_mac(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
MACAddr *mac = qdev_get_prop_ptr(obj, prop);
int i, pos;
char *str;
const char *p;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -388,7 +371,6 @@ static void get_netdev(Object *obj, Visitor *v, const char
*name,
static void set_netdev(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
-DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
NICPeers *peers_ptr = qdev_get_prop_ptr(obj, prop);
NetClientState **ncs = peers_ptr->ncs;
@@ -396,11 +378,6 @@ static void set_netdev(Object *obj, Visitor *v, const char
*name,
int queues, err = 0, i = 0;
char *str;
-if (dev->realized) {
-qdev_prop_set_after_realize(dev, name, errp);
-return;
-}
-
if (!visit_type_str(v, name, , errp)) {
return;
}
@@ -467,18 +444,12 @@ static void get_audiodev(Object *obj, Visitor *v, const
char* name,
static void set_audiode
Re: [PATCH 25/36] qdev: Rename qdev_get_prop_ptr() to object_static_prop_ptr()
On 10/29/20 6:02 PM, Eduardo Habkost wrote:
The function will be moved to common QOM code, as it is not
specific to TYPE_DEVICE anymore.
Signed-off-by: Eduardo Habkost
Reviewed-by: Stefan Berger
---
Cc: Stefan Berger
Cc: Stefano Stabellini
Cc: Anthony Perard
Cc: Paul Durrant
Cc: Kevin Wolf
Cc: Max Reitz
Cc: Paolo Bonzini
Cc: "Daniel P. Berrangé"
Cc: Eduardo Habkost
Cc: Cornelia Huck
Cc: Halil Pasic
Cc: Christian Borntraeger
Cc: Richard Henderson
Cc: David Hildenbrand
Cc: Thomas Huth
Cc: Matthew Rosato
Cc: Alex Williamson
Cc: qemu-de...@nongnu.org
Cc: xen-de...@lists.xenproject.org
Cc: qemu-block@nongnu.org
Cc: qemu-s3...@nongnu.org
---
include/hw/qdev-properties.h | 2 +-
backends/tpm/tpm_util.c | 6 +--
hw/block/xen-block.c | 4 +-
hw/core/qdev-properties-system.c | 46 +++
hw/core/qdev-properties.c| 64
hw/s390x/css.c | 4 +-
hw/s390x/s390-pci-bus.c | 4 +-
hw/vfio/pci-quirks.c | 4 +-
8 files changed, 67 insertions(+), 67 deletions(-)
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 0acc92ae2b..4146dac281 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -332,7 +332,7 @@ void qdev_prop_set_macaddr(DeviceState *dev, const char
*name,
const uint8_t *value);
void qdev_prop_set_enum(DeviceState *dev, const char *name, int value);
-void *qdev_get_prop_ptr(Object *obj, Property *prop);
+void *object_static_prop_ptr(Object *obj, Property *prop);
void qdev_prop_register_global(GlobalProperty *prop);
const GlobalProperty *qdev_find_global_prop(Object *obj,
diff --git a/backends/tpm/tpm_util.c b/backends/tpm/tpm_util.c
index 042cacfcca..2b5f788861 100644
--- a/backends/tpm/tpm_util.c
+++ b/backends/tpm/tpm_util.c
@@ -35,7 +35,7 @@
static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
Error **errp)
{
-TPMBackend **be = qdev_get_prop_ptr(obj, opaque);
+TPMBackend **be = object_static_prop_ptr(obj, opaque);
char *p;
p = g_strdup(*be ? (*be)->id : "");
@@ -47,7 +47,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-TPMBackend *s, **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend *s, **be = object_static_prop_ptr(obj, prop);
char *str;
if (!visit_type_str(v, name, , errp)) {
@@ -67,7 +67,7 @@ static void set_tpm(Object *obj, Visitor *v, const char
*name, void *opaque,
static void release_tpm(Object *obj, const char *name, void *opaque)
{
Property *prop = opaque;
-TPMBackend **be = qdev_get_prop_ptr(obj, prop);
+TPMBackend **be = object_static_prop_ptr(obj, prop);
if (*be) {
tpm_backend_reset(*be);
diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
index bd1aef63a7..20985c465a 100644
--- a/hw/block/xen-block.c
+++ b/hw/block/xen-block.c
@@ -336,7 +336,7 @@ static void xen_block_get_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_static_prop_ptr(obj, prop);
char *str;
switch (vdev->type) {
@@ -396,7 +396,7 @@ static void xen_block_set_vdev(Object *obj, Visitor *v,
const char *name,
void *opaque, Error **errp)
{
Property *prop = opaque;
-XenBlockVdev *vdev = qdev_get_prop_ptr(obj, prop);
+XenBlockVdev *vdev = object_static_prop_ptr(obj, prop);
char *str, *p;
const char *end;
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index d9355053d2..448d77ecab 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -60,7 +60,7 @@ static void get_drive(Object *obj, Visitor *v, const char
*name, void *opaque,
Error **errp)
{
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_static_prop_ptr(obj, prop);
const char *value;
char *p;
@@ -86,7 +86,7 @@ static void set_drive_helper(Object *obj, Visitor *v, const
char *name,
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-void **ptr = qdev_get_prop_ptr(obj, prop);
+void **ptr = object_static_prop_ptr(obj, prop);
char *str;
BlockBackend *blk;
bool blk_created = false;
@@ -179,7 +179,7 @@ static void release_drive(Object *obj, const char *name,
void *opaque)
{
DeviceState *dev = DEVICE(obj);
Property *prop = opaque;
-BlockBackend **ptr = qdev_get_prop_ptr(obj, prop);
+BlockBackend **ptr = object_static_prop_ptr(obj, prop);
if (*ptr) {
AioContext *ctx = blk_get_ai
Re: [PATCH 04/10] hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition
On 10/13/20 4:26 AM, Philippe Mathieu-Daudé wrote: On 10/13/20 9:20 AM, Gerd Hoffmann wrote: On Sun, Oct 11, 2020 at 09:32:23PM +0200, Philippe Mathieu-Daudé wrote: The TPM TIS device uses IRQ #5 by default. Add this default definition to the IsaIrqNumber enum. IRQ 5 has no fixed assignment. All kinds of add-on isa cards (sound, net) used to use irq #5 by default because that one wasn't assigned otherwise. Seems these days tpm and ipmi use it ... Yes, I'll drop this patch. I think this patch is good but maybe the name should be a different one. Rather than having these numbers in the code you could maybe call it something like this here, which makes grepping through the code a bit easier: ISA_IRQ_IRQ5 = 5, Regards, Stefan Thanks, Phil.
Re: [PATCH 08/10] hw/isa: Add the ISA_IRQ_NET_DEFAULT definition
On 10/13/20 3:23 AM, Gerd Hoffmann wrote: On Sun, Oct 11, 2020 at 09:32:27PM +0200, Philippe Mathieu-Daudé wrote: The network devices use IRQ #9 by default. Add this default definition to the IsaIrqNumber enum. IRQ #9 seems to be sort-of standard for acpi. Not sure whenever that is actually written down somewhere. IIRC in pre-ACPI days it was free like irq #5. Best docu I could find was this one: https://en.wikipedia.org/wiki/Interrupt_request_(PC_architecture)#Master_PIC take care, Gerd
Re: [PATCH 04/10] hw/isa: Add the ISA_IRQ_TPM_DEFAULT definition
On 10/11/20 3:32 PM, Philippe Mathieu-Daudé wrote:
The TPM TIS device uses IRQ #5 by default. Add this
default definition to the IsaIrqNumber enum.
Avoid magic values in the code, replace them by the
newly introduced definition.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/isa/isa.h | 1 +
hw/i386/acpi-build.c | 2 +-
hw/ipmi/isa_ipmi_bt.c | 2 +-
hw/ipmi/isa_ipmi_kcs.c | 2 +-
hw/tpm/tpm_tis_isa.c | 2 +-
5 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
index 519296d5823..e4f2aed004f 100644
--- a/include/hw/isa/isa.h
+++ b/include/hw/isa/isa.h
@@ -11,6 +11,7 @@
enum IsaIrqNumber {
ISA_IRQ_KBD_DEFAULT = 1,
ISA_IRQ_SER_DEFAULT = 4,
+ISA_IRQ_TPM_DEFAULT = 5,
ISA_NUM_IRQS= 16
};
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 45ad2f95334..2b6038ab015 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1886,7 +1886,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
Rewrite to take IRQ from TPM device model and
fix default IRQ value there to use some unused IRQ
*/
-/* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
+/* aml_append(crs, aml_irq_no_flags(ISA_IRQ_TPM_DEFAULT)); */
aml_append(dev, aml_name_decl("_CRS", crs));
tpm_build_ppi_acpi(tpm, dev);
diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index b7c2ad557b2..13a92bd2c44 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -137,7 +137,7 @@ static void *isa_ipmi_bt_get_backend_data(IPMIInterface *ii)
static Property ipmi_isa_properties[] = {
DEFINE_PROP_UINT32("ioport", ISAIPMIBTDevice, bt.io_base, 0xe4),
-DEFINE_PROP_INT32("irq", ISAIPMIBTDevice, isairq, 5),
+DEFINE_PROP_INT32("irq", ISAIPMIBTDevice, isairq, ISA_IRQ_TPM_DEFAULT),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index 7dd6bf0040a..c956b539688 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -144,7 +144,7 @@ static void *isa_ipmi_kcs_get_backend_data(IPMIInterface
*ii)
static Property ipmi_isa_properties[] = {
DEFINE_PROP_UINT32("ioport", ISAIPMIKCSDevice, kcs.io_base, 0xca2),
-DEFINE_PROP_INT32("irq", ISAIPMIKCSDevice, isairq, 5),
+DEFINE_PROP_INT32("irq", ISAIPMIKCSDevice, isairq, ISA_IRQ_TPM_DEFAULT),
DEFINE_PROP_END_OF_LIST(),
};
I don't know what these devices do but this looks like an unwanted clash.
diff --git a/hw/tpm/tpm_tis_isa.c b/hw/tpm/tpm_tis_isa.c
index 6fd876eebf1..5a4afda42df 100644
--- a/hw/tpm/tpm_tis_isa.c
+++ b/hw/tpm/tpm_tis_isa.c
@@ -91,7 +91,7 @@ static void tpm_tis_isa_reset(DeviceState *dev)
}
static Property tpm_tis_isa_properties[] = {
-DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, TPM_TIS_IRQ),
+DEFINE_PROP_UINT32("irq", TPMStateISA, state.irq_num, ISA_IRQ_TPM_DEFAULT),
DEFINE_PROP_TPMBE("tpmdev", TPMStateISA, state.be_driver),
DEFINE_PROP_BOOL("ppi", TPMStateISA, state.ppi_enabled, true),
DEFINE_PROP_END_OF_LIST(),
Re: [PATCH v3 08/19] tests/iotests: be a little more forgiving on the size test
On 2/25/20 7:46 AM, Alex Bennée wrote:
At least on ZFS this was failing as 512 was less than or equal to 512.
I suspect the reason is additional compression done by ZFS and however
qemu-img gets the actual size.
Loosen the criteria to make sure after is not bigger than before and
also dump the values in the report.
Signed-off-by: Alex Bennée
---
tests/qemu-iotests/214 | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/214 b/tests/qemu-iotests/214
index 3500e0c47a2..6d1324cd157 100755
--- a/tests/qemu-iotests/214
+++ b/tests/qemu-iotests/214
@@ -125,9 +125,9 @@ $QEMU_IO -c "write -P 0xcc $offset $data_size" "json:{\
sizeB=$($QEMU_IMG info --output=json "$TEST_IMG" |
sed -n '/"actual-size":/ s/[^0-9]//gp')
-if [ $sizeA -le $sizeB ]
+if [ $sizeA -lt $sizeB ]
then
-echo "Compression ERROR"
+echo "Compression ERROR ($sizeA vs $sizeB)"
fi
Nit: $sizeA < $sizeB ?
Reviewed-by: Stefan Berger
$QEMU_IMG check --output=json "$TEST_IMG" |
