|
From: | Fei Li |
Subject: | Re: [Qemu-devel] [PATCH for-4.0 v9 10/16] qemu_thread: supplement error handling for h_resize_hpt_prepare |
Date: | Thu, 3 Jan 2019 21:41:49 +0800 |
User-agent: | Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
在 2019/1/3 上午11:43, David Gibson 写道:
On Wed, Jan 02, 2019 at 02:44:17PM +0800, 李菲 wrote:在 2019/1/2 上午10:36, David Gibson 写道:On Tue, Dec 25, 2018 at 10:04:43PM +0800, Fei Li wrote:Add a local_err to hold the error, and return the corresponding error code to replace the temporary &error_abort. Cc: Markus Armbruster <address@hidden> Cc: David Gibson <address@hidden> Signed-off-by: Fei Li <address@hidden>This looks like a good change, but it no longer applies due to a change in the qemu_thread_create() signature.Sorry that I am not sure whether I understand. Do you mean using &error_abort is more suitable for this handling, rather than report the &local_err & return a failure reason?No, I just mean that context has been altered by a global change and the patch will need to be fixed up to cope with that.
Just to be clearer: does the "global change" mean the "[patch 06/16] qemu_thread: Make qemu_thread_create() handle errors properly", or another patch not in this patch series?
If it means the [patch 06/16], I want to explain more: the 06/16 handles all qemu_thread_create() by passing &error_abort as the parameter, and the following patches are to improve on the &error_abort for callers who canhandle more properly. E.g. if qemu_thread_create() fails in h_resize_hpt_prepare(), I think reporting the &local_err & returning the failure reason is more proper
than just abort() inside qemu_thread_create() when calls error_setg_errno().In other words, this patch is actually written to apply to patch 06. And I have no clue where it needs to be fixed up. Please correct me if I understand wrong.
Have a nice day, thanks :) Fei
--- hw/ppc/spapr_hcall.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 5bc2cf4540..7c16ade04a 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -478,6 +478,7 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu, sPAPRPendingHPT *pending = spapr->pending_hpt; uint64_t current_ram_size; int rc; + Error *local_err = NULL; if (spapr->resize_hpt == SPAPR_RESIZE_HPT_DISABLED) { return H_AUTHORITY; @@ -538,10 +539,13 @@ static target_ulong h_resize_hpt_prepare(PowerPCCPU *cpu, pending->shift = shift; pending->ret = H_HARDWARE; - /* TODO: let the further caller handle the error instead of abort() here */ - qemu_thread_create(&pending->thread, "sPAPR HPT prepare", - hpt_prepare_thread, pending, - QEMU_THREAD_DETACHED, &error_abort); + if (!qemu_thread_create(&pending->thread, "sPAPR HPT prepare", + hpt_prepare_thread, pending, + QEMU_THREAD_DETACHED, &local_err)) { + error_reportf_err(local_err, "failed to create hpt_prepare_thread: "); + g_free(pending); + return H_RESOURCE;I also think H_HARDWARE would be a better choice here. Although the failure is due to a resource constraint, it's not because the guest asked for too much, just because the host is in dire straits. From the guest's point of view it's basically a hardware failure.Ok, thanks. Will use H_HARDWARE instead. Have a nice day, thanks for the review. :) Fei+ } spapr->pending_hpt = pending;
[Prev in Thread] | Current Thread | [Next in Thread] |