Bug 1034433

This also fails with upstream qemu.  'git bisect' says:

ba2ab2f2ca4150a7e314fbb19fa158bd8ddc36eb is the first bad commit
commit ba2ab2f2ca4150a7e314fbb19fa158bd8ddc36eb
Author: Max Reitz <mreitz>
Date:   Thu Oct 24 20:35:06 2013 +0200

    qcow2: Flush image after creation
    
    Opening the qcow2 image with BDRV_O_NO_FLUSH prevents any flushes during
    the image creation. This means that the image has not yet been flushed
    to disk when qemu-img create exits. This flush is delayed until the next
    operation on the image involving opening it without BDRV_O_NO_FLUSH and
    closing (or directly flushing) it. For large images and/or images with a
    small cluster size and preallocated metadata, this flush may take a
    significant amount of time and may occur unexpectedly.
    
    Reopening the image without BDRV_O_NO_FLUSH right before the end of
    qcow2_create2() results in hoisting the potentially costly flush into
    the image creation, which is expected to take some time (whereas
    successive image operations may be not).
    
    Signed-off-by: Max Reitz <mreitz>
    Reviewed-by: Eric Blake <eblake>
    Reviewed-by: Benoit Canet <benoit>
    Signed-off-by: Kevin Wolf <kwolf>

I've confirmed this that the commit in comment 1 is the first
one which causes the hang in qemu-img.  Max, any ideas?

BTW I don't even agree with the premise of this commit.

In libguestfs we usually want to *avoid* flushing anything to
disk if at all possible, because the majority of images we're dealing
with are transient and throwaway.  Don't force syncing and flushing
behaviour.  If people want they can call sync/fsync themselves.

CCing kevin as well. Kevin, summary: there's a qemu-img regression queued for 1.7, Rich bisected it Comment #1

We're too late now for 1.7.0.

The symptoms don't ring a bell, it looks rather odd that a backing file can make
it hang. I guess Max should just debug it.

(In reply to Richard W.M. Jones from comment #3)
> BTW I don't even agree with the premise of this commit.
> 
> In libguestfs we usually want to *avoid* flushing anything to
> disk if at all possible, because the majority of images we're dealing
> with are transient and throwaway.  Don't force syncing and flushing
> behaviour.  If people want they can call sync/fsync themselves.

Hm... So you create the image and then only work with cache=unsafe on it?

> > In libguestfs we usually want to *avoid* flushing anything to
> > disk if at all possible, because the majority of images we're dealing
> > with are transient and throwaway.  Don't force syncing and flushing
> > behaviour.  If people want they can call sync/fsync themselves.
> 
> Hm... So you create the image and then only work with cache=unsafe on it?

For transient stuff that can be easily recreated, yes.

For example, virt-builder (building new images from scratch)
will do everything with cache=unsafe.  The final step is to
call fsync on the file[1] to commit it to disk before exiting
with a 0 status.

Provided a program using virt-builder checks the exit status
this is safe.

[1] Although this is only necessary because of the default
of qemu using cache=none.  If people know what they are doing
they can disable this fsync in virt-builder.

(ie. see --no-sync option and notes here:
http://libguestfs.org/virt-builder.1.html )

The issue's cause is apparently that bdrv_img_create opens the backing file and does not close it before it calls bdrv_create. Most block drivers open an empty file, write to it, and then close it when the image has been created. The aforementioned commit makes qcow2 close the complete image file and reopen it, but now it contains data indicating it is actually backed.

This results in the NBD client trying to maintain two connections to the NBD server (one from bdrv_img_create and one from the bdrv_open in qcow2_create2). The NBD server does not accept the second connection since it allows only one at a time. qemu-img create will accordingly hang in some kind of a deadlock.

I have sent a patch which makes bdrv_img_create close the backing file before calling bdrv_create, fixing the issue.

As for flushing at all: I think, there should be an option to specify the caching mode used by bdrv_(img_)create.

On second thought, it is probably not a good idea to take this bug if it isn't for RHEL, thus I reverted the assignee.

Latest patch is here:

https://lists.gnu.org/archive/html/qemu-devel/2013-11/msg04203.html

Fixed in qemu-1.7.0-1 built now for rawhide