* [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio
@ 2018-03-02 17:08 Anton Nefedov
2018-03-05 21:54 ` Mark Cave-Ayland
2018-03-21 20:06 ` John Snow
0 siblings, 2 replies; 5+ messages in thread
From: Anton Nefedov @ 2018-03-02 17:08 UTC (permalink / raw)
To: qemu-devel; +Cc: jsnow, mark.cave-ayland, Anton Nefedov
commit 947858b0 "ide: abort TRIM operation for invalid range"
is incorrect for macio; just ide_dma_error() without doing a callback
is not enough for that errorpath.
Instead, pass -EINVAL to the callback and handle it there
(see related motivation for read/write in 58ac32113).
It will however catch possible EINVAL from the block layer too.
Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
---
hw/ide/core.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 257b429..54510d4 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
QEMUIOVector *qiov;
BlockAIOCB *aiocb;
int i, j;
- bool is_invalid;
} TrimAIOCB;
static void trim_aio_cancel(BlockAIOCB *acb)
@@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
{
TrimAIOCB *iocb = opaque;
- if (iocb->is_invalid) {
- ide_dma_error(iocb->s);
- } else {
- iocb->common.cb(iocb->common.opaque, iocb->ret);
- }
+ iocb->common.cb(iocb->common.opaque, iocb->ret);
+
qemu_bh_delete(iocb->bh);
iocb->bh = NULL;
qemu_aio_unref(iocb);
@@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
}
if (!ide_sect_range_ok(s, sector, count)) {
- iocb->is_invalid = true;
+ iocb->ret = -EINVAL;
goto done;
}
@@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
iocb->qiov = qiov;
iocb->i = -1;
iocb->j = 0;
- iocb->is_invalid = false;
ide_issue_trim_cb(iocb, 0);
return &iocb->common;
}
@@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
if (ret == -ECANCELED) {
return;
}
+
+ if (ret == -EINVAL) {
+ ide_dma_error(s);
+ return;
+ }
+
if (ret < 0) {
if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
s->bus->dma->aiocb = NULL;
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio
2018-03-02 17:08 [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio Anton Nefedov
@ 2018-03-05 21:54 ` Mark Cave-Ayland
2018-03-16 11:20 ` Mark Cave-Ayland
2018-03-21 20:06 ` John Snow
1 sibling, 1 reply; 5+ messages in thread
From: Mark Cave-Ayland @ 2018-03-05 21:54 UTC (permalink / raw)
To: Anton Nefedov, qemu-devel; +Cc: jsnow
On 02/03/18 17:08, Anton Nefedov wrote:
> commit 947858b0 "ide: abort TRIM operation for invalid range"
> is incorrect for macio; just ide_dma_error() without doing a callback
> is not enough for that errorpath.
>
> Instead, pass -EINVAL to the callback and handle it there
> (see related motivation for read/write in 58ac32113).
>
> It will however catch possible EINVAL from the block layer too.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
> hw/ide/core.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 257b429..54510d4 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
> QEMUIOVector *qiov;
> BlockAIOCB *aiocb;
> int i, j;
> - bool is_invalid;
> } TrimAIOCB;
>
> static void trim_aio_cancel(BlockAIOCB *acb)
> @@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
> {
> TrimAIOCB *iocb = opaque;
>
> - if (iocb->is_invalid) {
> - ide_dma_error(iocb->s);
> - } else {
> - iocb->common.cb(iocb->common.opaque, iocb->ret);
> - }
> + iocb->common.cb(iocb->common.opaque, iocb->ret);
> +
> qemu_bh_delete(iocb->bh);
> iocb->bh = NULL;
> qemu_aio_unref(iocb);
> @@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
> }
>
> if (!ide_sect_range_ok(s, sector, count)) {
> - iocb->is_invalid = true;
> + iocb->ret = -EINVAL;
> goto done;
> }
>
> @@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
> iocb->qiov = qiov;
> iocb->i = -1;
> iocb->j = 0;
> - iocb->is_invalid = false;
> ide_issue_trim_cb(iocb, 0);
> return &iocb->common;
> }
> @@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
> if (ret == -ECANCELED) {
> return;
> }
> +
> + if (ret == -EINVAL) {
> + ide_dma_error(s);
> + return;
> + }
> +
> if (ret < 0) {
> if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
> s->bus->dma->aiocb = NULL;
>
Hi Anton,
Thanks for this. I applied this patch to git master with my macio patch
at https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06076.html
and it allowed to continue all the way through the Debian installer for
the PPC g3beige machine so I believe it is working.
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
ATB,
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio
2018-03-05 21:54 ` Mark Cave-Ayland
@ 2018-03-16 11:20 ` Mark Cave-Ayland
2018-03-16 18:38 ` John Snow
0 siblings, 1 reply; 5+ messages in thread
From: Mark Cave-Ayland @ 2018-03-16 11:20 UTC (permalink / raw)
To: jsnow, qemu-devel; +Cc: Anton Nefedov
On 05/03/18 21:54, Mark Cave-Ayland wrote:
> On 02/03/18 17:08, Anton Nefedov wrote:
>
>> commit 947858b0 "ide: abort TRIM operation for invalid range"
>> is incorrect for macio; just ide_dma_error() without doing a callback
>> is not enough for that errorpath.
>>
>> Instead, pass -EINVAL to the callback and handle it there
>> (see related motivation for read/write in 58ac32113).
>>
>> It will however catch possible EINVAL from the block layer too.
>>
>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
>> ---
>> hw/ide/core.c | 17 +++++++++--------
>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>> index 257b429..54510d4 100644
>> --- a/hw/ide/core.c
>> +++ b/hw/ide/core.c
>> @@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
>> QEMUIOVector *qiov;
>> BlockAIOCB *aiocb;
>> int i, j;
>> - bool is_invalid;
>> } TrimAIOCB;
>> static void trim_aio_cancel(BlockAIOCB *acb)
>> @@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
>> {
>> TrimAIOCB *iocb = opaque;
>> - if (iocb->is_invalid) {
>> - ide_dma_error(iocb->s);
>> - } else {
>> - iocb->common.cb(iocb->common.opaque, iocb->ret);
>> - }
>> + iocb->common.cb(iocb->common.opaque, iocb->ret);
>> +
>> qemu_bh_delete(iocb->bh);
>> iocb->bh = NULL;
>> qemu_aio_unref(iocb);
>> @@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>> }
>> if (!ide_sect_range_ok(s, sector, count)) {
>> - iocb->is_invalid = true;
>> + iocb->ret = -EINVAL;
>> goto done;
>> }
>> @@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
>> iocb->qiov = qiov;
>> iocb->i = -1;
>> iocb->j = 0;
>> - iocb->is_invalid = false;
>> ide_issue_trim_cb(iocb, 0);
>> return &iocb->common;
>> }
>> @@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
>> if (ret == -ECANCELED) {
>> return;
>> }
>> +
>> + if (ret == -EINVAL) {
>> + ide_dma_error(s);
>> + return;
>> + }
>> +
>> if (ret < 0) {
>> if (ide_handle_rw_error(s, -ret,
>> ide_dma_cmd_to_retry(s->dma_cmd))) {
>> s->bus->dma->aiocb = NULL;
>>
>
> Hi Anton,
>
> Thanks for this. I applied this patch to git master with my macio patch
> at https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06076.html
> and it allowed to continue all the way through the Debian installer for
> the PPC g3beige machine so I believe it is working.
>
> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Hi John,
I know that you're busy, but just wondering if you have managed to
review these 2 TRIM patches? They are a candidate for 2.12 since they
prevent a qemu-system-ppc segfault on Mac machines when issuing IDE TRIM
commands.
ATB,
Mark.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio
2018-03-16 11:20 ` Mark Cave-Ayland
@ 2018-03-16 18:38 ` John Snow
0 siblings, 0 replies; 5+ messages in thread
From: John Snow @ 2018-03-16 18:38 UTC (permalink / raw)
To: Mark Cave-Ayland, qemu-devel; +Cc: Anton Nefedov
On 03/16/2018 07:20 AM, Mark Cave-Ayland wrote:
> On 05/03/18 21:54, Mark Cave-Ayland wrote:
>
>> On 02/03/18 17:08, Anton Nefedov wrote:
>>
>>> commit 947858b0 "ide: abort TRIM operation for invalid range"
>>> is incorrect for macio; just ide_dma_error() without doing a callback
>>> is not enough for that errorpath.
>>>
>>> Instead, pass -EINVAL to the callback and handle it there
>>> (see related motivation for read/write in 58ac32113).
>>>
>>> It will however catch possible EINVAL from the block layer too.
>>>
>>> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
>>> ---
>>> hw/ide/core.c | 17 +++++++++--------
>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/ide/core.c b/hw/ide/core.c
>>> index 257b429..54510d4 100644
>>> --- a/hw/ide/core.c
>>> +++ b/hw/ide/core.c
>>> @@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
>>> QEMUIOVector *qiov;
>>> BlockAIOCB *aiocb;
>>> int i, j;
>>> - bool is_invalid;
>>> } TrimAIOCB;
>>> static void trim_aio_cancel(BlockAIOCB *acb)
>>> @@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
>>> {
>>> TrimAIOCB *iocb = opaque;
>>> - if (iocb->is_invalid) {
>>> - ide_dma_error(iocb->s);
>>> - } else {
>>> - iocb->common.cb(iocb->common.opaque, iocb->ret);
>>> - }
>>> + iocb->common.cb(iocb->common.opaque, iocb->ret);
>>> +
>>> qemu_bh_delete(iocb->bh);
>>> iocb->bh = NULL;
>>> qemu_aio_unref(iocb);
>>> @@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
>>> }
>>> if (!ide_sect_range_ok(s, sector, count)) {
>>> - iocb->is_invalid = true;
>>> + iocb->ret = -EINVAL;
>>> goto done;
>>> }
>>> @@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
>>> iocb->qiov = qiov;
>>> iocb->i = -1;
>>> iocb->j = 0;
>>> - iocb->is_invalid = false;
>>> ide_issue_trim_cb(iocb, 0);
>>> return &iocb->common;
>>> }
>>> @@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
>>> if (ret == -ECANCELED) {
>>> return;
>>> }
>>> +
>>> + if (ret == -EINVAL) {
>>> + ide_dma_error(s);
>>> + return;
>>> + }
>>> +
>>> if (ret < 0) {
>>> if (ide_handle_rw_error(s, -ret,
>>> ide_dma_cmd_to_retry(s->dma_cmd))) {
>>> s->bus->dma->aiocb = NULL;
>>>
>>
>> Hi Anton,
>>
>> Thanks for this. I applied this patch to git master with my macio
>> patch at
>> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg06076.html
>> and it allowed to continue all the way through the Debian installer
>> for the PPC g3beige machine so I believe it is working.
>>
>> Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>
> Hi John,
>
> I know that you're busy, but just wondering if you have managed to
> review these 2 TRIM patches? They are a candidate for 2.12 since they
> prevent a qemu-system-ppc segfault on Mac machines when issuing IDE TRIM
> commands.
>
>
> ATB,
>
> Mark.
Will send a PR once it looks as if Peter Maydell has caught up with all
of the last-minute PR panic for soft freeze -- they're not forgotten, I
promise!
Thank you for checking in on me.
--John
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio
2018-03-02 17:08 [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio Anton Nefedov
2018-03-05 21:54 ` Mark Cave-Ayland
@ 2018-03-21 20:06 ` John Snow
1 sibling, 0 replies; 5+ messages in thread
From: John Snow @ 2018-03-21 20:06 UTC (permalink / raw)
To: Anton Nefedov, qemu-devel; +Cc: mark.cave-ayland
On 03/02/2018 12:08 PM, Anton Nefedov wrote:
> commit 947858b0 "ide: abort TRIM operation for invalid range"
> is incorrect for macio; just ide_dma_error() without doing a callback
> is not enough for that errorpath.
>
> Instead, pass -EINVAL to the callback and handle it there
> (see related motivation for read/write in 58ac32113).
>
> It will however catch possible EINVAL from the block layer too.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
> hw/ide/core.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 257b429..54510d4 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -402,7 +402,6 @@ typedef struct TrimAIOCB {
> QEMUIOVector *qiov;
> BlockAIOCB *aiocb;
> int i, j;
> - bool is_invalid;
> } TrimAIOCB;
>
> static void trim_aio_cancel(BlockAIOCB *acb)
> @@ -430,11 +429,8 @@ static void ide_trim_bh_cb(void *opaque)
> {
> TrimAIOCB *iocb = opaque;
>
> - if (iocb->is_invalid) {
> - ide_dma_error(iocb->s);
> - } else {
> - iocb->common.cb(iocb->common.opaque, iocb->ret);
> - }
> + iocb->common.cb(iocb->common.opaque, iocb->ret);
> +
> qemu_bh_delete(iocb->bh);
> iocb->bh = NULL;
> qemu_aio_unref(iocb);
> @@ -462,7 +458,7 @@ static void ide_issue_trim_cb(void *opaque, int ret)
> }
>
> if (!ide_sect_range_ok(s, sector, count)) {
> - iocb->is_invalid = true;
> + iocb->ret = -EINVAL;
> goto done;
> }
>
> @@ -502,7 +498,6 @@ BlockAIOCB *ide_issue_trim(
> iocb->qiov = qiov;
> iocb->i = -1;
> iocb->j = 0;
> - iocb->is_invalid = false;
> ide_issue_trim_cb(iocb, 0);
> return &iocb->common;
> }
> @@ -848,6 +843,12 @@ static void ide_dma_cb(void *opaque, int ret)
> if (ret == -ECANCELED) {
> return;
> }
> +
> + if (ret == -EINVAL) {
> + ide_dma_error(s);
> + return;
> + }
> +
> if (ret < 0) {
> if (ide_handle_rw_error(s, -ret, ide_dma_cmd_to_retry(s->dma_cmd))) {
> s->bus->dma->aiocb = NULL;
>
Thanks, applied to my IDE tree:
https://github.com/jnsnow/qemu/commits/ide
https://github.com/jnsnow/qemu.git
--js
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-03-21 20:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-02 17:08 [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio Anton Nefedov
2018-03-05 21:54 ` Mark Cave-Ayland
2018-03-16 11:20 ` Mark Cave-Ayland
2018-03-16 18:38 ` John Snow
2018-03-21 20:06 ` John Snow
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.