[PATCH] hw/ide: resolve core crash caused by optical drive pop-up during virtual machine startup

[PATCH] hw/ide: resolve core crash caused by optical drive pop-up during virtual machine startup

[yanlonglong]

Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
---
 hw/ide/atapi.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index a42b748521..d14feed45b 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -153,6 +153,11 @@ static void cd_read_sector_cb(void *opaque, int ret)
 static int cd_read_sector(IDEState *s)
 {
     void *buf;
+    
+    //resolve core crash caused by optical drive pop-up during virtual machine startup
+    if (!blk_is_available(s->blk)) {
+        return -ENOMEDIUM;
+    }
 
     if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
         block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);
-- 
2.43.0

Re: [PATCH] hw/ide: resolve core crash caused by optical drive pop-up during virtual machine startup

[Markus Armbruster]

Reproducer?

Is this a regression?

yanlonglong <yanlonglong@kylinos.cn> writes:

> Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
> ---
>  hw/ide/atapi.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index a42b748521..d14feed45b 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -153,6 +153,11 @@ static void cd_read_sector_cb(void *opaque, int ret)
>  static int cd_read_sector(IDEState *s)
>  {
>      void *buf;
> +    
> +    //resolve core crash caused by optical drive pop-up during virtual machine startup

docs/devel/style.rst:

    We use traditional C-style /``*`` ``*``/ comments and avoid // comments.

> +    if (!blk_is_available(s->blk)) {
> +        return -ENOMEDIUM;
> +    }
>  
>      if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
>          block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);

Re: [PATCH] hw/ide: resolve core crash caused by optical drive pop-up during virtual machine startup

[Philippe Mathieu-Daudé]

Hi,

Cc'ing maintainers:

$ ./scripts/get_maintainer.pl -f hw/ide/atapi.c
John Snow <jsnow@redhat.com> (odd fixer:IDE)
qemu-block@nongnu.org (open list:IDE)
qemu-devel@nongnu.org (open list:All patches CC here)

On 20/3/26 03:07, yanlonglong wrote:
> Signed-off-by: yanlonglong <yanlonglong@kylinos.cn>
> ---
>   hw/ide/atapi.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index a42b748521..d14feed45b 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -153,6 +153,11 @@ static void cd_read_sector_cb(void *opaque, int ret)
>   static int cd_read_sector(IDEState *s)
>   {
>       void *buf;
> +
> +    //resolve core crash caused by optical drive pop-up during virtual machine startup
> +    if (!blk_is_available(s->blk)) {
> +        return -ENOMEDIUM;
> +    }

IMHO we shouldn't reach this because drive ejection should cancel any
running transfer (with ide_transfer_stop IIRC). We could assert here:

        assert(blk_is_available(s->blk));

to check the missing cancel transfer path.

>       if (s->cd_sector_size != 2048 && s->cd_sector_size != 2352) {
>           block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_READ);