The blktapctrl process is responsible for spawning individual tapdisk
processes. It does this using the 'system' method, but unfortunately
none of its file descriptors have the close-on-exec flag set. The
parent blktapctl process opens a couple of unix domain sockets
per-tapdisk it spawns. So the first tapdisk get 2 FDs leaked to it,
the second gets 4 FDs leaked to it, the 3rd gets 6 and so on. The use
of 'system' also unneccessarily invokes the shell.
Replace system with fork/execvp, and explicitly close all file handles
up to _SC_OPEN_MAX.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Fixes correctness issues with xenstored and gcc-4.2.
Original patch by Charles Coffing <ccoffing@novell.com>
Signed-off-by: Keir Fraser <keir@xensource.com>
blktap requires a xen specific kernel AIO ABI which has been vetoed by
upstream in favour of another approach. Rather than include this ABI,
Fedora has been carrying a patch which makes tap:aio use a thread to
poll for aio events and notify the main thread via a pipe.
The upstream approach of allowing io_getevents() poll normal file
descriptors via epoll is still progressing:
http://lkml.org/lkml/2007/1/3/16
but when that does make it upstream, blktap will require significant
re-working to use that approach.
In the meantime, here's a patch which uses the poll-in-a-thread
approach only if AIO poll support isn't available. It also hides the
details behind a simple abstraction and makes both tap:aio and
tap:qcow use it.
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
Note that some of the existing error checking for asprintf was wrong
for Linux. asprintf in glibc returns -1 on error, but leaves the
pointer uninitialized. Only the BSDs zero out the pointer on error.
Also, while fixing these warnings I saw several error paths that were
incorrect. This patch minimally fixes the warn_unused_result; more
complete error path cleanup will be a later patch.
Signed-off-by: Charles Coffing <ccoffing@novell.com>
The only non-obvious change here is in tools/xenfb/Makefile. This now picks up
the headers it requires via the copy in tools/libxc.
Signed-off-by: Ian Campbell <ian.campbell@xensource.com>
When compiling the various blktap drivers with warnings cranked up, I
get errors that return values are not checked.
Signed-off-by: Charles Coffing <ccoffing@novell.com>
blktapctrl already prints out exits reasons for all goto's to
open_failed except of not finding a device number for blktap0
This patch adds just that message so the user will get more info than
just "Unable to start blktapctrl"
Common source of this issue is that blktap is missing/failed to
load/...
Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
In blktap's qcow we need split up read/write requests if the requests
span multiple clusters. However, with our MAX_AIO_REQUESTS define we
assume that there is only ever a single aio request per tapdisk
request and under heavy i/o we can run out of room causing us to
cancel requests.
Dynamically allocate (based on cluster_bits) the various io request
queues the driver maintains.
Signed-off-by: Mark McLoughlin <markmc@redhat.com>
qcow2raw would hang the conversion just short of 100%, because the
final write queue was never being submitted. If a blktap drivers read
synchronously, then "submit_events" and "complete" variables are
checked before getting set, so the fact that reads have finished is
not noticed, so the final write queue was never submitted.
Note that I have also removed the variables write_complete and
read_complete. They were unused, and they were also not set correctly
in the synchronous case.
Signed-off-by: Charles Coffing <ccoffing@novell.com>
Enables tapdisk to chain an arbitrary number of VDIs, propagating reads of
holes in children to their parent images. Introduces two new functions to the
tapdisk interface to facilitate this. Modifies the QCoW plugin to take
advantage of these changes, thus providing support for arbitrarily long chains
of QCoW image types.
For environments where space must be guaranteed in advance
use the -p flag to remove sparseness from the qcow file.
Signed-off-by: Julian Chesterfield <julian@xensource.com>
with I/O errors.
There are a number of flaws in the blktap userspace daemon when dealing
with I/O errors.
- The backends which use AIO check the io_events.res member to determine
if an I/O error occurred. Which is good. But when calling the callback
to signal completion of the I/O, they pass the io_events.res2 member
Now this seems fine at first glance[1]
"res is the usual result of an I/O operation: the number of bytes
transfered, or a negative error code. res2 is a second status
value which will be returned to the user"
Except that
"currently (2.6.0-test9), callers of aio_complete() within the
kernel always set res2 to zero."
And this hasn't changed anytime since 2.6.0, so by passing through
the status from 'res2', the callback thinks the I/O operation succeeded
even when it failed :-(
The fix is simple instead of passing 'res2', just pass
ep->res == io->u.c.nbytes ? 0 : 1
This would solve the error reporting to the guest, except that there
is a second flaw...
- The tapdisk I/O completion callback checks the status parameter
passed in, syslog's it and then returns. It never bothers to send
the I/O completion response back to the blktap kernel driver when
a failure occurrs.
Fortunately the fix for this is also simple. Instead of returning
from the callback when dealing with an error, we simply toggle the
status field for the pending response to BLKIF_RSP_ERROR and then
continue with the normal codepath. So the error eventually gets
back to the guest.
The scenario I used to discover the problem and test the patch is thus:
- In dom0 create a filesystem with only 200 MB of free space
- Create a 1 GB sparse file on this volume.
- Configure the guest so this sparse file appears as /dev/xvdb
- In the domU create a single partition on /dev/xvdb and format
it with ext3.
- In the DomU, mount /dev/xvdb1 on /mnt and then run
dd if=/dev/zero of=/mnt/data.bin bs=1GB count=1
Without this patch, the 'dd' command would succeed in writing 1 GB of data
even though the underlying disk in Dom0 was only 200 MB in size. More complex
tests of copying a whole directory heirarchy across resulted in catastrophic
data corruption of the filessytem itself. Manual fsck was needed to fixup
the filesystem & there were many very bad errors needing fixing.
With this patch applied the DomU sees the I/O failures and kernel logs
messages
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 722127
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 730327
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 738527
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 746727
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 754927
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 763127
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 771327
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 779527
Dec 1 11:02:53 dhcp-5-203 kernel: end_request: I/O error, dev xvdc, sector 792399
It will retry the I/O operation until it runs out of sectors to try, and then
fail the operation. The filesystem is not seriously damaged - ext3 journal
recovery will trivially cleanup if the guest is rebooted after the disk in
Dom0 is enlarged.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
[1] http://lwn.net/Articles/24366/
While the kernel side of blktap drivers is using 64-bit sector numbers
correctly when reporting disk sizing, the userspace parts where this
dat is actually generated is truncating at 32-bit. There are 3 variables
related to disk sizing which are used in the kernel/userspace code
'size', 'sector size' & 'info'. In kernel space these are treated as being
unsigned long long, unsigned long, and unsigned int, respectively. In
the blktap userspace code this were all varyingly long, unsigned long.
This mismatch caused 32-bit wraparound on large disks. The attached
patch updates the blktap userspace to use the same data types as
kernel space for all these params, thus both fixing the 32-bit bug &
giving the code consistency.
This patch has been tested both by comparing /proc/partitions in the
guest, with the image size seen in the host, and by using Stephen
Tweedie's 'verify-data' tool on a variety of multi-TB sized disks.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
- remove magic number 8 and 64.
page size / sector size is 8 on x86, however on ia64 page size is
16kb.
blkring size is 64 on x86. however it's 128 on ia64.
- replace 0xFFFF with INVALID_GRANT_HANDLE
- don't map io ring into user space with uncachable.
kernel access the page with cacable and tapdisk uses memory barrier
so that it isn't necessary.
- remove printk warning and inserted new line.
- remove unused variable, page.
- add one BUG()
Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
tapdisk was using a 1msec select() timeout in its main event loop. On
timeout, the loop does nothing except repeat, so the timeout achieves
nothing; worse, on a machine with HZ set to less than 1000, a msec
timeout returns immediately, sending tapdisk into a 100%-CPU-
utilisation loop.
Simply remove the timeout entirely; everything still works fine, and
much more efficiently when idle.
Signed-off-by: Stephen Tweedie <sct@redhat.com>
blktap currently uses a hardcoded major of 254 for the device. This is
not robust in anyway and needs to be dynamic.
Note: it is better not to have the daemon create the node, and have udev
create it instead. But since the daemon currently creates the node
anyway, it is still the way this is done. That change needs to be made
at another time.
Signed-off-by: Steven Rostedt <srostedt@redhat.com>
