No error report when using the qemu-img.exetoconvert a disk to vmdk format which is saved on a disk that has nomorespace

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?


Thanks.
Guangmu Zhu

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: