Skip to content
Snippets Groups Projects
  1. Apr 30, 2019
    • Alberto Garcia's avatar
      qcow2: Fix error handling in the compression code · e1f4a37a
      Alberto Garcia authored
      
      This patch fixes a few things in the way error codes are handled in
      the qcow2 compression code:
      
      a) qcow2_co_pwritev_compressed() expects qcow2_co_compress() to only
         return -1 or -2 on failure, but this is not correct. Since the
         change from qcow2_compress() to qcow2_co_compress() in commit
         ceb029cd the new code can also return -EINVAL (although
         there does not seem to exist any code path that would cause that
         error in the current implementation).
      
      b) -1 and -2 are ad-hoc error codes defined in qcow2_compress().
         This patch replaces them with standard constants from errno.h.
      
      c) Both qcow2_compress() and qcow2_co_do_compress() return a negative
         value on failure, but qcow2_co_pwritev_compressed() stores the
         value in an unsigned data type.
      
      Signed-off-by: default avatarAlberto Garcia <berto@igalia.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      e1f4a37a
    • Kevin Wolf's avatar
      qcow2: Fix qcow2_make_empty() with external data file · db04524f
      Kevin Wolf authored
      
      make_completely_empty() is an optimisated path for bdrv_make_empty()
      where completely new metadata is created inside the image file instead
      of going through all clusters and discarding them. For an external data
      file, however, we actually need to do discard operations on the data
      file; just overwriting the qcow2 file doesn't get rid of the data.
      
      The necessary slow path with an explicit discard operation already
      exists for other cases. Use it for external data files, too.
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      db04524f
    • Hanna Reitz's avatar
      qemu-img: Make create hint at protocol options · f4619af0
      Hanna Reitz authored
      
      qemu-img create allows giving just a format and "-o help" to get a list
      of the options supported by that format.  Users may not realize that the
      protocol level may offer even more options, which they only get to see
      by specifying a filename.
      
      This patch adds a note to hint at that fact.
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      f4619af0
    • Hanna Reitz's avatar
      iotests: Perform the correct test in 082 · 0ef5a8e6
      Hanna Reitz authored
      
      In the "amend" section of 082, we perform a single "convert" test
      (namely "convert -o help").  That does not make sense, especially
      because we have done exactly that "convert" test earlier in 082 already.
      
      Replacing "convert" by "amend" yields an error, which is correct because
      there is no point in "amend" having a default format.  The user has to
      either specify the format, or give a file for qemu-img to probe.
      
      Signed-off-by: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      0ef5a8e6
    • Kevin Wolf's avatar
      qcow2: Fix full preallocation with external data file · 718c0fce
      Kevin Wolf authored
      
      preallocate_co() already gave the data file the full size without
      forwarding the requested preallocation mode to the protocol. When
      bdrv_co_truncate() was called later with the preallocation mode, the
      file didn't actually grow any more, so the data file stayed unallocated
      even if full preallocation was requested.
      
      Pass the right preallocation mode to preallocate_co() and remove the
      second bdrv_co_truncate() to fix this. As a side effect, the ugly
      one-byte write in preallocate_co() is replaced with a truncate call,
      now leaving the last block unallocated on the protocol level as it
      should be.
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      718c0fce
    • Kevin Wolf's avatar
      qcow2: Add errp to preallocate_co() · 360bd074
      Kevin Wolf authored
      
      We'll add a bdrv_co_truncate() call in the next patch which can return
      an Error that we don't want to discard. So add an errp parameter to
      preallocate_co().
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      360bd074
    • Kevin Wolf's avatar
      qcow2: Avoid COW during metadata preallocation · f29fbf7c
      Kevin Wolf authored
      
      Limiting the allocation to INT_MAX bytes isn't particularly clever
      because it means that the final cluster will be a partial cluster which
      will be completed through a COW operation. This results in unnecessary
      data read and write requests which lead to an unwanted non-sparse
      filesystem block for metadata preallocation.
      
      Align the maximum allocation size down to the cluster size to avoid this
      situation.
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      f29fbf7c
    • Eric Blake's avatar
      qemu-img: Saner printing of large file sizes · de38b500
      Eric Blake authored
      Disk sizes close to INT64_MAX cause overflow, for some pretty
      ridiculous output:
      
        $ ./nbdkit -U - memory size=$((2**63 - 512)) --run 'qemu-img info $nbd'
        image: nbd+unix://?socket=/tmp/nbdkitHSAzNz/socket
      
      
        file format: raw
        virtual size: -8388607T (9223372036854775296 bytes)
        disk size: unavailable
      
      But there's no reason to have two separate implementations of integer
      to human-readable abbreviation, where one has overflow and stops at
      'T', while the other avoids overflow and goes all the way to 'E'. With
      this patch, the output now claims 8EiB instead of -8388607T, which
      really is the correct rounding of largest file size supported by qemu
      (we could go 511 bytes larger if we used byte-accurate sizing instead
      of rounding up to the next sector boundary, but that wouldn't change
      the human-readable result).
      
      Quite a few iotests need updates to expected output to match.
      
      Reported-by: default avatarRichard W.M. Jones <rjones@redhat.com>
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Tested-by: default avatarRichard W.M. Jones <rjones@redhat.com>
      Reviewed-by: default avatarAlberto Garcia <berto@igalia.com>
      Reviewed-by: default avatarVladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Tested-by: default avatarMax Reitz <mreitz@redhat.com>
      de38b500
    • Eric Blake's avatar
      cutils: Fix size_to_str() on 32-bit platforms · 754da867
      Eric Blake authored
      
      When extracting a human-readable size formatter, we changed 'uint64_t
      div' pre-patch to 'unsigned long div' post-patch. Which breaks on
      32-bit platforms, resulting in 'inf' instead of intended values larger
      than 999GB.
      
      Fixes: 22951aaa
      CC: qemu-stable@nongnu.org
      Reported-by: default avatarMax Reitz <mreitz@redhat.com>
      Signed-off-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      754da867
    • Stefano Garzarella's avatar
      block/vhdx: Use IEC binary prefixes for size constants · 0cb98af2
      Stefano Garzarella authored
      
      Using IEC binary prefixes in order to make the code more readable,
      with the exception of DEFAULT_LOG_SIZE because it's passed to
      stringify().
      
      Signed-off-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      0cb98af2
    • Stefano Garzarella's avatar
      block/vhdx: Remove redundant IEC binary prefixes definition · e9991e29
      Stefano Garzarella authored
      
      IEC binary prefixes are already defined in "qemu/units.h",
      so we can remove redundant definitions in "block/vhdx.h".
      
      Signed-off-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      Reviewed-by: default avatarPhilippe Mathieu-Daudé <f4bug@amsat.org>
      Reviewed-by: default avatarJohn Snow <jsnow@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      e9991e29
    • Sam Eiderman's avatar
      vmdk: Set vmdk parent backing_format to vmdk · 7502be83
      Sam Eiderman authored
      
      Commit b69864e5 ("vmdk: Support version=3 in VMDK descriptor files")
      fixed the probe function to correctly guess vmdk descriptors with
      version=3.
      
      This solves the issue where vmdk snapshot with parent vmdk descriptor
      containing "version=3" would be treated as raw instead vmdk.
      
      In the future case where a new vmdk version is introduced, we will again
      experience this issue, even if the user will provide "-f vmdk" it will
      only apply to the tip image and not to the underlying "misprobed" parent
      image.
      
      The code in vmdk.c already assumes that the backing file of vmdk must be
      vmdk (see vmdk_is_cid_valid which returns 0 if backing file is not
      vmdk).
      
      So let's make it official by supplying the backing_format as vmdk.
      
      Reviewed-by: default avatarMark Kanda <mark.kanda@oracle.com>
      Reviewed-By: default avatarLiran Alon <liran.alon@oracle.com>
      Reviewed-by: default avatarArbel Moshe <arbel.moshe@oracle.com>
      Signed-off-by: default avatarShmuel Eiderman <shmuel.eiderman@oracle.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarFam Zheng <fam@euphon.net>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      7502be83
    • Zhengui Li's avatar
      vpc: unlock Coroutine lock to make IO submit Concurrently · 126734c4
      Zhengui Li authored
      
      Concurrent IO becomes serial IO because of the qemu Coroutine lock,
      which reduce IO performance severely.
      
      So unlock Coroutine lock before bdrv_co_pwritev and
      bdrv_co_preadv to fix it.
      
      Signed-off-by: default avatarZhengui li <lizhengui@huawei.com>
      Reviewed-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      126734c4
    • Kevin Wolf's avatar
      block: Fix AioContext switch for bs->drv == NULL · 1bffe1ae
      Kevin Wolf authored
      Even for block nodes with bs->drv == NULL, we can't just ignore a
      bdrv_set_aio_context() call. Leaving the node in its old context can
      mean that it's still in an iothread context in bdrv_close_all() during
      shutdown, resulting in an attempted unlock of the AioContext lock which
      we don't hold.
      
      This is an example stack trace of a related crash:
      
       #0  0x00007ffff59da57f in raise () at /lib64/libc.so.6
       #1  0x00007ffff59c4895 in abort () at /lib64/libc.so.6
       #2  0x0000555555b97b1e in error_exit (err=<optimized out>, msg=msg@entry=0x555555d386d0 <__func__.19059> "qemu_mutex_unlock_impl") at util/qemu-thread-posix.c:36
       #3  0x0000555555b97f7f in qemu_mutex_unlock_impl (mutex=mutex@entry=0x5555568002f0, file=file@entry=0x555555d378df "util/async.c", line=line@entry=507) at util/qemu-thread-posix.c:97
       #4  0x0000555555b92f55 in aio_context_release (ctx=ctx@entry=0x555556800290) at util/async.c:507
       #5  0x0000555555b05cf8 in bdrv_prwv_co (child=child@entry=0x7fffc80012f0, offset=offset@entry=131072, qiov=qiov@entry=0x7fffffffd4f0, is_write=is_write@entry=true, flags=flags@entry=0)
               at block/io.c:833
       #6  0x0000555555b060a9 in bdrv_pwritev (qiov=0x7fffffffd4f0, offset=131072, child=0x7fffc80012f0) at block/io.c:990
       #7  0x0000555555b060a9 in bdrv_pwrite (child=0x7fffc80012f0, offset=131072, buf=<optimized out>, bytes=<optimized out>) at block/io.c:990
       #8  0x0000555555ae172b in qcow2_cache_entry_flush (bs=bs@entry=0x555556810680, c=c@entry=0x5555568cc740, i=i@entry=0) at block/qcow2-cache.c:51
       #9  0x0000555555ae18dd in qcow2_cache_write (bs=bs@entry=0x555556810680, c=0x5555568cc740) at block/qcow2-cache.c:248
       #10 0x0000555555ae15de in qcow2_cache_flush (bs=0x555556810680, c=<optimized out>) at block/qcow2-cache.c:259
       #11 0x0000555555ae16b1 in qcow2_cache_flush_dependency (c=0x5555568a1700, c=0x5555568a1700, bs=0x555556810680) at block/qcow2-cache.c:194
       #12 0x0000555555ae16b1 in qcow2_cache_entry_flush (bs=bs@entry=0x555556810680, c=c@entry=0x5555568a1700, i=i@entry=0) at block/qcow2-cache.c:194
       #13 0x0000555555ae18dd in qcow2_cache_write (bs=bs@entry=0x555556810680, c=0x5555568a1700) at block/qcow2-cache.c:248
       #14 0x0000555555ae15de in qcow2_cache_flush (bs=bs@entry=0x555556810680, c=<optimized out>) at block/qcow2-cache.c:259
       #15 0x0000555555ad242c in qcow2_inactivate (bs=bs@entry=0x555556810680) at block/qcow2.c:2124
       #16 0x0000555555ad2590 in qcow2_close (bs=0x555556810680) at block/qcow2.c:2153
       #17 0x0000555555ab0c62 in bdrv_close (bs=0x555556810680) at block.c:3358
       #18 0x0000555555ab0c62 in bdrv_delete (bs=0x555556810680) at block.c:3542
       #19 0x0000555555ab0c62 in bdrv_unref (bs=0x555556810680) at block.c:4598
       #20 0x0000555555af4d72 in blk_remove_bs (blk=blk@entry=0x5555568103d0) at block/block-backend.c:785
       #21 0x0000555555af4dbb in blk_remove_all_bs () at block/block-backend.c:483
       #22 0x0000555555aae02f in bdrv_close_all () at block.c:3412
       #23 0x00005555557f9796 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4776
      
      The reproducer I used is a qcow2 image on gluster volume, where the
      virtual disk size (4 GB) is larger than the gluster volume size (64M),
      so we can easily trigger an ENOSPC. This backend is assigned to a
      virtio-blk device using an iothread, and then from the guest a
      'dd if=/dev/zero of=/dev/vda bs=1G count=1' causes the VM to stop
      because of an I/O error. qemu_gluster_co_flush_to_disk() sets
      bs->drv = NULL on error, so when virtio-blk stops the dataplane, the
      block nodes stay in the iothread AioContext. A 'quit' monitor command
      issued from this paused state crashes the process.
      
      Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1631227
      
      
      Cc: qemu-stable@nongnu.org
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      Reviewed-by: default avatarEric Blake <eblake@redhat.com>
      Reviewed-by: default avatarMax Reitz <mreitz@redhat.com>
      Reviewed-by: default avatarStefano Garzarella <sgarzare@redhat.com>
      1bffe1ae
    • Thomas Huth's avatar
      tests/qemu-iotests: Fix output of qemu-io related tests · 36b9986b
      Thomas Huth authored
      
      One of the recent commits changed the way qemu-io prints out its
      errors and warnings - they are now prefixed with the program name.
      We've got to adapt the iotests accordingly to prevent that they
      are failing.
      
      Fixes: 99e98d7c ("qemu-io: Use error_[gs]et_progname()")
      Signed-off-by: default avatarThomas Huth <thuth@redhat.com>
      Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
      36b9986b
  2. Apr 29, 2019
Loading