On Fri, Sep 15, 2023 at 06:04:16PM +0100, Jonathan Cameron wrote:
> Establishing that only register accesses of size 4 and 8 can occur
> using these functions requires looking at their callers. Make it
> easier to see that by using switch statements.
> Assertions are used to enforce that the
On Fri, Sep 15, 2023 at 06:04:15PM +0100, Jonathan Cameron wrote:
> Bring this read function inline with the others that do
> check for unexpected size values.
>
> Also reduces line lengths to sub 80 chars.
>
> Signed-off-by: Jonathan Cameron
> Reviewed-by: Philippe Mathieu-Daudé
> ---
virtio-blk and virtio-scsi devices will need a way to specify the
mapping between IOThreads and virtqueues. At the moment all virtqueues
are assigned to a single IOThread or the main loop. This single thread
can be a CPU bottleneck, so it is necessary to allow finer-grained
assignment to spread
Add the iothread-vq-mapping parameter to assign virtqueues to IOThreads.
Store the vq:AioContext mapping in the new struct
VirtIOBlockDataPlane->vq_aio_context[] field and refactor the code to
use the per-vq AioContext instead of the BlockDriverState's AioContext.
Reimplement --device
virtio-blk and virtio-scsi devices need a way to specify the mapping between
IOThreads and virtqueues. At the moment all virtqueues are assigned to a single
IOThread or the main loop. This single thread can be a CPU bottleneck, so it is
necessary to allow finer-grained assignment to spread the
On Mon, Sep 18, 2023 at 04:02:59PM +0100, Jonathan Cameron wrote:
> These crossed with the previous fix to get rid of examples
> using aarch64 for which support is not yet upstream.
>
> Signed-off-by: Jonathan Cameron
Reviewed-by: Fan Ni
> ---
> docs/system/devices/cxl.rst | 4 ++--
> 1
On Mon, Sep 18, 2023 at 04:02:58PM +0100, Jonathan Cameron wrote:
> This has been missing from the start. Assume it should match
> with cxl/cxl-component-utils.c as both were part of early
> postings from Ben.
>
> Suggested-by: Philippe Mathieu-Daudé
> Signed-off-by: Jonathan Cameron
> ---
On Mon, Sep 18, 2023 at 04:02:57PM +0100, Jonathan Cameron wrote:
> From: Dmitry Frolov
>
> According to cxl_interleave_ways_enc(), fw->num_targets is allowed to be up
> to 16. This also corresponds to CXL r3.0 spec. So, the fw->target_hbs[]
> array is iterated from 0 to 15. But it is staticaly
Architecture specific hardware doesn't have a particular dependency
on the accelerator vCPU (created with cpu_exec_realizefn), and can
be initialized *after* the vCPU is realized. Doing so allows further
generic API simplification (in few commits).
Signed-off-by: Philippe Mathieu-Daudé
---
Architecture specific hardware doesn't have a particular dependency
on the accelerator vCPU (created with cpu_exec_realizefn), and can
be initialized *after* the vCPU is realized. Doing so allows further
generic API simplification (in few commits).
Signed-off-by: Philippe Mathieu-Daudé
---
When looking at the arm_cpu_realizefn() method, most of the
code run before the cpu_exec_realizefn() call checks whether
the requested CPU features are compatible with the requested
accelerator. Extract this code to a dedicated handler matching
our recently added CPUClass::verify_accel_features()
Architecture specific hardware doesn't have a particular dependency
on the accelerator vCPU (created with cpu_exec_realizefn), and can
be initialized *after* the vCPU is realized. Doing so allows further
generic API simplification (in few commits).
Signed-off-by: Philippe Mathieu-Daudé
---
Some CPUs need to check the requested features are compatible
with the requested accelerator. This has to be done *before*
the accelerator realizes a vCPU.
Introduce the verify_accel_features() handler and call it
just before accel_cpu_realizefn().
Signed-off-by: Philippe Mathieu-Daudé
---
Architecture specific hardware doesn't have a particular dependency
on the accelerator vCPU (created with cpu_exec_realizefn), and can
be initialized *after* the vCPU is realized. Doing so allows further
generic API simplification (in few commits).
Signed-off-by: Philippe Mathieu-Daudé
---
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have s390_realize_cpu_model()
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
---
target/s390x/s390x-internal.h | 2 +-
target/s390x/cpu.c
When looking at the x86_cpu_realizefn() method, most of the
code run before the cpu_exec_realizefn() call checks whether
the requested CPU features are compatible with the requested
accelerator. Extract this code to a dedicated handler matching
our recently added CPUClass::verify_accel_features()
We create at least one vCPU address space by default in
qemu_init_vcpu(), itself called in cpu_common_realizefn().
Since we don't have qemu_deinit_vcpu() helper (we probably
don't need any), simply destroy all the address spaces in
cpu_common_unrealizefn(), *after* the thread is destroyed.
Note:
cpu_exec_realizefn() is called in each ${target}_cpu_realize(),
before calling their parent_realize(), which is simply
cpu_common_realizefn(). Directly call it there instead.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/core/cpu-common.c | 4
target/alpha/cpu.c| 8
Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have cpu_exec_realizefn()
return a boolean indicating whether an error is set or not.
Signed-off-by: Philippe Mathieu-Daudé
---
include/hw/core/cpu.h | 2 +-
cpu.c | 6 --
2
APIC state is created under a certain condition,
use the same condition to realize it.
Having a NULL APIC state is a bug: use assert().
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/cpu-sysemu.c | 9 +++--
target/i386/cpu.c| 8 +---
2 files changed, 8 insertions(+), 9
From: xianglai li
Introduce new function to destroy CPU address space resources
for cpu hot-(un)plug.
Co-authored-by: "Salil Mehta"
Cc: "Salil Mehta"
Cc: Xiaojuan Yang
Cc: Song Gao
Cc: "Michael S. Tsirkin"
Cc: Igor Mammedov
Cc: Ani Sinha
Cc: Paolo Bonzini
Cc: Richard Henderson
Cc:
s390_realize_cpu_model() checks if CPU model and definitions
are compatible with the KVM / TCG accelerators, before realizing
the vCPU. Use it directly as CPUClass::verify_accel_features()
handler (called from cpu_exec_realizefn()).
Signed-off-by: Philippe Mathieu-Daudé
---
target/s390x/cpu.c |
While create_vcpu_thread() creates a vCPU thread, its counterpart
is cpu_remove_sync(), which join and destroy the thread.
create_vcpu_thread() is called in qemu_init_vcpu(), itself called
in cpu_common_realizefn(). Since we don't have qemu_deinit_vcpu()
helper (we probably don't need any),
Architecture specific hardware doesn't have a particular dependency
on the accelerator vCPU (created with cpu_exec_realizefn), and can
be initialized *after* the vCPU is realized. Doing so allows further
generic API simplification (in few commits).
Signed-off-by: Philippe Mathieu-Daudé
---
These fields from the environment don't affect how accelerators
create their vCPU thread. We can safely reorder them *after* the
cpu_exec_realizefn() call. Doing so allows further generic API
simplification (in the next commit).
Signed-off-by: Philippe Mathieu-Daudé
---
target/sparc/cpu.c | 12
s390_cpu_realize_sysemu() runs some checks for the TCG accelerator,
previous to creating the vCPU. s390_realize_cpu_model() also does
run some checks for KVM.
Move the sysemu call to s390_realize_cpu_model(). Having a single
call before cpu_exec_realizefn() will allow us to factor a
QOM objects shouldn't access each other internals fields
except using the QOM API.
Declare the 'cpu' and 'base-addr' properties, set them
using object_property_set_link() and qdev_prop_set_uint32()
respectively.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/intc/apic_common.c| 2 ++
qemu_init_vcpu() is called in each ${target}_cpu_realize() before
the call to parent_realize(), which is cpu_common_realizefn().
Call it once there.
Signed-off-by: Philippe Mathieu-Daudé
---
hw/core/cpu-common.c| 3 +++
target/alpha/cpu.c | 2 --
target/arm/cpu.c| 2 --
QDev instance is expected to be in an unknown state until full
object realization. Thus we shouldn't call DeviceReset() on an
unrealized instance. Move the cpu_reset() call from *before*
the parent realize() handler (effectively cpu_common_realizefn)
to *after* it.
Signed-off-by: Philippe
Signed-off-by: Philippe Mathieu-Daudé
---
target/i386/kvm/kvm-cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
index 7237378a7d..1fe62ce176 100644
--- a/target/i386/kvm/kvm-cpu.c
+++ b/target/i386/kvm/kvm-cpu.c
@@ -37,6 +37,7 @@
Hi,
TL;DR: This series factor duplicated common code in CPUs
DeviceRealize() handlers out, moving as a single call in
cpu_common_realize().
In an effort to have most of:
- CPU core code independant of accelerators
- CPU core code target agnostic
- CPU target code independant of accelerators
-
On 18.09.23 17:56, Ani Sinha wrote:
On Mon, Sep 18, 2023 at 8:59 PM David Hildenbrand wrote:
On 18.09.23 17:22, Ani Sinha wrote:
On Mon, Sep 18, 2023 at 7:25 PM Ani Sinha wrote:
32-bit systems do not have a reserved memory for hole64 but they may have a
reserved memory space for memory
On Mon, Sep 18, 2023 at 8:59 PM David Hildenbrand wrote:
>
> On 18.09.23 17:22, Ani Sinha wrote:
> > On Mon, Sep 18, 2023 at 7:25 PM Ani Sinha wrote:
> >>
> >> 32-bit systems do not have a reserved memory for hole64 but they may have a
> >> reserved memory space for memory hotplug. Since, hole64
On Mon, Sep 18, 2023 at 11:15:57AM -0300, Fabiano Rosas wrote:
> This fixes the issue. It looks ok to me. Do you want me to send a separate
> patch with it?
I've consulted Stefan on how to proceed. I think it's wanted that we have
these fixes in first even before Juan's back (note: Juan will be
On 18/9/23 16:58, Cédric Le Goater wrote:
Rename 'name' variable to avoid this warning :
../hw/ppc/spapr_drc.c: In function ‘prop_get_fdt’:
../hw/ppc/spapr_drc.c:344:21: warning: declaration of ‘name’ shadows a
parameter [-Wshadow=compatible-local]
344 | const char *name =
On 18/9/23 16:58, Cédric Le Goater wrote:
Rename PCIDevice variable to avoid this warning :
../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a
previous local [-Wshadow=compatible-local]
3217 |
On 18.09.23 17:22, Ani Sinha wrote:
On Mon, Sep 18, 2023 at 7:25 PM Ani Sinha wrote:
32-bit systems do not have a reserved memory for hole64 but they may have a
reserved memory space for memory hotplug. Since, hole64 starts after the
reserved hotplug memory, the unaligned hole64 start address
Stefan Hajnoczi writes:
> On Fri, 15 Sept 2023 at 11:10, Alex Bennée wrote:
>>
>> The following changes since commit 005ad32358f12fe9313a4a01918a55e60d4f39e5:
>>
>> Merge tag 'pull-tpm-2023-09-12-3' of
>> https://github.com/stefanberger/qemu-tpm into staging (2023-09-13 13:41:57
>> -0400)
On 18/9/23 13:00, Peter Maydell wrote:
On Mon, 18 Sept 2023 at 11:17, Philippe Mathieu-Daudé wrote:
We shouldn't call QDev DeviceReset() before DeviceRealize().
Since the OMAP MMC model is not QDev'ified, it has to manually
call the SDCard reset() handler. This breaks QDev assumptions
that
On Mon, Sep 18, 2023 at 7:25 PM Ani Sinha wrote:
>
> 32-bit systems do not have a reserved memory for hole64 but they may have a
> reserved memory space for memory hotplug. Since, hole64 starts after the
> reserved hotplug memory, the unaligned hole64 start address gives us the
> end address for
On 9/15/23 20:41, Denis V. Lunev wrote:
This patch creates above mentioned helper and moves its usage to the
beginning of parallels_open(). This simplifies parallels_open() a bit.
The patch also ensures that we store prealloc_size on block driver state
always in sectors. This makes code cleaner
On 9/15/23 20:41, Denis V. Lunev wrote:
This patch contains test which minimally tests write-zeroes on top of
working discard.
The following checks are added:
* write 2 clusters, write-zero to the first allocated cluster
* write 2 cluster, write-zero to the half the first allocated cluster
On Mon, Sep 18, 2023 at 04:54:22AM -0600, Daniel Xu wrote:
> Currently, commands run through guest-exec are "silent" until they
> finish running. This is fine for short lived commands. But for commands
> that take a while, this is a bad user experience.
>
> Usually long running programs know that
On 9/15/23 20:41, Denis V. Lunev wrote:
The zero flag is missed in the Parallels format specification. We can
resort to discard if we have no backing file.
Signed-off-by: Denis V. Lunev
---
block/parallels.c | 14 ++
1 file changed, 14 insertions(+)
diff --git
On 9/15/23 20:41, Denis V. Lunev wrote:
This patch contains test which minimally tests discard and new cluster
allocation logic.
The following checks are added:
* write 2 clusters, discard the first allocated
* write another cluster, check that the hole is filled
* write 2 clusters, discard the
Daniel Xu writes:
> Currently, commands run through guest-exec are "silent" until they
> finish running. This is fine for short lived commands. But for commands
> that take a while, this is a bad user experience.
>
> Usually long running programs know that they will run for a while. To
> improve
These crossed with the previous fix to get rid of examples
using aarch64 for which support is not yet upstream.
Signed-off-by: Jonathan Cameron
---
docs/system/devices/cxl.rst | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/docs/system/devices/cxl.rst
This has been missing from the start. Assume it should match
with cxl/cxl-component-utils.c as both were part of early
postings from Ben.
Suggested-by: Philippe Mathieu-Daudé
Signed-off-by: Jonathan Cameron
---
hw/mem/cxl_type3.c | 9 +
1 file changed, 9 insertions(+)
diff --git
Hi Kevin,
The following CI failure looks like it is related to this pull
request. Please take a look:
https://gitlab.com/qemu-project/qemu/-/jobs/5112083994
▶ 823/840 qcow2 iothreads-commit-active FAIL
823/840 qemu:block / io-qcow2-iothreads-commit-active ERROR 6.16s exit status 1
>>>
On Mon, 18 Sep 2023 13:41:20 +0100
Peter Maydell wrote:
> On Mon, 19 Jun 2023 at 10:58, Jonathan Cameron via
> wrote:
> >
> > On Fri, 16 Jun 2023 14:10:24 -0400
> > Gregory Price wrote:
> > >
> > > Last I tested cxl-2023-05-25 branch of Johnathan's fork is working on x86:
> > >
> > >
Misc set of trivial fixes. No conflicts with other sets outstanding
so can go with main CXL patches or perhaps via the trivial tree.
Dmitry Frolov (1):
hw/cxl: Fix out of bound array access
Jonathan Cameron (2):
hw/mem/cxl_type3: Add missing copyright and license notice
docs/cxl: Cleanout
From: Dmitry Frolov
According to cxl_interleave_ways_enc(), fw->num_targets is allowed to be up
to 16. This also corresponds to CXL r3.0 spec. So, the fw->target_hbs[]
array is iterated from 0 to 15. But it is staticaly declared of length 8.
Thus, out of bound array access may occur.
Fixes:
Hello,
Here is a first round of cleanups for local variable shadowing
warrnings.
Thanks,
C.
Cédric Le Goater (8):
hw/ppc: Clean up local variable shadowing in _FDT helper routine
pnv/psi: Clean up local variable shadowing
spapr: Clean up local variable shadowing in spapr_dt_cpus()
Rename 'name' variable to avoid this warning :
../hw/ppc/spapr_drc.c: In function ‘prop_get_fdt’:
../hw/ppc/spapr_drc.c:344:21: warning: declaration of ‘name’ shadows a
parameter [-Wshadow=compatible-local]
344 | const char *name = NULL;
| ^~~~
Introduce a helper routine defining one CPU device node to fix this
warning :
../hw/ppc/spapr.c: In function ‘spapr_dt_cpus’:
../hw/ppc/spapr.c:812:19: warning: declaration of ‘cs’ shadows a previous
local [-Wshadow=compatible-local]
812 | CPUState *cs = rev[i];
|
Remove extra 'i' variable to fix this warning :
../hw/ppc/spapr.c: In function ‘spapr_init_cpus’:
../hw/ppc/spapr.c:2668:13: warning: declaration of ‘i’ shadows a previous
local [-Wshadow=compatible-local]
2668 | int i;
| ^
../hw/ppc/spapr.c:2645:9: note:
Rename PCIDevice variable to avoid this warning :
../hw/ppc/spapr.c: In function ‘spapr_get_fw_dev_path’:
../hw/ppc/spapr.c:3217:20: warning: declaration of ‘pcidev’ shadows a
previous local [-Wshadow=compatible-local]
3217 | PCIDevice *pcidev = CAST(PCIDevice, dev,
Rename SysBusDevice variable to avoid this warning :
../hw/ppc/spapr_pci.c: In function ‘spapr_phb_realize’:
../hw/ppc/spapr_pci.c:1872:24: warning: declaration of ‘s’ shadows a previous
local [-Wshadow=local]
1872 | SpaprPhbState *s;
|^
this fixes numerous warnings of this type :
In file included from ../hw/ppc/spapr_pci.c:43:
../hw/ppc/spapr_pci.c: In function ‘spapr_dt_phb’:
../include/hw/ppc/fdt.h:18:13: warning: declaration of ‘ret’ shadows a
previous local [-Wshadow=compatible-local]
18 | int ret =
to fix :
../hw/ppc/pnv_psi.c: In function ‘pnv_psi_p9_mmio_write’:
../hw/ppc/pnv_psi.c:741:24: warning: declaration of ‘addr’ shadows a
parameter [-Wshadow=compatible-local]
741 | hwaddr addr = val & ~(PSIHB9_ESB_CI_VALID |
PSIHB10_ESB_CI_64K);
|
Remove extra 'drc_index' variable to avoid this warning :
../hw/ppc/spapr_drc.c: In function ‘rtas_ibm_configure_connector’:
../hw/ppc/spapr_drc.c:1240:26: warning: declaration of ‘drc_index’ shadows a
previous local [-Wshadow=compatible-local]
1240 | uint32_t drc_index =
Kevin Wolf writes:
> Am 31.08.2023 um 15:25 hat Markus Armbruster geschrieben:
>> Local variables shadowing other local variables or parameters make the
>> code needlessly hard to understand. Tracked down with -Wshadow=local.
>> Clean up: delete inner declarations when they are actually
Markus Armbruster 于2023年9月18日周一 22:46写道：
>
> Sam Li writes:
>
> > Markus Armbruster 于2023年9月1日周五 19:08写道：
> >>
> >> Sam Li writes:
> >>
> >> > To configure the zoned format feature on the qcow2 driver, it
> >> > requires following arguments: the device size, zoned profile,
> >>
> >> "Zoned
rdma_add_block() can't fail. Return void, and drop the unreachable
error handling.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 30 +-
1 file changed, 9 insertions(+), 21 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index
This is just to make the error value more obvious. Callers don't
mind.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index
The QEMUFileHooks methods don't come with a written contract. Digging
through the code calling them, we find:
* save_page():
Negative values RAM_SAVE_CONTROL_DELAYED and
RAM_SAVE_CONTROL_NOT_SUPP are special. Any other negative value is
an unspecified error.
qemu_rdma_save_page()
These guards are all redundant now.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 164 +++
1 file changed, 51 insertions(+), 113 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index d29affe410..c88cd1f468 100644
---
rdma_delete_block() always returns 0, which its only caller ignores.
Return void instead.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index 3b9e21f8de..320c291a96 100644
---
Kevin Wolf writes:
> Am 31.08.2023 um 15:25 hat Markus Armbruster geschrieben:
>> Local variables shadowing other local variables or parameters make the
>> code needlessly hard to understand. Tracked down with -Wshadow=local.
>> Clean up: delete inner declarations when they are actually
Several error messages include numeric error codes returned by failed
functions:
* ibv_poll_cq() returns an unspecified negative value. Useless.
* rdma_accept and rmda_get_cm_event() return -1. Useless.
* qemu_rdma_poll() returns either -1 or an unspecified negative
value. Useless.
*
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
qemu_rdma_accept() returns 0 in some cases even when it didn't
complete its job due to errors. Impact is not obvious. I figure the
caller will soon fail again with a misleading error message.
Fix it to return -1 on any failure.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 19
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
qemu_rdma_dump_id() dumps RDMA device details to stdout.
rdma_start_outgoing_migration() calls it via qemu_rdma_source_init()
and qemu_rdma_resolve_host() to show source device details.
rdma_start_incoming_migration() arranges its call via
rdma_accept_incoming_migration() and qemu_rdma_accept()
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
qemu_rdma_search_ram_block() can't fail. Return void, and drop the
unreachable error handling.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
error_report() obeys -msg, reports the current error location if any,
and reports to the current monitor if any. Reporting to stderr
directly with fprintf() or perror() is wrong, because it loses all
this.
Fix the offenders.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 39
qemu_rdma_exchange_get_response() compares int parameter @expecting
with uint32_t head->type. Actual arguments are non-negative
enumeration constants, RDMAControlHeader uint32_t member type, or
qemu_rdma_exchange_recv() int parameter expecting. Actual arguments
for the latter are non-negative
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index e3b8d0506c..177d73a2ba 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3057,7 +3057,7 @@
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
QIOChannelClass methods qio_channel_rdma_readv() and
qio_channel_rdma_writev() violate their method contract when
rdma->error_state is non-zero:
1. They return whatever is in rdma->error_state then. Only -1 will be
fine. -2 will be misinterpreted as "would block". Anything less
than -2
rdma_getaddrinfo() returns 0 on success. On error, it returns one of
the EAI_ error codes like getaddrinfo() does, or -1 with errno set.
This is broken by design: POSIX implicitly specifies the EAI_ error
codes to be non-zero, no more. They could clash with -1. Nothing we
can do about this
qemu_rdma_buffer_mergable() is semantically a predicate. It returns
int 0 or 1. Return bool instead.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index
All we do with the value of RDMAContext member @error_state is test
whether it's zero. Change to bool and rename to @errored.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 66
1 file changed, 33 insertions(+), 33 deletions(-)
diff
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 19 +++
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git
qemu_rdma_resolve_host() and qemu_rdma_dest_init() iterate over
addresses to find one that works, holding onto the first Error from
qemu_rdma_broken_ipv6_kernel() for use when no address works. Issues:
1. If @errp was _abort or _fatal, we'd terminate instead
of trying the next address. Can't
Just for symmetry with qemu_rdma_post_send_control(). Error messages
lose detail I consider of no use to users.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
include/qapi/error.h demands:
* - Functions that use Error to report errors have an Error **errp
* parameter. It should be the last parameter, except for functions
* taking variable arguments.
qemu_rdma_connect() does not conform. Clean it up.
Signed-off-by: Markus Armbruster
---
wrid_desc[] uses 4001 pointers to map four integer values to strings.
print_wrid() accesses wrid_desc[] out of bounds when passed a negative
argument. It returns null for values 2..1999 and 2001..3999.
qemu_rdma_poll() and qemu_rdma_block_for_wrid() print wrid_desc[wr_id]
and passes
qemu_rdma_wait_comp_channel() returns 0 on success, and either -1 or
rdma->error_state on failure. Callers actually expect a negative
error value. I believe rdma->error_state can't be positive, but let's
make things more obvious by simply returning -1 on any failure.
Signed-off-by: Markus
Just for consistency with qemu_rdma_write_one() and
qemu_rdma_write_flush(), and for slightly simpler code.
Signed-off-by: Markus Armbruster
---
migration/rdma.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/migration/rdma.c b/migration/rdma.c
index
qemu_get_cm_event_timeout() neglects to set an error when it fails
because rdma_get_cm_event() fails. Harmless, as its caller
qemu_rdma_connect() substitutes a generic error then. Fix it anyway.
qemu_rdma_connect() also sets the generic error when its own call of
rdma_get_cm_event() fails.
Functions that use an Error **errp parameter to return errors should
not also report them to the user, because reporting is the caller's
job. When the caller does, the error is reported twice. When it
doesn't (because it recovered from the error), there is no error to
report, i.e. the report is
Several functions return negative errno codes on failure. Callers
check for specific codes exactly never. For some of the functions,
callers couldn't check even if they wanted to, because the functions
also return negative values that aren't errno codes, leaving readers
confused on what the
We use int instead of uint64_t in a few places. Change them to
uint64_t.
This cleans up a comparison of signed qemu_rdma_block_for_wrid()
parameter @wrid_requested with unsigned @wr_id. Harmless, because the
actual arguments are non-negative enumeration constants.
Signed-off-by: Markus
Sam Li writes:
> Markus Armbruster 于2023年9月1日周五 19:08写道：
>>
>> Sam Li writes:
>>
>> > To configure the zoned format feature on the qcow2 driver, it
>> > requires following arguments: the device size, zoned profile,
>>
>> "Zoned profile" is gone in v3.
>>
>> > zone model, zone size, zone
101 - 200 of 391 matches
Mail list logo
