| Message ID | 20190201051806.53183-5-lifei1214@126.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu_thread_create: propagate the error to callers to handle | expand |
Fei Li <lifei1214@126.com> writes: > From: Fei Li <fli@suse.com> > > Utilize the existed errp to propagate the error and do the > corresponding cleanup to replace the temporary &error_abort. > > Cc: Markus Armbruster <armbru@redhat.com> > Cc: Jiri Slaby <jslaby@suse.cz> > Signed-off-by: Fei Li <fli@suse.com> > --- > hw/misc/edu.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/hw/misc/edu.c b/hw/misc/edu.c > index 21adbfddce..8fe232b6d6 100644 > --- a/hw/misc/edu.c > +++ b/hw/misc/edu.c > @@ -356,9 +356,14 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) > > qemu_mutex_init(&edu->thr_mutex); > qemu_cond_init(&edu->thr_cond); > - /* TODO: let the further caller handle the error instead of abort() here */ > - qemu_thread_create(&edu->thread, "edu", edu_fact_thread, > - edu, QEMU_THREAD_JOINABLE, &error_abort); > + if (qemu_thread_create(&edu->thread, "edu", edu_fact_thread, > + edu, QEMU_THREAD_JOINABLE, errp) < 0) { > + qemu_cond_destroy(&edu->thr_cond); > + qemu_mutex_destroy(&edu->thr_mutex); > + timer_del(&edu->dma_timer); > + msi_uninit(pdev); > + return; > + } > > memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, > "edu-mmio", 1 * MiB); In review of v9, I pointed out that pci_edu_uninit() neglects to call msix_uninit(), and you offered to fix it. Not in this series. Do you still intend to fix it? That's a separate patch, though, so Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 在 2019年2月1日，20:58，Markus Armbruster <armbru@redhat.com> 写道： > > Fei Li <lifei1214@126.com> writes: > >> From: Fei Li <fli@suse.com> >> >> Utilize the existed errp to propagate the error and do the >> corresponding cleanup to replace the temporary &error_abort. >> >> Cc: Markus Armbruster <armbru@redhat.com> >> Cc: Jiri Slaby <jslaby@suse.cz> >> Signed-off-by: Fei Li <fli@suse.com> >> --- >> hw/misc/edu.c | 11 ++++++++--- >> 1 file changed, 8 insertions(+), 3 deletions(-) >> >> diff --git a/hw/misc/edu.c b/hw/misc/edu.c >> index 21adbfddce..8fe232b6d6 100644 >> --- a/hw/misc/edu.c >> +++ b/hw/misc/edu.c >> @@ -356,9 +356,14 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) >> >> qemu_mutex_init(&edu->thr_mutex); >> qemu_cond_init(&edu->thr_cond); >> - /* TODO: let the further caller handle the error instead of abort() here */ >> - qemu_thread_create(&edu->thread, "edu", edu_fact_thread, >> - edu, QEMU_THREAD_JOINABLE, &error_abort); >> + if (qemu_thread_create(&edu->thread, "edu", edu_fact_thread, >> + edu, QEMU_THREAD_JOINABLE, errp) < 0) { >> + qemu_cond_destroy(&edu->thr_cond); >> + qemu_mutex_destroy(&edu->thr_mutex); >> + timer_del(&edu->dma_timer); >> + msi_uninit(pdev); >> + return; >> + } >> >> memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, >> "edu-mmio", 1 * MiB); > > In review of v9, I pointed out that pci_edu_uninit() neglects to call > msix_uninit(), and you offered to fix it. Not in this series. Do you > still intend to fix it? Yes, I have already sent that patch separately as I consider it is little relevant. And that patch has been merged. :) > > That's a separate patch, though, so > Reviewed-by: Markus Armbruster <armbru@redhat.com> Thanks for the review! Have a nice day Fei
diff --git a/hw/misc/edu.c b/hw/misc/edu.c index 21adbfddce..8fe232b6d6 100644 --- a/hw/misc/edu.c +++ b/hw/misc/edu.c @@ -356,9 +356,14 @@ static void pci_edu_realize(PCIDevice *pdev, Error **errp) qemu_mutex_init(&edu->thr_mutex); qemu_cond_init(&edu->thr_cond); - /* TODO: let the further caller handle the error instead of abort() here */ - qemu_thread_create(&edu->thread, "edu", edu_fact_thread, - edu, QEMU_THREAD_JOINABLE, &error_abort); + if (qemu_thread_create(&edu->thread, "edu", edu_fact_thread, + edu, QEMU_THREAD_JOINABLE, errp) < 0) { + qemu_cond_destroy(&edu->thr_cond); + qemu_mutex_destroy(&edu->thr_mutex); + timer_del(&edu->dma_timer); + msi_uninit(pdev); + return; + } memory_region_init_io(&edu->mmio, OBJECT(edu), &edu_mmio_ops, edu, "edu-mmio", 1 * MiB);