Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

2018-03-21 Thread John Snow 


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 
> ---
>  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 >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

Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

2018-03-16 Thread John Snow 


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 
>>> ---
>>>   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 >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 
> 
> 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

Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

2018-03-16 Thread Mark Cave-Ayland 

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 
---
  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 >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 


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.

Re: [Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

2018-03-05 Thread Mark Cave-Ayland 

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 
---
  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 >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 


ATB,

Mark.

[Qemu-devel] [PATCH] ide: fix invalid TRIM range abortion for macio

2018-03-02 Thread 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 
---
 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 >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