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

["Guangmu Zhu" <guangmuzhu@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3438 bytes --]

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


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:

[-- Attachment #2: Type: text/html, Size: 5454 bytes --]