* Re: [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors
2023-11-20 15:04 [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors Philippe Mathieu-Daudé
@ 2023-11-21 0:28 ` zhenwei pi
2023-11-21 0:59 ` Gonglei (Arei) via
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: zhenwei pi @ 2023-11-21 0:28 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Gonglei (Arei), Markus Armbruster, Daniel P . Berrangé,
qemu-stable
Looks good to me. Thanks!
Reviewed-by: zhenwei pi <pizhenwei@bytedance.com>
On 11/20/23 23:04, Philippe Mathieu-Daudé wrote:
> Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init()
> can set their Error** argument. Do not ignore them, return early on
> failure. Use the ERRP_GUARD() macro as suggested in commit ae7c80a7bd
> ("error: New macro ERRP_GUARD()").
>
> Cc: qemu-stable@nongnu.org
> Fixes: e7a775fd9f ("cryptodev: Account statistics")
> Fixes: 2580b452ff ("cryptodev: support QoS")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> backends/cryptodev.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index e5006bd215..fff89fd62a 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -398,6 +398,7 @@ static void cryptodev_backend_set_ops(Object *obj, Visitor *v,
> static void
> cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> {
> + ERRP_GUARD();
> CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
> CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> uint32_t services;
> @@ -406,11 +407,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> QTAILQ_INIT(&backend->opinfos);
> value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
> cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
> + if (*errp) {
> + return;
> + }
> value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
> cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
> + if (*errp) {
> + return;
> + }
>
> if (bc->init) {
> bc->init(backend, errp);
> + if (*errp) {
> + return;
> + }
> }
>
> services = backend->conf.crypto_services;
--
zhenwei pi
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors
2023-11-20 15:04 [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors Philippe Mathieu-Daudé
2023-11-21 0:28 ` zhenwei pi
@ 2023-11-21 0:59 ` Gonglei (Arei) via
2023-11-21 7:40 ` Markus Armbruster
2024-01-18 8:02 ` Michael Tokarev
3 siblings, 0 replies; 5+ messages in thread
From: Gonglei (Arei) via @ 2023-11-21 0:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel@nongnu.org
Cc: Zhenwei Pi, Markus Armbruster, Daniel P . Berrangé,
qemu-stable@nongnu.org
> -----Original Message-----
> From: Philippe Mathieu-Daudé [mailto:philmd@linaro.org]
> Sent: Monday, November 20, 2023 11:04 PM
> To: qemu-devel@nongnu.org
> Cc: Zhenwei Pi <pizhenwei@bytedance.com>; Gonglei (Arei)
> <arei.gonglei@huawei.com>; Markus Armbruster <armbru@redhat.com>;
> Daniel P . Berrangé <berrange@redhat.com>; Philippe Mathieu-Daudé
> <philmd@linaro.org>; qemu-stable@nongnu.org
> Subject: [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore
> throttle/backends Errors
>
> Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init() can
> set their Error** argument. Do not ignore them, return early on failure. Use
> the ERRP_GUARD() macro as suggested in commit ae7c80a7bd
> ("error: New macro ERRP_GUARD()").
>
> Cc: qemu-stable@nongnu.org
> Fixes: e7a775fd9f ("cryptodev: Account statistics")
> Fixes: 2580b452ff ("cryptodev: support QoS")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors
2023-11-20 15:04 [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors Philippe Mathieu-Daudé
2023-11-21 0:28 ` zhenwei pi
2023-11-21 0:59 ` Gonglei (Arei) via
@ 2023-11-21 7:40 ` Markus Armbruster
2024-01-18 8:02 ` Michael Tokarev
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2023-11-21 7:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Zhenwei Pi, Gonglei (Arei), Markus Armbruster,
Daniel P . Berrangé, qemu-stable
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init()
> can set their Error** argument. Do not ignore them, return early on
> failure.
Let's mention why we need to: "Without that, running into another
failure trips error_setv()'s assertion."
> Use the ERRP_GUARD() macro as suggested in commit ae7c80a7bd
> ("error: New macro ERRP_GUARD()").
>
> Cc: qemu-stable@nongnu.org
> Fixes: e7a775fd9f ("cryptodev: Account statistics")
> Fixes: 2580b452ff ("cryptodev: support QoS")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> backends/cryptodev.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/backends/cryptodev.c b/backends/cryptodev.c
> index e5006bd215..fff89fd62a 100644
> --- a/backends/cryptodev.c
> +++ b/backends/cryptodev.c
> @@ -398,6 +398,7 @@ static void cryptodev_backend_set_ops(Object *obj, Visitor *v,
> static void
> cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> {
> + ERRP_GUARD();
> CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
> CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
> uint32_t services;
> @@ -406,11 +407,20 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
> QTAILQ_INIT(&backend->opinfos);
> value = backend->tc.buckets[THROTTLE_OPS_TOTAL].avg;
> cryptodev_backend_set_throttle(backend, THROTTLE_OPS_TOTAL, value, errp);
> + if (*errp) {
> + return;
> + }
> value = backend->tc.buckets[THROTTLE_BPS_TOTAL].avg;
> cryptodev_backend_set_throttle(backend, THROTTLE_BPS_TOTAL, value, errp);
> + if (*errp) {
> + return;
> + }
>
> if (bc->init) {
> bc->init(backend, errp);
> + if (*errp) {
> + return;
> + }
> }
>
> services = backend->conf.crypto_services;
Preferably with the commit message improved:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors
2023-11-20 15:04 [PATCH-for-8.2 v2] backends/cryptodev: Do not ignore throttle/backends Errors Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2023-11-21 7:40 ` Markus Armbruster
@ 2024-01-18 8:02 ` Michael Tokarev
3 siblings, 0 replies; 5+ messages in thread
From: Michael Tokarev @ 2024-01-18 8:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Zhenwei Pi, Gonglei (Arei), Markus Armbruster,
Daniel P . Berrangé, qemu-stable
20.11.2023 18:04, Philippe Mathieu-Daudé:
> Both cryptodev_backend_set_throttle() and CryptoDevBackendClass::init()
> can set their Error** argument. Do not ignore them, return early on
> failure. Use the ERRP_GUARD() macro as suggested in commit ae7c80a7bd
> ("error: New macro ERRP_GUARD()").
>
> Cc: qemu-stable@nongnu.org
> Fixes: e7a775fd9f ("cryptodev: Account statistics")
> Fixes: 2580b452ff ("cryptodev: support QoS")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Ping?
Can we get this to master before Jan-27? :)
It's been reviewed by a few people already.
Thanks,
/mjt
^ permalink raw reply [flat|nested] 5+ messages in thread