An empty GSList is represented by a NULL pointer, therefore it's a
perfectly valid argument for g_slist_find() and there's no need to
make any additional check.
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Message-id: 1435583533-5758-1-git-send-email-berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

a malicious caller could otherwise specify a very
large value via the URI and force libnfs to allocate
a large amount of memory for the readahead buffer.

Cc: qemu-stable@nongnu.org
Signed-off-by: NPeter Lieven <pl@kamp.de>
Message-id: 1435317241-25585-1-git-send-email-pl@kamp.de
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

RHEL7 and others are stuck with libiscsi 1.9.0 since there
unfortunately was an ABI breakage after that release.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Message-id: 1435313881-19366-1-git-send-email-pl@kamp.de
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Unsetting dirty globally with discard is not very correct. The discard may zero
out sectors (depending on can_write_zeroes_with_unmap), we should replicate
this change to destination side to make sure that the guest sees the same data.

Calling bdrv_reset_dirty also troubles mirror job because the hbitmap iterator
doesn't expect unsetting of bits after current position.

So let's do it the opposite way which fixes both problems: set the dirty bits
if we are to discard it.

Reported-by: wangxiaolong@ucloud.cn
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

If guest discards a source cluster, mirroring with bdrv_aio_readv is overkill.
Some protocols do zero upon discard, where it's best to use
bdrv_aio_write_zeroes, otherwise, bdrv_aio_discard will be enough.
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

If specified as "true", it allows discarding on target sectors where source is
not allocated.
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Like bdrv_is_allocated_above, this function follows the backing chain until seeing
BDRV_BLOCK_ALLOCATED.  Base is not included.

Reimplement bdrv_is_allocated on top.

[Initialized bdrv_co_get_block_status_above() ret to 0 to silence
mingw64 compiler warning about the unitialized variable.  assert(bs !=
base) prevents that case but I suppose the program could be compiled
with -DNDEBUG.
--Stefan]
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

If we wish to make differential backups a feature that's easy to access,
it might be pertinent to rename the "dirty-bitmap" mode to "incremental"
to make it clear what /type/ of backup the dirty-bitmap is helping us
perform.

This is an API breaking change, but 2.4 has not yet gone live,
so we have this flexibility.
Signed-off-by: NJohn Snow <jsnow@redhat.com>
Message-id: 1433463642-21840-2-git-send-email-jsnow@redhat.com
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Fixes a crash during image compression
Signed-off-by: NJindřich Makovička <makovick@gmail.com>
Tested-by: NRichard W.M. Jones <rjones@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

libiscsi starting with 1.15 will properly support timeout of iscsi
commands. The default will remain no timeout, but this can
be changed via cmdline parameters, e.g.:

qemu -iscsi timeout=30 -drive file=iscsi://...

If a timeout occurs a reconnect is scheduled and the timed out command
will be requeued for processing after a successful reconnect.

The required API call iscsi_set_timeout is present since libiscsi
1.10 which was released in October 2013. However, due to some bugs
in the libiscsi code the use is not recommended before version 1.15.

Please note that this patch bumps the libiscsi requirement to 1.10
to have all function and macros defined. The patch fixes also a
off-by-one error in the NOP timeout calculation which was fixed
while touching these code parts.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Message-id: 1434455107-19328-1-git-send-email-pl@kamp.de
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Until now, an SG device was identified only by checking if its path
started with "/dev/sg". Then, hdev_open() would set the bs->sg flag
accordingly. The patch relies on the actual properties of the device
instead of the specified file path.

To this end, test for an SG device (e.g. /dev/sg0) by ensuring that
all of the following holds:

 - The specified file name corresponds to a character device
 - The device supports the SG_GET_VERSION_NUM ioctl
 - The device supports the SG_GET_SCSI_ID ioctl
Signed-off-by: NDimitris Aragiorgis <dimara@arrikto.com>
Message-id: 1435056300-14924-6-git-send-email-dimara@arrikto.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Get rid of several #ifdef DEBUG_FLOPPY and substitute them with
DPRINTF.
Signed-off-by: NDimitris Aragiorgis <dimara@arrikto.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435056300-14924-5-git-send-email-dimara@arrikto.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Building the QEMU tools fails if we #define DEBUG_BLOCK inside
block/raw-posix.c. Here instead of adding qemu-log.o in block-obj-y
so that DEBUG_BLOCK_PRINT can be used, we substitute the latter with
a simple DPRINTF() (that does not cause bit-rot).
Signed-off-by: NDimitris Aragiorgis <dimara@arrikto.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435056300-14924-4-git-send-email-dimara@arrikto.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

During migration, QEMU uses fsync()/fdatasync() on the open file
descriptor for read-write block devices to flush data just before
stopping the VM.

However, fsync() on a scsi-generic device returns -EINVAL which
causes the migration to fail. This patch skips flushing data in case
of an SG device, since submitting SCSI commands directly via an SG
character device (e.g. /dev/sg0) bypasses the page cache completely,
anyway.

Note that fsync() not only flushes the page cache but also the disk
cache. The scsi-generic device never sends flushes, and for
migration it assumes that the same SCSI device is used by the
destination host, so it does not issue any SCSI SYNCHRONIZE CACHE
(10) command.

Finally, remove the bdrv_is_sg() test from iscsi_co_flush() since
this is now redundant (we flush the underlying protocol at the end
of bdrv_co_flush() which, with this patch, we never reach).
Signed-off-by: NDimitris Aragiorgis <dimara@arrikto.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435056300-14924-3-git-send-email-dimara@arrikto.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Instead of checking bs->sg use bdrv_is_sg() consistently throughout
the code.
Signed-off-by: NDimitris Aragiorgis <dimara@arrikto.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435056300-14924-2-git-send-email-dimara@arrikto.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Until now the vvfat volume label was hardcoded to be
"QEMU VVFAT", now you can pass a file.label=labelname option
to the -drive to change it.

The FAT structure defines the volume label to be limited to
11 bytes and is filled up spaces when shorter than that. The
trailing spaces however aren't exposed to the user by
operating systems.

[Added missing comment '#' characters in block-core.json to fix build
errors.
--Stefan]
Signed-off-by: NWolfgang Bumiller <w.bumiller@proxmox.com>
Message-id: 1434706529-13895-2-git-send-email-w.bumiller@proxmox.com
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

This patch introduces the blk_drain() function which allows to replace
blk_drain_all() when only one BlockDriverState needs to be drained.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NAlexander Yarygin <yarygin@linux.vnet.ibm.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Acked-by: NKevin Wolf <kwolf@redhat.com>
Message-id: 1434537440-28236-2-git-send-email-yarygin@linux.vnet.ibm.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Calling throttle_group_config() cancels all timers from a particular
BlockDriverState, so any_timer_armed[] should be updated accordingly.

However, with the current code it may happen that a timer is armed in
a different BlockDriverState from the same group, so any_timer_armed[]
would be set to false in a situation where there is still a timer
armed.

The consequence is that we might end up with two timers armed. This
should not have any noticeable impact however, since all accesses to
the ThrottleGroup are protected by a lock, and the situation would
become normal again shortly thereafter as soon as all timers have been
fired.

The correct way to solve this is to check that we're actually
cancelling a timer before updating any_timer_armed[].
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Message-id: 1434382875-3998-1-git-send-email-berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

After the commit 9b536adc ("block: acquire AioContext in
bdrv_drain_all()") the aio_poll() function got called for every
BlockDriverState, in assumption that every device may have its own
AioContext. If we have thousands of disks attached, there are a lot of
BlockDriverStates but only a few AioContexts, leading to tons of
unnecessary aio_poll() calls.

This patch changes the bdrv_drain_all() function allowing it find shared
AioContexts and to call aio_poll() only for unique ones.

Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NAlexander Yarygin <yarygin@linux.vnet.ibm.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Tested-by: NChristian Borntraeger <borntraeger@de.ibm.com>
Message-id: 1433936297-7098-4-git-send-email-yarygin@linux.vnet.ibm.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

In particular, don't include it into headers.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

Remove it except for two things in qerror.h:

* Two #include to be cleaned up separately to avoid cluttering this
  patch.

* The QERR_ macros.  Mark as obsolete.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

These macros expand into error class enumeration constant, comma,
string.  Unclean.  Has been that way since commit 13f59ae8.

The error class is always ERROR_CLASS_GENERIC_ERROR since the previous
commit.

Clean up as follows:

* Prepend every use of a QERR_ macro by ERROR_CLASS_GENERIC_ERROR, and
  delete it from the QERR_ macro.  No change after preprocessing.

* Rewrite error_set(ERROR_CLASS_GENERIC_ERROR, ...) into
  error_setg(...).  Again, no change after preprocessing.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NLuiz Capitulino <lcapitulino@redhat.com>

We require a C99 compiler, so let's use 'bool' instead of 'int'
when dealing with boolean values.  There are few enough clients
to fix them all in one pass.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Acked-by: NLuiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

When reopening an image, the block layer already takes care to reopen
bs->file as well with recalculated inherited flags. The same must happen
for any other child (most notably missing before this patch: backing
files).

If bs->file (or any other child) didn't originally inherit from bs, e.g.
because it was created separately and then only referenced, it must not
inherit flags on reopen either, so check the inherited_from field before
propagation the reopen down.

VMDK already reopened its extents manually; this code can now be
dropped.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Instead of letting every caller of bdrv_open() determine the right flags
for its child node manually and pass them to the function, pass the
parent node and the role of the newly opened child (like backing file,
protocol layer, etc.).
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>

Besides standardising on a single interface for opening child nodes,
this patch allows the user to specify options to individual extent
nodes. Overriding file names isn't possible with this yet, so it's of
limited usefulness, but still a step forward.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NJeff Cody <jcody@redhat.com>

Besides standardising on a single interface for opening child nodes,
this simplifies the .bdrv_open() implementation of the quorum block
driver by using block layer functionality for handling BlockdevRefs.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NJeff Cody <jcody@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>

Image files with an unaligned image size have a final hole that starts
at EOF, i.e. in the middle of a sector. Currently, *pnum == 0 is
returned when checking the status of this sector. In qemu-img, this
triggers an assertion failure.

In order to fix this, one type for the sector that contains EOF must be
found. Treating a hole as data is safe, so this patch rounds the
calculated number of data sectors up, so that a partial sector at EOF is
treated as a full data sector.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1229394Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Tested-by: NCole Robinson <crobinso@redhat.com>

Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

It has the similar issue with b1649fae. Since the calculation
is repeated for a few times already, introduce a function so it can be
reused.
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

If a relatively large cluster size is chosen, the default of 1 MB L2
cache is not really appropriate. In this case, unless overridden by the
user, the default cache size should not be determined by its size in
bytes but by the number of L2 tables (clusters) it is supposed to
contain.

Note that without this patch, MIN_L2_CACHE_SIZE will effectively take
over the same role. However, providing space for just two L2 tables is
not enough to be the default.
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The L2 cache must cover at least two L2 tables, because during COW two
L2 tables are accessed simultaneously.
Reported-by: NAlexander Graf <agraf@suse.de>
Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: NMax Reitz <mreitz@redhat.com>
Tested-by: NAlexander Graf <agraf@suse.de>
Reviewed-by: NAlberto Garcia <berto@igalia.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 172df91f09c69c6f0440a697bbd1b3f95b077ee4.1433779731.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

bdrv_swap() touches the fields of a BlockDriverState that are
protected by the ThrottleGroup lock. Although those fields end up in
their original place, they are temporarily swapped in the process,
so there's a chance that an operation on a member of the same group
happening on a different thread can try to use them.
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: d92dc40d7c4f1fc5cda5cbbf4ffb7a4670b79d17.1433779731.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

The throttle group support use a cooperative round robin scheduling
algorithm.

The principles of the algorithm are simple:
- Each BDS of the group is used as a token in a circular way.
- The active BDS computes if a wait must be done and arms the right
  timer.
- If a wait must be done the token timer will be armed so the token
  will become the next active BDS.
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: f0082a86f3ac01c46170f7eafe2101a92e8fde39.1433779731.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 2fdb4de17210b733a13eb472c33cd08b45f8fd21.1433779731.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Group throttling will share ThrottleState between multiple bs.
As a consequence the ThrottleState will be accessed by multiple aio
context.

Timers are tied to their aio context so they must go out of the
ThrottleState structure.

This commit paves the way for each bs of a common ThrottleState to
have its own timer.
Signed-off-by: NBenoit Canet <benoit.canet@nodalink.com>
Signed-off-by: NAlberto Garcia <berto@igalia.com>
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-id: 6cf9ea96d8b32ae2f8769cead38f68a6a0c8c909.1433779731.git.berto@igalia.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Image files with an unaligned image size have a final hole that starts
at EOF, i.e. in the middle of a sector. Currently, *pnum == 0 is
returned when checking the status of this sector. In qemu-img, this
triggers an assertion failure.

In order to fix this, one type for the sector that contains EOF must be
found. Treating a hole as data is safe, so this patch rounds the
calculated number of data sectors up, so that a partial sector at EOF is
treated as a full data sector.

This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1229394Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Message-id: 1433840108-9996-1-git-send-email-kwolf@redhat.com
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Acked-by: NKevin Wolf <kwolf@redhat.com>