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

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

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

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?


Sorry.
Guangmu Zhu
-------------------------------------------------


Thanks for your reply.


I read the source code again and have some question:


1. qume-img wrote the target file in the function "bdrv_aligned_pwritev" which called the "drv->bdrv_co_writev" function. I haven't know how the qume-img found the right driver for "drv", but I guessed it's "BlockDriver bdrv_file" for the file on Windows, is it?

2. If the above is correct, the member "bdrv_aio_writev" of "BlockDriver" is the function "raw_aio_writev" while the member "bdrv_co_writev" is NULL. The function "bdrv_register" registers the "BlockDriver" and checks whether the "bdrv_co_readv" or "bdrv_aio_readv" is NULL. If the "bdrv_co_readv" is NULL, the member "bdrv_co_writev" is "bdrv_co_writev_em" which calls the function "bdrv_co_io_em". There are some codes of it:

static int coroutine_fn bdrv_co_io_em(BlockDriverState *bs, int64_t sector_num,
                                      int nb_sectors, QEMUIOVector *iov,
                                      bool is_write)
{
    CoroutineIOCompletion co = {
        .coroutine = qemu_coroutine_self(),
    };
    BlockAIOCB *acb;


    if (is_write) {
        acb = bs->drv->bdrv_aio_writev(bs, sector_num, iov, nb_sectors,
                                       bdrv_co_io_em_complete, &co);
    } else {
        acb = bs->drv->bdrv_aio_readv(bs, sector_num, iov, nb_sectors,
                                      bdrv_co_io_em_complete, &co);
    }


    trace_bdrv_co_io_em(bs, sector_num, nb_sectors, is_write, acb);
    if (!acb) {
        return -EIO;
    }
    qemu_coroutine_yield();


    return co.ret;
}



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;


So though the "count" would be zero if error occurred while writing some file, the return value will always be zero. Maybe I missed something?


4. The function "worker_thread" calls the callback:


        ret = req->func(req->arg);


        req->ret = ret;
        /* Write ret before state.  */
        smp_wmb();
        req->state = THREAD_DONE;



        qemu_mutex_lock(&pool->lock);


        qemu_bh_schedule(pool->completion_bh);



The "pool->completion_bh" is function "thread_pool_completion_bh", which calls "elem->common.cb(elem->common.opaque, elem->ret);". And the "elem->common.cb" is function "bdrv_co_io_em_complete":


static void bdrv_co_io_em_complete(void *opaque, int ret)
{
    CoroutineIOCompletion *co = opaque;


    co->ret = ret;
    qemu_coroutine_enter(co->coroutine, NULL);
}



5. Finally the return value(zero) will be stored in "co.ret" in function "bdrv_co_io_em". However, what would happen if the req haven't done but the function "bdrv_co_io_em" returned? The write operation would return an uninitialized value(is it zero? I don't know.), is it?


Maybe these made the program report nothing with writer errors, I think.


Thanks again in advance.
Guangmu Zhu


-------------------------------------------------
Am 22.09.2015 um 08:09 hat Guangmu Zhu geschrieben:
> I used the qemu-img.exe to convert a disk to vmdk format and the output file
> size could be 300 MB. However the left space of the disk the output file
> located on was about 200 MB. After a while, the left space had been zero but
> the program didn't stop or report any error. It was just going on as normal.
> 
> I read the source code and found the error report was controlled by
> "BlockdevOnError on_read_error, on_write_error" in "struct BlockDriverState",
> which had the default value "BLOCKDEV_ON_ERROR_REPORT" for "on_read_error" and
> "BLOCKDEV_ON_ERROR_ENOSPC" for "on_writer_error". The qemu-img.exe had no
> option to change the default behavior of the error report.
> 
> So I think if there were some ways to change the default value of the error
> report, it might be better. Further more, I suggest we could just add some
> codes to the "img_convert" function:
> 
>       1827:    out_blk = img_open("target", out_filename, out_fmt, flags, true,
> quiet);
>       1828:    if (!out_blk) {
>       1829:        ret = -1;
>       1830:        goto out;
>       1831:    }
>       1832:    out_bs = blk_bs(out_blk);
> ++ 1833:
> ++ 1834:    bdrv_set_on_error
> (out_bs, BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT);

This shouldn't make any difference for qemu-img. The error handling mode
is only for emulated devices in qemu proper.

It looks more like VMDK is somehow failing to report an error at all whn
it's running out of free disk space (though I couldn't spot an error in
the code at first sight).

Kevin

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