If the bdrv_read() of the snapshot's L1 table fails, return the right
error code and make sure that the old L1 table is still loaded and we
don't break the BlockDriverState completely.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
First the snapshot must be deleted and only then the refcounts can be
decreased.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
The refcount updates must be moved so that in the worst case we can get
cluster leaks, but refcounts may never be too low.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Besides fixing the return code, this adds some comments that make clear
how the code works and that it potentially breaks images if we fail in
the wrong place. Actually fixing this is left for the next patch.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Increase refcounts only after allocating a new L1 table has succeeded in
order to make leaks less likely. If writing the snapshot table fails,
revert in-memory state to be consistent with that on disk.
While at it, make it return the real error codes instead of -1.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
sn->id_str could be leaked before this. The rest of this patch changes
comments, fixes coding style or removes checks that are unnecessary with
g_malloc.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Failing in the middle wouldn't help with the integrity of the image, so
doing everything in a single request seems better.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Doesn't immediately fix anything as the callers don't use the return
value, but they will be fixed next.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Looks better when reviewing these source files.
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Since common file operation functions lack of error detection,
so change them to bdrv series functions.
Signed-off-by: Li Zhi Hui <zhihuili@linux.vnet.ibm.com>
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Xen_disk.c has support for using synchronous I/O instead of asynchronous,
but it is compiled out by default. Remove it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This patch is only to refactor some lines of codes to get better and more robust codes.
As you have seen, in qed_read_table_cb() it's nice to
use qiov->size because that function doesn't obviously use a single
struct iovec.
In other two functions, if qiov use more than one struct iovec, the existing way will get wrong nb_sectors.
To make the code more robust, it will be nicer to refactor the existing way as below.
Signed-off-by: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
Acked-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
A BlockDriverState should not issue requests on itself through the
public block layer interface. Nested, or reentrant, requests are
problematic because they do I/O throttling and request tracking twice.
Features like block layer copy-on-read use request tracking to avoid
race conditions between concurrent requests. The reentrant request will
have to "wait" for its parent request to complete. But the parent is
waiting for the reentrant request to make progress so we have reached
deadlock.
The solution is for block drivers to avoid the public block layer
interfaces for reentrant requests. Instead they should call their own
internal functions if they wish to perform reentrant requests.
This is also a good opportunity to make copy_sectors() a true
coroutine_fn. That means calling bdrv_co_writev() instead of
bdrv_write(). Behavior is unchanged but we're being explicit that this
executes in coroutine context.
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Unlocking during COW allows for more parallelism. One change it requires is
that buffers are dynamically allocated instead of just using a per-image
buffer.
While touching the code, drop the synchronous qcow2_read() function and replace
it by a bdrv_read() call.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Fsdriver callback that operate on file descriptor need to
differentiate between directory fd and file fd.
Based on the original patch from Sassan Panahinejad <sassan@sassan.me.uk>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
As per the 9p rfc, during TVERSION its necessary to clean all the active
fids, so that we start the session from a clean state. Its also needed in
scenarios where the guest is booting off 9p, and boot fails, and client
restarts, without any knowledge of the past, it will issue a TVERSION again
so this ensures that we always start from a clean state.
Signed-off-by: Deepak C Shetty <deepakcs@linux.vnet.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Now when you try to migrate with VirtFS export path mounted, you get a proper QMP error:
(qemu) migrate tcp:localhost:4444
Migration is disabled when VirtFS export path '/tmp/' is mounted in the guest using mount_tag 'v_tmp'
(qemu)
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
handle fs driver require a set of newly added syscalls. Don't
Compile handle FS driver if those syscalls are not available.
Instead of adding #ifdef for all those syscalls we check for
open by handle syscall. If that is available then rest of the
syscalls used by the driver should be available.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Edgar E. Iglesias <edgar.iglesias@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Alexander Graf <agraf@suse.de>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Blue Swirl <blauwirbel@gmail.com>
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
This was spotted by cppcheck.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Just for cleanliness; it would take a truly gigantic cursor to break.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
Generally $(BUILD_DIR) == $(CURDIR), but that isn't necessarilly the
case, so use $(BUILD_DIR)/qapi-generated for generated files to
avoid potentionally sticking generating files in odd places outside
the build's include paths.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Fix a bug in handling dotted paths, and exclude directory prefixes
from generated guardnames to avoid odd/pseudo-random guardnames in
generated headers.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
ATR size exceeding the limit is diagnosed, but then we merrily use it
anyway, overrunning card->atr[].
The message is read from a character device. Obvious security
implications unless the other end of the character device is trusted.
Spotted by Coverity. CVE-2011-4111.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
This reverts commit be85c90b74.
This patch is incorrect and breaks the build with a freshly cloned git tree.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
--*dir) option pattern precede --{en,dis}able-usb-redir) patterns in the
option analysis switch, making the latter options have no effect.
There were some --*dir that are supported by Autoconf and not by QEMU configure.
The aim was to let QEMU packagers use the rpm (or similar) macro that overrides
directories for their distribution.
Replace --*dir with exact option names.
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
strtosz() & friends require the size to be at the end of the string,
or be followed by whitespace or ','. I find this surprising, because
the name suggests it works like strtol().
The check simplifies callers that accept exactly that follow set
slightly. No such callers exist.
The check is redundant for callers that accept a smaller follow set,
and thus need to check themselves anyway. Right now, this is the case
for all but one caller. All of them neglected to check, or checked
incorrectly, but the previous few commits fixed them up.
Finally, the check is problematic for callers that accept a larger
follow set. This is the case in monitor_parse_command().
Fortunately, the problems there are relatively harmless.
monitor_parse_command() uses strtosz() for argument type 'o'. When
the last argument is of type 'o', a trailing ',' is diagnosed
differently than other trailing junk:
(qemu) migrate_set_speed 1x
invalid size
(qemu) migrate_set_speed 1,
migrate_set_speed: extraneous characters at the end of line
A related inconsistency exists with non-last arguments. No such
command exists, but let's use memsave to explore the inconsistency.
The monitor permits, but does not require whitespace between
arguments. For instance, "memsave (1-1)1024foo" is parsed as command
memsave with three arguments 0, 1024 and "foo". Yes, this is daft,
but at least it's consistently daft.
If I change memsave's second argument from 'i' to 'o', then "memsave
(1-1)1foo" is rejected, because the size is followed by an 'f'. But
"memsave (1-1)1," is still accepted, and duly saves to file ",".
We don't have any users of strtosz that profit from the check. In the
users we have, it appears to encourage sloppy error checking, or gets
in the way. Drop the bothersome check.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
strtosz_suffix() fails unless the size is followed by 0, whitespace or
','. Useless here, because we need to fail for any junk following the
size, even if it starts with whitespace or ','. Check manually.
Things like "qemu-img create xxx 1024," and "qemu-img convert -S '1024
junk'" are now caught.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
cpu_x86_find_by_name() uses strtosz_suffix_unit(), but screws up the
error checking. It detects some failures, but not all. Undetected
failures result in a zero tsc_khz value (error value -1 divided by
1000), which means "no tsc_freq set".
To reproduce, try "-cpu qemu64,tsc_freq=9999999T".
strtosz_suffix_unit() fails, because the value overflows int64_t,
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
strtosz_suffix() fails unless the size is followed by 0, whitespace or
','. Useless here, because we need to fail for any junk following the
size, even if it starts with whitespace or ','. Check manually.
Things like "-m 1024," are now caught.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
