Re: [Qemu-devel] No error report when using the qemu-img.exetoconvert a disk to vmdk format which is saved on a disk that has nomorespace

[Kevin Wolf]

Am 24.09.2015 um 10:01 hat Guangmu Zhu geschrieben:
> Hi Kevin,
> 
> I tried the patch you provide, and I haven't seen that problem yet. If the 
> disk
> space is full, an error will be reported with the message "Invalid argument"
> and the program will stop.
> 
> Will you merge the patch to the master?

I'll post it in a separate email thread as a proper patch now in order
to give people a chance to review it. After that, I'll include it in a
pull request for master, but it might still be a few days until then.

Kevin

> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 68f2338..b562c94 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -119,9 +119,9 @@ static int aio_worker(void *arg)
>      case QEMU_AIO_WRITE:
>          count = handle_aiocb_rw(aiocb);
>          if (count == aiocb->aio_nbytes) {
> -            count = 0;
> +            ret = 0;
>          } else {
> -            count = -EINVAL;
> +            ret = -EINVAL;
>          }
>          break;
>      case QEMU_AIO_FLUSH:
> 
> -------------------------------------------------
> 
> I'll try the patch and report in a week for I'm too busy these days. And if I
> could, I would like to help to maintain the Windows backend.
> 
> Sincerely.
> Guangmu Zhu
> 
> -------------------------------------------------
> 
> Am 23.09.2015 um 13:30 hat Guangmu Zhu geschrieben:
> > If the "BlockDriver" is "bdrv_vmdk", the function "vmdk_co_write" will be
> > called instead. In function "vmdk_write_extent" I see "ret = bdrv_pwrite
> > (extent->file, write_offset, write_buf, write_len);". So the "extend->file"
> is
> > "bdrv_file", is it?
> 
> Yes, exactly. You'll go through bdrv_vmdk first, and then the nested
> call goes to bdrv_file.
> 
> > -------------------------------------------------
> >
> > Correct a mistake:
> > So though the "count" would be "-EINVAL" if error occurred while writing 
> > some
> > file, the return value will always be zero. Maybe I missed something?
> 
> I think you're right. Instead of setting count = 0/-EINVAL in
> aio_worker, we should be setting ret.
> 
> Can you try the patch below and report back?
> 
> > 3. The "bs->drv->bdrv_aio_writev" is function "raw_aio_writev" in file
> > "raw-win32.c" and the quemu-img uses synchronous IO always, so the function
> > "paio_submit" in the same file will be called. This function submits the
> "aio"
> > to "worker_thread" with the callback "aio_worker". There are some codes in
> > "aio_worker":
> >
> >     ssize_t ret = 0;
> >     ......
> >     case QEMU_AIO_WRITE:
> >         count = handle_aiocb_rw(aiocb);
> >         if (count == aiocb->aio_nbytes) {
> >             count = 0;
> >         } else {
> >             count = -EINVAL;
> >         }
> >         break;
> >     ......
> >     return ret;
> 
> Independently of your problem, the code in aio_worker() looks a bit
> fishy, because handle_aiocb_rw() can't distinguish between an error
> and 0 bytes transferred.
> 
> For writes, that probably doesn't matter, but for reads, I think we
> return a successful read of zeroes instead of signalling an error. This
> might need another patch.
> 
> Generally, the Windows backend is not getting a lot of attention and
> could use someone who checks it, cleans it up and fixes bugs.
> 
> Kevin
> 
> 
> diff --git a/block/raw-win32.c b/block/raw-win32.c
> index 68f2338..b562c94 100644
> --- a/block/raw-win32.c
> +++ b/block/raw-win32.c
> @@ -119,9 +119,9 @@ static int aio_worker(void *arg)
>      case QEMU_AIO_WRITE:
>          count = handle_aiocb_rw(aiocb);
>          if (count == aiocb->aio_nbytes) {
> -            count = 0;
> +            ret = 0;
>          } else {
> -            count = -EINVAL;
> +            ret = -EINVAL;
>          }
>          break;
>      case QEMU_AIO_FLUSH: