Re: [PULL 00/18] Block patches

On 23.06.20 14:55, Peter Maydell wrote: > On Mon, 22 Jun 2020 at 16:11, Max Reitz wrote: >> >> The following changes since commit bae31bfa48b9caecee25da3d5333901a126a06b4: >> >> Merge remote-tracking branch >> 'remotes/kraxel/tags/audio-20200619-pull-request' into staging (2020-06-19 >> 22:56:

[PULL v2 0/2] Block patches

The following changes since commit d88d5a3806d78dcfca648c62dae9d88d3e803bd2: Merge remote-tracking branch 'remotes/philmd-gitlab/tags/renesas-hw-20200622' into staging (2020-06-23 13:55:52 +0100) are available in the Git repository at: https://github.com/XanClic/qemu.git tags/pull-block-202

[PULL v2 1/2] iotests: Fix 051 output after qdev_init_nofail() removal

From: Philippe Mathieu-Daudé Commit 96927c744 replaced qdev_init_nofail() call by isa_realize_and_unref() which has a different error message. Update the test output accordingly. Gitlab CI error after merging b77b5b3dc7: https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375 Reported-by: T

[PULL v2 2/2] iotests: don't test qcow2.py inside 291

From: Vladimir Sementsov-Ogievskiy 820c6bee534ec3b added testing of qcow2.py into 291, and it breaks 291 with external data file. Actually, 291 is bad place for qcow2.py testing, better add a separate test. For now, drop qcow2.py testing from 291 to fix the regression. Fixes: 820c6bee534ec3b Re

Re: [PULL 00/18] Block patches

On 24/06/2020 09.27, Max Reitz wrote: On 23.06.20 14:55, Peter Maydell wrote: On Mon, 22 Jun 2020 at 16:11, Max Reitz wrote: The following changes since commit bae31bfa48b9caecee25da3d5333901a126a06b4: Merge remote-tracking branch 'remotes/kraxel/tags/audio-20200619-pull-request' into st

[PULL 00/12] Block patches

The following changes since commit 171199f56f5f9bdf1e5d670d09ef1351d8f01bae: Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200619-3' into staging (2020-06-22 14:45:25 +0100) are available in the Git repository at: https://github.com/stefanha/qemu.git tags/block-

[PULL 03/12] coroutine: add check for SafeStack in sigaltstack

From: Daniele Buono Current implementation of LLVM's SafeStack is not compatible with code that uses an alternate stack created with sigaltstack(). Since coroutine-sigaltstack relies on sigaltstack(), it is not compatible with SafeStack. The resulting binary is incorrect, with different coroutine

[PULL 04/12] configure: add flags to support SafeStack

From: Daniele Buono This patch adds a flag to enable/disable the SafeStack instrumentation provided by LLVM. On enable, make sure that the compiler supports the flags, and that we are using the proper coroutine implementation (coroutine-ucontext). On disable, explicitly disable the option if it

[PULL 01/12] minikconf: explicitly set encoding to UTF-8

QEMU currently only has ASCII Kconfig files but Linux actually uses UTF-8. Explicitly specify the encoding and that we're doing text file I/O. It's unclear whether or not QEMU will ever need Unicode in its Kconfig files. If we start using the help text then it will become an issue sooner or later.

[PULL 02/12] coroutine: support SafeStack in ucontext backend

From: Daniele Buono LLVM's SafeStack instrumentation does not yet support programs that make use of the APIs in ucontext.h With the current implementation of coroutine-ucontext, the resulting binary is incorrect, with different coroutines sharing the same unsafe stack and producing undefined beha

[PULL 05/12] check-block: enable iotests with SafeStack

From: Daniele Buono SafeStack is a stack protection technique implemented in llvm. It is enabled with a -fsanitize flag. iotests are currently disabled when any -fsanitize option is used, because such options tend to produce additional warnings and false positives. While common -fsanitize option

[PULL 06/12] block/nvme: poll queues without q->lock

A lot of CPU time is spent simply locking/unlocking q->lock during polling. Check for completion outside the lock to make q->lock disappear from the profile. Signed-off-by: Stefan Hajnoczi Reviewed-by: Sergio Lopez Message-id: 20200617132201.1832152-2-stefa...@redhat.com Signed-off-by: Stefan Ha

[PULL 08/12] block/nvme: don't access CQE after moving cq.head

Do not access a CQE after incrementing q->cq.head and releasing q->lock. It is unlikely that this causes problems in practice but it's a latent bug. The reason why it should be safe at the moment is that completion processing is not re-entrant and the CQ doorbell isn't written until the end of nvm

[PULL 09/12] block/nvme: switch to a NVMeRequest freelist

There are three issues with the current NVMeRequest->busy field: 1. The busy field is accidentally accessed outside q->lock when request submission fails. 2. Waiters on free_req_queue are not woken when a request is returned early due to submission failure. 2. Finding a free request involves

[PULL 07/12] block/nvme: drop tautologous assertion

nvme_process_completion() explicitly checks cid so the assertion that follows is always true: if (cid == 0 || cid > NVME_QUEUE_SIZE) { ... continue; } assert(cid <= NVME_QUEUE_SIZE); Signed-off-by: Stefan Hajnoczi Reviewed-by: Sergio Lopez Reviewed-by: Philippe Mathieu-Daudé

[PULL 10/12] block/nvme: clarify that free_req_queue is protected by q->lock

Existing users access free_req_queue under q->lock. Document this. Signed-off-by: Stefan Hajnoczi Reviewed-by: Sergio Lopez Reviewed-by: Philippe Mathieu-Daudé Message-id: 20200617132201.1832152-6-stefa...@redhat.com Signed-off-by: Stefan Hajnoczi --- block/nvme.c | 2 +- 1 file changed, 1 in

[PULL 12/12] block/nvme: support nested aio_poll()

QEMU block drivers are supposed to support aio_poll() from I/O completion callback functions. This means completion processing must be re-entrant. The standard approach is to schedule a BH during completion processing and cancel it at the end of processing. If aio_poll() is invoked by a callback f

[PULL 11/12] block/nvme: keep BDRVNVMeState pointer in NVMeQueuePair

Passing around both BDRVNVMeState and NVMeQueuePair is unwieldy. Reduce the number of function arguments by keeping the BDRVNVMeState pointer in NVMeQueuePair. This will come in handly when a BH is introduced in a later patch and only one argument can be passed to it. Signed-off-by: Stefan Hajnocz

Re: [PATCH 1/2] vvfat: Check that updated filenames are valid

Am 23.06.2020 um 20:21 hat Eric Blake geschrieben: > On 6/23/20 12:55 PM, Kevin Wolf wrote: > > FAT allows only a restricted set of characters in file names, and for > > some of the illegal characters, it's actually important that we catch > > them: If filenames can contain '/', the guest can const

Re: [PATCH 2/2] vvfat: Fix array_remove_slice()

Am 23.06.2020 um 20:30 hat Eric Blake geschrieben: > On 6/23/20 12:55 PM, Kevin Wolf wrote: > > array_remove_slice() calls array_roll() with array->next - 1 as the > > destination index. This is only correct for count == 1, otherwise we're > > writing past the end of the array. array->next - count

[PATCH v2 01/25] iotests: Fix 051 output after qdev_init_nofail() removal

From: Philippe Mathieu-Daudé Commit 96927c744 replaced qdev_init_nofail() call by isa_realize_and_unref() which has a different error message. Update the test output accordingly. Gitlab CI error after merging b77b5b3dc7: https://gitlab.com/qemu-project/qemu/-/jobs/597414772#L4375 Reported-by: T

Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)

On 6/22/20 5:42 AM, Markus Armbruster wrote: > There are three ways to configure backends: > > * -nic, -serial, -drive, ... (onboard devices) > > * Set the property with -device, or, if you feel masochistic, with > -set device (pluggable devices) > > * Set the property with -global (both) >

Re: virtio-scsi and another complex AioContext issue

On Tue, Jun 23, 2020 at 03:24:54PM +0100, Stefan Hajnoczi wrote: > On Mon, Jun 22, 2020 at 04:16:04PM +0200, Sergio Lopez wrote: > > On Fri, Jun 19, 2020 at 02:04:06PM +0100, Stefan Hajnoczi wrote: > > > On Thu, Jun 11, 2020 at 10:36:22AM +0200, Sergio Lopez wrote: > > > > Hi, > > > > > > > > While

[PATCH 02/46] error: Document Error API usage rules

This merely codifies existing practice, with one exception: the rule advising against returning void, where existing practice is mixed. When the Error API was created, we adopted the (unwritten) rule to return void when the function returns no useful value on success, unlike GError, which recommen

[PATCH 10/46] qemu-option: Check return value instead of @err where convenient

Convert uses like opts = qemu_opts_create(..., &err); if (err) { ... } to opts = qemu_opts_create(..., &err); if (!opts) { ... } Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Signed-off-by: Markus Armbruster ---

[PATCH 01/46] error: Improve examples in error.h's big comment

Show errp instead of &err where &err is actually unusual. Add a missing declaration. Add a second error pileup example. Signed-off-by: Markus Armbruster --- include/qapi/error.h | 19 +++ 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/include/qapi/error.h b/incl

[PATCH 09/46] error: Avoid error_propagate() after migrate_add_blocker()

When migrate_add_blocker(blocker, &errp) is followed by error_propagate(errp, err), we can often just as well do migrate_add_blocker(..., errp). Do that with this Coccinelle script: @@ expression blocker, err, errp; expression ret; @@ -ret = migrate_add_blocker(blocker, &e

[PATCH 06/46] error: Avoid error_propagate() when error is not used here

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. Coccinelle script: @@ identifier fun, err, errp; expression list args; @@ -fun(args, &err); +fun(args, errp); .

[PATCH 12/46] qemu-option: Factor out helper find_default_by_name()

Signed-off-by: Markus Armbruster --- util/qemu-option.c | 47 ++ 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 9941005c91..ddcf3072c5 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.

[PATCH 04/46] macio: Tidy up error handling in macio_newworld_realize()

macio_newworld_realize() effectively ignores ns->gpio realization errors, leaking the Error object. Fortunately, macio_gpio_realize() can't actually fail. Tidy up. Cc: Mark Cave-Ayland Cc: David Gibson Signed-off-by: Markus Armbruster --- hw/misc/macio/macio.c | 4 +++- 1 file changed, 3 ins

[PATCH 14/46] qemu-option: Factor out helper opt_create()

There is just one use so far. The next commit will add more. Signed-off-by: Markus Armbruster --- util/qemu-option.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index d9293814b4..3cdf0c0800 100644 --- a

[PATCH 15/46] qemu-option: Tidy up opt_set() not to free arguments on failure

opt_set() frees its argument @value on failure. Slightly unclean; functions ideally do nothing on failure. To tidy this up, move opt_create() from opt_set() into its callers, along with the cleanup. Signed-off-by: Markus Armbruster --- util/qemu-option.c | 33 ++---

[PATCH 07/46] error: Avoid more error_propagate() when error is not used here

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. The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- blockdev.c |

[PATCH 08/46] error: Avoid unnecessary error_propagate() after error_setg()

Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: if (...) { error_setg(&err, ...); goto out; } ... out: error_propagate(errp, err); return; When all paths to label out are that way, replace b

[PATCH 13/46] qemu-option: Simplify around find_default_by_name()

Signed-off-by: Markus Armbruster --- util/qemu-option.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index ddcf3072c5..d9293814b4 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -286,11 +286,9 @@ const char *

[PATCH 03/46] qdev: Smooth error checking of qdev_realize() & friends

Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), sysbus_realize(), sysbus_realize_and_unref(), usb_realiz

[PATCH 31/46] qom: Use error_reportf_err() instead of g_printerr() in examples

Signed-off-by: Markus Armbruster --- include/qom/object.h | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/include/qom/object.h b/include/qom/object.h index 94a61ccc3f..b70edd8cd9 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -671,8 +671,7 @@ Object *obj

[PATCH 27/46] qapi: Purge error_propagate() from QAPI core

Signed-off-by: Markus Armbruster --- qapi/qapi-visit-core.c | 40 +++- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c index 5a9c47aabf..7e5f40e7f0 100644 --- a/qapi/qapi-visit-core.c +++ b/qapi/qa

[PATCH 05/46] virtio-crypto-pci: Tidy up virtio_crypto_pci_realize()

virtio_crypto_pci_realize() continues after realization of its "virtio-crypto-device" fails. Only an object_property_set_link() follows; looks harmless to me. Tidy up anyway: return after failure, just like virtio_rng_pci_realize() does. Cc: "Gonglei (Arei)" Cc: Michael S. Tsirkin Signed-off-b

[PATCH 33/46] qom: Crash more nicely on object_property_get_link() failure

Pass &error_abort instead of NULL where the returned value is dereferenced or asserted to be non-null. Signed-off-by: Markus Armbruster --- hw/core/platform-bus.c | 5 +++-- hw/ppc/spapr_drc.c | 3 ++- hw/ppc/spapr_hcall.c | 3 ++- hw/ppc/spapr_pci_nvlink2.c | 8 +--- ui/vn

[PATCH 24/46] qapi: Smooth error checking manually

When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- accel/kvm/kvm-all.c | 50 ++--

[PATCH 25/46] qapi: Smooth visitor error checking in generated code

Use visitor functions' return values to check for failure. Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.txt | 60 ++-- scripts/qapi/commands.py | 22 +

[PATCH 20/46] block: Avoid error accumulation in bdrv_img_create()

When creating an image fails because the format doesn't support option "backing_file" or "backing_fmt", bdrv_img_create() first has qemu_opt_set() put a generic error into @local_err, then puts the real error into @errp with error_setg(), and then propagates the former to the latter, which throws a

[PATCH 30/46] s390x/pci: Fix harmless mistake in zpci's property fid's setter

s390_pci_set_fid() sets zpci->fid_defined to true even when visit_type_uint32() failed. Reproducer: "-device zpci,fid=junk". Harmless in practice, because qdev_device_add() then fails, throwing away @zpci. Fix it anyway. Cc: Matthew Rosato Cc: Cornelia Huck Signed-off-by: Markus Armbruster --

[PATCH 39/46] qom: Smooth error checking manually

When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- hw/core/bus.c | 6 +- hw/s390x/s390-v

[PATCH 44/46] qemu-img: Ignore Error objects where the return value suffices

Signed-off-by: Markus Armbruster --- qemu-img.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 27bf94e7ae..c11bfe0268 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -464,22 +464,18 @@ static int add_old_style_options(const char *fmt, QemuOp

[PATCH 26/46] qapi: Smooth another visitor error checking pattern

Convert visit_type_FOO(v, ..., &ptr, &err); ... if (err) { ... } to visit_type_FOO(v, ..., &ptr, errp); ... if (!ptr) { ... } for functions that set @ptr to non-null / null on success / error. Eliminate error_propagate() that are now unnecessary.

[PATCH 28/46] block/parallels: Simplify parallels_open() after previous commit

Signed-off-by: Markus Armbruster --- block/parallels.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/block/parallels.c b/block/parallels.c index 9e85ab995e..3c22dfdc9d 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -839,6 +839,7 @@ static int parallels_open

[PATCH 32/46] qom: Rename qdev_get_type() to object_get_type()

Commit 2f262e06f0 lifted qdev_get_type() from qdev to object without renaming it accordingly. Do that now. Signed-off-by: Markus Armbruster --- qom/object.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index b8aac074c2..f6e9f0e413 100644 --

[PATCH 37/46] qom: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- include/qom/object.h| 42 - include/qom/object_interfaces.h | 12 +++-- include/qom/qom-qobject.h | 4 +- qom/object.c| 84

[PATCH 18/46] qemu-option: Smooth error checking manually

When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- block.c | 2 +- block/gluster.c | 8 +++

[PATCH 19/46] block: Avoid unnecessary error_propagate() after error_setg()

The previous commit enables another round of the transformation from recent commit "error: Avoid unnecessary error_propagate() after error_setg()". Signed-off-by: Markus Armbruster --- block/quorum.c | 16 +++- block/replication.c | 12 +--- block/vxhs.c| 10

[PATCH 00/46] Less clumsy error checking

When the Error API was created, we adopted the (unwritten) rule to return void when the function returns no useful value on success, unlike GError, which recommends to return true on success and false on error then. When a function returns a distinct error value, say false, a checked call that pas

[PATCH 29/46] acpi: Avoid unnecessary error_propagate() after error_setg()

The commit before previous enables another round of the transformation from recent commit "error: Avoid unnecessary error_propagate() after error_setg()". Signed-off-by: Markus Armbruster --- hw/acpi/core.c | 15 ++- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/hw/ac

[PATCH 11/46] qemu-option: Make uses of find_desc_by_name() more similar

This is to make the next commit easier to review. Signed-off-by: Markus Armbruster --- util/qemu-option.c | 32 ++-- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index 6119f971a4..9941005c91 100644 --- a/util/

[PATCH 17/46] qemu-option: Smooth error checking with Coccinelle

The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for QemuOpts functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = {opts_do_parse, parse_option_bo

[PATCH 23/46] qapi: Smooth error checking with Coccinelle

The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle script: @@ identifier fun =~ "check_list|input_t

[PATCH 40/46] qom: Make functions taking Error ** return bool, not 0/-1

Just for consistency. Also fix the example in object_set_props()'s documentation. Signed-off-by: Markus Armbruster --- include/qom/object.h | 28 +++- qom/object.c | 14 +++--- 2 files changed, 18 insertions(+), 24 deletions(-) diff --git a/include/qom/o

[PATCH 35/46] qom: Use return values to check for error where that's simpler

When using the Error object to check for error, we need to receive it into a local variable, then propagate() it to @errp. Using the return value permits allows receiving it straight to @errp. Signed-off-by: Markus Armbruster --- qom/object.c | 10 -- 1 file changed, 4 insertions(+), 6

[PATCH 22/46] qapi: Make visitor functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.txt | 51 +-- include/qapi/clone-visitor.h | 8 +- include/qapi/visitor-impl.h | 26 +++--- include/qapi/visitor.h| 102 -

[PATCH 43/46] qdev: Smooth error checking manually

When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for one more manually. Signed-off-by: Markus Armbruster --- hw/block/fdc.c | 8 +++- 1 file changed, 3 insertions

[PATCH 45/46] qdev: Ignore Error objects where the return value suffices

Signed-off-by: Markus Armbruster --- hw/core/qdev-properties.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index e1ad147339..8eb4283a56 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -653,

[PATCH 16/46] qemu-option: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- include/qemu/option.h | 16 blockdev.c| 5 ++- util/qemu-option.c| 92 +-- 3 files changed, 64 insertions(+), 49 deletio

[PATCH 34/46] qom: Don't handle impossible object_property_get_link() failure

Don't handle object_property_get_link() failure that can't happen unless the programmer screwed up, pass &error_abort. Signed-off-by: Markus Armbruster --- hw/arm/bcm2835_peripherals.c | 7 +-- hw/arm/bcm2836.c | 7 +-- hw/display/bcm2835_fb.c | 8 +--- hw/dma/bcm

[PATCH 41/46] qdev: Make functions taking Error ** return bool, not void

See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- include/hw/qdev-properties.h | 4 ++-- hw/core/qdev-properties-system.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/hw/qdev-properties.h b/includ

[PATCH 21/46] hmp: Eliminate a variable in hmp_migrate_set_parameter()

Signed-off-by: Markus Armbruster --- monitor/hmp-cmds.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c index 2b0b58a336..d7810cb564 100644 --- a/monitor/hmp-cmds.c +++ b/monitor/hmp-cmds.c @@ -1247,7 +1247,6 @@ void hmp_migrate_

[PATCH 38/46] qom: Smooth error checking with Coccinelle

The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., errp)) { ... } for QOM functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = {object_apply_global_props, object_i

[PATCH 46/46] hmp: Ignore Error objects where the return value suffices

qdev_print_props() receives and throws away Error objects just to check for object_property_get_str() and object_property_print() failure. Unnecessary, both return suitable values, so use those instead. Signed-off-by: Markus Armbruster --- qdev-monitor.c | 12 ++-- 1 file changed, 6 ins

[PATCH 42/46] qdev: Smooth error checking with Coccinelle

The previous commit enables conversion of qdev_prop_set_drive_err(..., &err); if (err) { ... } to if (!qdev_prop_set_drive_err(..., errp)) { ... } Coccinelle script: @@ identifier fun = qdev_prop_set_drive_err; expression list args, args2; typedef Er

Re: [PATCH v10 1/9] error: auto propagated local_err

Greg Kurz writes: > On Mon, 15 Jun 2020 07:21:03 +0200 > Markus Armbruster wrote: > >> Greg Kurz writes: >> >> > On Tue, 17 Mar 2020 18:16:17 +0300 >> > Vladimir Sementsov-Ogievskiy wrote: >> > >> >> Introduce a new ERRP_AUTO_PROPAGATE macro, to be used at start of >> >> functions with an err

Re: [PATCH 00/46] Less clumsy error checking

On 24/06/20 18:42, Markus Armbruster wrote: > When the Error API was created, we adopted the (unwritten) rule to > return void when the function returns no useful value on success, > unlike GError, which recommends to return true on success and false on > error then. I was actually never aware of

Re: [PATCH 01/46] error: Improve examples in error.h's big comment

On 6/24/20 11:42 AM, Markus Armbruster wrote: Show errp instead of &err where &err is actually unusual. Add a missing declaration. Add a second error pileup example. Signed-off-by: Markus Armbruster --- include/qapi/error.h | 19 +++ 1 file changed, 15 insertions(+), 4 dele

Re: [PATCH 02/46] error: Document Error API usage rules

On 6/24/20 11:43 AM, Markus Armbruster wrote: This merely codifies existing practice, with one exception: the rule advising against returning void, where existing practice is mixed. When the Error API was created, we adopted the (unwritten) rule to return void when the function returns no useful

Re: [PATCH 03/46] qdev: Smooth error checking of qdev_realize() & friends

On 6/24/20 11:43 AM, Markus Armbruster wrote: Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), s

Re: [PATCH 06/46] error: Avoid error_propagate() when error is not used here

On 6/24/20 11:43 AM, Markus Armbruster wrote: 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. Coccinelle script: This seems to be a recurring cleanup (witness commit 06592d7e, c0e90679, 6b62d961).

Re: [PATCH 07/46] error: Avoid more error_propagate() when error is not used here

On 6/24/20 11:43 AM, Markus Armbruster wrote: 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. The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-of

Re: [PATCH 08/46] error: Avoid unnecessary error_propagate() after error_setg()

On 6/24/20 11:43 AM, Markus Armbruster wrote: Replace error_setg(&err, ...); error_propagate(errp, err); by error_setg(errp, ...); Related pattern: Nice explanation. Bonus: the elimination of gotos will make later patches in this series easier to review. Candidates for con

Re: [PATCH 09/46] error: Avoid error_propagate() after migrate_add_blocker()

On 6/24/20 11:43 AM, Markus Armbruster wrote: When migrate_add_blocker(blocker, &errp) is followed by error_propagate(errp, err), we can often just as well do migrate_add_blocker(..., errp). Do that with this Coccinelle script: Double-check @err is not used afterwards. Dereferencing it woul

Re: [PATCH 10/46] qemu-option: Check return value instead of @err where convenient

On 6/24/20 11:43 AM, Markus Armbruster wrote: Convert uses like opts = qemu_opts_create(..., &err); if (err) { ... } to opts = qemu_opts_create(..., &err); if (!opts) { ... } Eliminate error_propagate() that are now unnecessary. Delete @err tha

Re: [PATCH 11/46] qemu-option: Make uses of find_desc_by_name() more similar

On 6/24/20 11:43 AM, Markus Armbruster wrote: This is to make the next commit easier to review. Signed-off-by: Markus Armbruster --- util/qemu-option.c | 32 ++-- 1 file changed, 18 insertions(+), 14 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Princip

Re: [PATCH 12/46] qemu-option: Factor out helper find_default_by_name()

On 6/24/20 11:43 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- util/qemu-option.c | 47 ++ 1 file changed, 27 insertions(+), 20 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc.

Re: [PATCH 13/46] qemu-option: Simplify around find_default_by_name()

On 6/24/20 11:43 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- util/qemu-option.c | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/util/qemu-option.c b/util/qemu-option.c index ddcf3072c5..d9293814b4 100644 --- a/util/qemu-option.c +++ b/ut

Re: [PATCH v5 0/7] Introduce 'yank' oob qmp command to recover from hanging qemu

On Tue, 23 Jun 2020 16:42:30 +0200 Lukas Straub wrote: > Hello Everyone, > In many cases, if qemu has a network connection (qmp, migration, chardev, > etc.) > to some other server and that server dies or hangs, qemu hangs too. > These patches introduce the new 'yank' out-of-band qmp command to r

Re: [PATCH v10 1/9] error: auto propagated local_err

On Wed, 24 Jun 2020 18:53:05 +0200 Markus Armbruster wrote: > Greg Kurz writes: > > > On Mon, 15 Jun 2020 07:21:03 +0200 > > Markus Armbruster wrote: > > > >> Greg Kurz writes: > >> > >> > On Tue, 17 Mar 2020 18:16:17 +0300 > >> > Vladimir Sementsov-Ogievskiy wrote: > >> > > >> >> Introduce

Re: [PATCH 14/46] qemu-option: Factor out helper opt_create()

On 6/24/20 11:43 AM, Markus Armbruster wrote: There is just one use so far. The next commit will add more. Signed-off-by: Markus Armbruster --- util/qemu-option.c | 27 ++- 1 file changed, 18 insertions(+), 9 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, P

Re: [PATCH 15/46] qemu-option: Tidy up opt_set() not to free arguments on failure

On 6/24/20 11:43 AM, Markus Armbruster wrote: opt_set() frees its argument @value on failure. Slightly unclean; functions ideally do nothing on failure. To tidy this up, move opt_create() from opt_set() into its callers, along with the cleanup. Signed-off-by: Markus Armbruster --- util/qemu

Re: [PATCH 16/46] qemu-option: Make functions taking Error ** return bool, not void

On 6/24/20 11:43 AM, Markus Armbruster wrote: See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- include/qemu/option.h | 16 blockdev.c| 5 ++- util/qemu-option.c| 92 +--

Re: [PATCH 17/46] qemu-option: Smooth error checking with Coccinelle

On 6/24/20 11:43 AM, Markus Armbruster wrote: The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for QemuOpts functions that now return true / false on success / error. Coccinelle script:

Re: [PATCH 18/46] qemu-option: Smooth error checking manually

On 6/24/20 11:43 AM, Markus Armbruster wrote: When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- block.

Re: [PATCH 19/46] block: Avoid unnecessary error_propagate() after error_setg()

On 6/24/20 11:43 AM, Markus Armbruster wrote: The previous commit enables another round of the transformation from recent commit "error: Avoid unnecessary error_propagate() after error_setg()". Signed-off-by: Markus Armbruster --- block/quorum.c | 16 +++- block/replication.

Re: [PATCH 20/46] block: Avoid error accumulation in bdrv_img_create()

On 6/24/20 11:43 AM, Markus Armbruster wrote: When creating an image fails because the format doesn't support option "backing_file" or "backing_fmt", bdrv_img_create() first has qemu_opt_set() put a generic error into @local_err, then puts the real error into @errp with error_setg(), and then pro

Re: [PATCH 21/46] hmp: Eliminate a variable in hmp_migrate_set_parameter()

On 6/24/20 11:43 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- monitor/hmp-cmds.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization:

Re: [PATCH 30/46] s390x/pci: Fix harmless mistake in zpci's property fid's setter

On 6/24/20 12:43 PM, Markus Armbruster wrote: s390_pci_set_fid() sets zpci->fid_defined to true even when visit_type_uint32() failed. Reproducer: "-device zpci,fid=junk". Harmless in practice, because qdev_device_add() then fails, throwing away @zpci. Fix it anyway. Cc: Matthew Rosato Cc: Cor

Re: [PATCH 22/46] qapi: Make visitor functions taking Error ** return bool, not void

On 6/24/20 11:43 AM, Markus Armbruster wrote: See recent commit "error: Document Error API usage rules" for rationale. Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.txt | 51 +-- include/qapi/clone-visitor.h | 8 +- include/qapi/visitor-impl.h | 26 +++--- i

Re: [PATCH 23/46] qapi: Smooth error checking with Coccinelle

On 6/24/20 11:43 AM, Markus Armbruster wrote: The previous commit enables conversion of visit_foo(..., &err); if (err) { ... } to if (!visit_foo(..., errp)) { ... } for visitor functions that now return true / false on success / error. Coccinelle scrip

Re: [PATCH 24/46] qapi: Smooth error checking manually

On 6/24/20 11:43 AM, Markus Armbruster wrote: When foo(..., &err) is followed by error_propagate(errp, err), we can often just as well do foo(..., errp). The previous commit did that for simple cases with Coccinelle. Do it for a few more manually. Signed-off-by: Markus Armbruster --- Review

Re: [PATCH 25/46] qapi: Smooth visitor error checking in generated code

On 6/24/20 11:43 AM, Markus Armbruster wrote: Use visitor functions' return values to check for failure. Eliminate error_propagate() that are now unnecessary. Delete @err that are now unused. Signed-off-by: Markus Armbruster --- docs/devel/qapi-code-gen.txt | 60 ++--

Re: [PATCH 26/46] qapi: Smooth another visitor error checking pattern

On 6/24/20 11:43 AM, Markus Armbruster wrote: Convert visit_type_FOO(v, ..., &ptr, &err); ... if (err) { ... } to visit_type_FOO(v, ..., &ptr, errp); ... if (!ptr) { ... } for functions that set @ptr to non-null / null on success / error

Re: [PATCH 27/46] qapi: Purge error_propagate() from QAPI core

On 6/24/20 11:43 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- qapi/qapi-visit-core.c | 40 +++- 1 file changed, 19 insertions(+), 21 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc.

Re: [PATCH 28/46] block/parallels: Simplify parallels_open() after previous commit

On 6/24/20 11:43 AM, Markus Armbruster wrote: Signed-off-by: Markus Armbruster --- block/parallels.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qe

  1   2   >