summaryrefslogtreecommitdiffstats
path: root/block.c
Commit message (Collapse)AuthorAgeFilesLines
* block: avoid buffer overrun by using pstrcpy, not strncpyJim Meyering2012-10-051-2/+3
| | | | | | | | | | Also, use PATH_MAX, rather than the arbitrary 1024. Using PATH_MAX is more consistent with other filename-related variables in this file, like backing_filename and tmp_filename. Acked-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Jim Meyering <meyering@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
* block: introduce block job errorPaolo Bonzini2012-09-281-4/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The following behaviors are possible: 'report': The behavior is the same as in 1.1. An I/O error, respectively during a read or a write, will complete the job immediately with an error code. 'ignore': An I/O error, respectively during a read or a write, will be ignored. For streaming, the job will complete with an error and the backing file will be left in place. For mirroring, the sector will be marked again as dirty and re-examined later. 'stop': The job will be paused and the job iostatus will be set to failed or nospace, while the VM will keep running. This can only be specified if the block device has rerror=stop and werror=stop or enospc. 'enospc': Behaves as 'stop' for ENOSPC errors, 'report' for others. In all cases, even for 'report', the I/O error is reported as a QMP event BLOCK_JOB_ERROR, with the same arguments as BLOCK_IO_ERROR. It is possible that while stopping the VM a BLOCK_IO_ERROR event will be reported and will clobber the event from BLOCK_JOB_ERROR, or vice versa. This is not really avoidable since stopping the VM completes all pending I/O requests. In fact, it is already possible now that a series of BLOCK_IO_ERROR events are reported with rerror=stop, because vm_stop calls bdrv_drain_all and this can generate further errors. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* iostatus: reorganize io error codePaolo Bonzini2012-09-281-8/+38
| | | | | | | | | | | | Move the common part of IDE/SCSI/virtio error handling to the block layer. The new function bdrv_error_action subsumes all three of bdrv_emit_qmp_error_event, vm_stop, bdrv_iostatus_set_err. The same scheme will be used for errors in block jobs. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* iostatus: change is_read to a boolPaolo Bonzini2012-09-281-2/+2
| | | | | | | | | Do this while we are touching this part of the code, before introducing more uses of "int is_read". Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* iostatus: move BlockdevOnError declaration to QAPIPaolo Bonzini2012-09-281-3/+3
| | | | | | | | | This will let block-stream reuse the enum. Places that used the enums are renamed accordingly. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* iostatus: rename BlockErrorAction, BlockQMPEventActionPaolo Bonzini2012-09-281-4/+4
| | | | | | | | | | | | We want to remove knowledge of BLOCK_ERR_STOP_ENOSPC from drivers; drivers should only be told whether to stop/report/ignore the error. On the other hand, we want to keep using the nicer BlockErrorAction name in the drivers. So rename the enums, while leaving aside the names of the enum values for now. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: move job APIs to separate filesPaolo Bonzini2012-09-281-127/+1Star
| | | | | | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: helper function, to find the base image of a chainJeff Cody2012-09-281-0/+16
| | | | | | | | | This is a simple helper function, that will return the base image of a given image chain. Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: add support functions for live commit, to find and delete images.Jeff Cody2012-09-281-0/+143
| | | | | | | | | | | | | | | | | | | | | | | | Add bdrv_find_overlay(), and bdrv_drop_intermediate(). bdrv_find_overlay(): given 'bs' and the active (topmost) BDS of an image chain, find the image that is the immediate top of 'bs' bdrv_drop_intermediate(): Given 3 BDS (active, top, base), drop images above base up to and including top, and set base to be the backing file of top's overlay node. E.g., this converts: bottom <- base <- intermediate <- top <- active to bottom <- base <- active Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: remove keep_read_only flag from BlockDriverState structJeff Cody2012-09-241-2/+0Star
| | | | | | | | The keep_read_only flag is no longer used, in favor of the bdrv flag BDRV_O_ALLOW_RDWR. Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: convert bdrv_commit() to use bdrv_reopen()Jeff Cody2012-09-241-43/+5Star
| | | | | | | | Currently, bdrv_commit() reopens images r/w itself, via risky _delete() and _open() calls. Use the new safe method for drive reopen. Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Framework for reopening files safelyJeff Cody2012-09-241-0/+232
| | | | | | | | | | | | | | | | | | | | | | | | | This is based on Supriya Kannery's bdrv_reopen() patch series. This provides a transactional method to reopen multiple images files safely. Image files are queue for reopen via bdrv_reopen_queue(), and the reopen occurs when bdrv_reopen_multiple() is called. Changes are staged in bdrv_reopen_prepare() and in the equivalent driver level functions. If any of the staged images fails a prepare, then all of the images left untouched, and the staged changes for each image abandoned. Block drivers are passed a reopen state structure, that contains: * BDS to reopen * flags for the reopen * opaque pointer for any driver-specific data that needs to be persistent from _prepare to _commit/_abort * reopen queue pointer, if the driver needs to queue additional BDS for a reopen Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: make bdrv_set_enable_write_cache() modify open_flagsJeff Cody2012-09-241-0/+7
| | | | | | | | | | | | | bdrv_set_enable_write_cache() sets the bs->enable_write_cache flag, but without the flag recorded in bs->open_flags, then next time a reopen() is performed the enable_write_cache setting may be inadvertently lost. This will set the flag in open_flags, so it is preserved across reopens. Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: correctly set the keep_read_only flagJeff Cody2012-09-241-7/+7
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | I believe the bs->keep_read_only flag is supposed to reflect the initial open state of the device. If the device is initially opened R/O, then commit operations, or reopen operations changing to R/W, are prohibited. Currently, the keep_read_only flag is only accurate for the active layer, and its backing file. Subsequent images end up always having the keep_read_only flag set. For instance, what happens now: [ base ] kro = 1, ro = 1 | v [ snap-1 ] kro = 1, ro = 1 | v [ snap-2 ] kro = 0, ro = 1 | v [ active ] kro = 0, ro = 0 What we want: [ base ] kro = 0, ro = 1 | v [ snap-1 ] kro = 0, ro = 1 | v [ snap-2 ] kro = 0, ro = 1 | v [ active ] kro = 0, ro = 0 Signed-off-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Don't forget to delete temporary fileDunrong Huang2012-09-121-1/+5
| | | | | | | The caller would not delete temporary file after failed get_tmp_filename(). Signed-off-by: Dunrong Huang <riegamaths@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: fix block tray statusPavel Hrdina2012-09-121-2/+2
| | | | | | | The tray status should change also if you eject empty block device. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: Flush parent to OS with cache=unsafeKevin Wolf2012-08-151-1/+2
| | | | | | | | | | | | Commit 29cdb251 already added a comment that no unnecessary flushes to disk will occur, this patch makes the code even get to the point of the comment. This is mostly theoretical because in practice we only stack one format on top of one protocol, the former implementing flush_to_os and the latter only flush_to_disk. It starts to matter when drivers that are not on top implement flush_to_os. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
* qmp: query-block: add 'encryption_key_missing' fieldLuiz Capitulino2012-08-131-0/+1
| | | | | Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
* block: Use bdrv_get_backing_file_depth()Benoît Canet2012-08-031-0/+3
| | | | | | | | | | Use the dedicated counting function in qmp_query_block in order to propagate the backing file depth to HMP and add backing_file_depth to qmp-commands.hx Signed-off-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
* block: create bdrv_get_backing_file_depth()Benoît Canet2012-08-031-0/+13
| | | | | | | | | | Create bdrv_get_backing_file_depth() in order to be able to show in QMP and HMP how many ancestors backing an image a block device have. Signed-off-by: Benoit Canet <benoit@irqsave.net> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
* Avoid returning voidBlue Swirl2012-07-281-1/+1
| | | | | | | It's silly and non-conforming to standards to return void, don't do it. Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
* block: Geometry and translation hints are now useless, purge themMarkus Armbruster2012-07-171-32/+0Star
| | | | | | | | | | | | | | | | | | | | | | | | | | There are two producers of these hints: drive_init() on behalf of -drive, and hd_geometry_guess(). The only consumer of the hint is hd_geometry_guess(). The callers of hd_geometry_guess() call it only when drive_init() didn't set the hints. Therefore, drive_init()'s hints are never used. Thus, hd_geometry_guess() only ever sees hints it produced itself in a prior call. Only the first call computes something, subsequent calls just repeat the first call's results. However, hd_geometry_guess() is never called more than once: the device models don't, and the block device is destroyed on unplug. Thus, dropping the repeat feature doesn't break anything now. If a block device wasn't destroyed on unplug and could be reused with a new device, then repeating old results would be wrong. Thus, dropping the repeat feature prevents future breakage. This renders the hints unused. Purge them from the block layer. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* hd-geometry: Move disk geometry guessing back from block.cMarkus Armbruster2012-07-171-121/+0Star
| | | | | | | | | | | Commit f3d54fc4 factored it out of hw/ide.c for reuse. Sensible, except it was put into block.c. Device-specific functionality should be kept in device code, not the block layer. Move it to hw/hd-geometry.c, and make stylistic changes required to keep checkpatch.pl happy. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* fdc: Move floppy geometry guessing back from block.cMarkus Armbruster2012-07-171-101/+0Star
| | | | | | | | | | | | | | | | | Commit 5bbdbb46 moved it to block.c because "other geometry guessing functions already reside in block.c". Device-specific functionality should be kept in device code, not the block layer. Move it back. Disk geometry guessing is still in block.c. To be moved out in a later patch series. Bonus: the floppy type used in pc_cmos_init() now obviously matches the one in the FDrive. Before, we relied on bdrv_get_floppy_geometry_hint() picking the same type both in fd_revalidate() and in pc_cmos_init(). Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* Merge remote-tracking branch 'mjt/mjt-iov2' into stagingAnthony Liguori2012-07-091-6/+6
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * mjt/mjt-iov2: rewrite iov_send_recv() and move it to iov.c cleanup qemu_co_sendv(), qemu_co_recvv() and friends export iov_send_recv() and use it in iov_send() and iov_recv() rename qemu_sendv to iov_send, change proto and move declarations to iov.h change qemu_iovec_to_buf() to match other to,from_buf functions consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent allow qemu_iovec_from_buffer() to specify offset from which to start copying consolidate qemu_iovec_memset{,_skip}() into single function and use existing iov_memset() rewrite iov_* functions change iov_* function prototypes to be more appropriate virtio-serial-bus: use correct lengths in control_out() message Conflicts: tests/Makefile Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
| * change qemu_iovec_to_buf() to match other to,from_buf functionsMichael Tokarev2012-06-111-1/+1
| | | | | | | | | | | | | | | | It now allows specifying offset within qiov to start from and amount of bytes to copy. Actual implementation is just a call to iov_to_buf(). Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
| * consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistentMichael Tokarev2012-06-111-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | qemu_iovec_concat() is currently a wrapper for qemu_iovec_copy(), use the former (with extra "0" arg) in a few places where it is used. Change skip argument of qemu_iovec_copy() from uint64_t to size_t, since size of qiov itself is size_t, so there's no way to skip larger sizes. Rename it to soffset, to make it clear that the offset is applied to src. Also change the only usage of uint64_t in hw/9pfs/virtio-9p.c, in v9fs_init_qiov_from_pdu() - all callers of it actually uses size_t too, not uint64_t. One added restriction: as for all other iovec-related functions, soffset must point inside src. Order of argumens is already good: qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes) vs: qemu_iovec_concat(QEMUIOVector *dst, QEMUIOVector *src, size_t soffset, size_t sbytes) (note soffset is after _src_ not dst, since it applies to src; for memset it applies to qiov). Note that in many places where this function is used, the previous call is qemu_iovec_reset(), which means many callers actually want copy (replacing dst content), not concat. So we may want to add a wrapper like qemu_iovec_copy() with the same arguments but which calls qemu_iovec_reset() before _concat(). Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
| * allow qemu_iovec_from_buffer() to specify offset from which to start copyingMichael Tokarev2012-06-111-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Similar to qemu_iovec_memset(QEMUIOVector *qiov, size_t offset, int c, size_t bytes); the new prototype is: qemu_iovec_from_buf(QEMUIOVector *qiov, size_t offset, const void *buf, size_t bytes); The processing starts at offset bytes within qiov. This way, we may copy a bounce buffer directly to a middle of qiov. This is exactly the same function as iov_from_buf() from iov.c, so use the existing implementation and rename it to qemu_iovec_from_buf() to be shorter and to match the utility function. As with utility implementation, we now assert that the offset is inside actual iovec. Nothing changed for current callers, because `offset' parameter is new. While at it, stop using "bounce-qiov" in block/qcow2.c and copy decrypted data directly from cluster_data instead of recreating a temp qiov for doing that. Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
* | block: Factor bdrv_read_unthrottled() out of guess_disk_lchs()Markus Armbruster2012-07-091-7/+17
| | | | | | | | | | | | | | | | To prepare move of guess_disk_lchs() into hw/, where it poking BlockDriverState member io_limits_enabled directly would be unclean. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | fdc: Drop broken code for user-defined floppy geometryMarkus Armbruster2012-07-091-34/+28Star
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | bdrv_get_floppy_geometry_hint() fails to store through its parameter drive when bs has a geometry hint. Makes fd_revalidate() assign random crap to drv->drive. Has been broken that way for ages. Harmless, because: * The only way to set a geometry hint is -drive if=none,cyls=... Since commit c219331e, probably unintentional. * The only use of drv->drive is as argument to another bdrv_get_floppy_geometry_hint(). Which doesn't use it, since the geometry hint is still there. Drop the broken code, ignore -drive parameter cyls, heads and secs for floppies even with if=none, just like before commit c219331e. Matches -help, which explains cyls, heads, secs as "hard disk physical geometry". Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: introduce bdrv_swap, implement bdrv_append on top of itPaolo Bonzini2012-07-091-85/+99
| | | | | | | | | | | | | | | | | | | | The new function can be made a bit nicer than bdrv_append. It swaps the whole contents, and then swaps back (using the usual t=a;a=b;b=t idiom) the fields that need to stay on top. Thus, it does not need explicit bdrv_detach_dev, bdrv_iostatus_disable, etc. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: copy over job and dirty bitmap fields in bdrv_appendPaolo Bonzini2012-07-091-0/+15
| | | | | | | | | | | | | | | | | | While these should not be in use at the time a transaction is started, a command in the prepare phase of a transaction might have added them, so they need to be brought over. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: Replace bdrv_get_format() by bdrv_get_format_name()Markus Armbruster2012-06-151-7/+4Star
| | | | | | | | | | | | | | | | | | | | | | So callers don't need to know anything about maximum name length. Returning a pointer is safe, because the name string lives as long as the block driver it names, and block drivers don't die. Requested by Peter Maydell. Signed-off-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: always open drivers in writeback modePaolo Bonzini2012-06-151-1/+2
| | | | | | | | | | | | | | | | | | Formats are entirely in charge of flushes for metadata writes. For guest-initiated writes, a writethrough cache is faked in the block layer. So we can always open in writeback mode. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: add bdrv_set_enable_write_cachePaolo Bonzini2012-06-151-0/+5
| | | | | | | | | | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: copy enable_write_cache in bdrv_appendPaolo Bonzini2012-06-151-0/+2
| | | | | | | | | | | | | | | | Because the guest will be able to flip enable_write_cache, the actual state may not match what is used to open the new snapshot. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: flush in writethrough mode after writesPaolo Bonzini2012-06-151-2/+9
| | | | | | | | | | | | | | | | | | | | We want to make the formats handle their own flushes autonomously, while keeping for guests the ability to use a writethrough cache. Since formats will write metadata via bs->file, bdrv_co_do_writev is the only place where we need to add a flush. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | block: New bdrv_get_flags()Markus Armbruster2012-06-151-0/+5
| | | | | | | | | | | | Signed-off-by: Markus Armbruster <armbru@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | qemu-img check -r for repairing imagesKevin Wolf2012-06-151-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | The QED block driver already provides the functionality to not only detect inconsistencies in images, but also fix them. However, this functionality cannot be manually invoked with qemu-img, but the check happens only automatically during bdrv_open(). This adds a -r switch to qemu-img check that allows manual invocation of an image repair. Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* | stream: move is_allocated_above to block.cPaolo Bonzini2012-06-151-0/+49
|/ | | | | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: prevent snapshot mode $TMPDIR symlink attackJim Meyering2012-05-301-13/+24
| | | | | | | | | | | | | | | | | | | | | | | | | | In snapshot mode, bdrv_open creates an empty temporary file without checking for mkstemp or close failure, and ignoring the possibility of a buffer overrun given a surprisingly long $TMPDIR. Change the get_tmp_filename function to return int (not void), so that it can inform its two callers of those failures. Also avoid the risk of buffer overrun and do not ignore mkstemp or close failure. Update both callers (in block.c and vvfat.c) to propagate temp-file-creation failure to their callers. get_tmp_filename creates and closes an empty file, while its callers later open that presumed-existing file with O_CREAT. The problem was that a malicious user could provoke mkstemp failure and race to create a symlink with the selected temporary file name, thus causing the qemu process (usually root owned) to open through the symlink, overwriting an attacker-chosen file. This addresses CVE-2012-2652. http://bugzilla.redhat.com/CVE-2012-2652 Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com> Signed-off-by: Jim Meyering <meyering@redhat.com> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
* qemu-img: make "info" backing file output correct and easier to usePaolo Bonzini2012-05-101-8/+11
| | | | | | | | | | qemu-img info should use the same logic as qemu when printing the backing file path, or debugging becomes quite tricky. We can also simplify the output in case the backing file has an absolute path or a protocol. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: move field reset from bdrv_open_common to bdrv_closePaolo Bonzini2012-05-101-7/+6Star
| | | | | | | | | | bdrv_close should leave fields in the same state as bdrv_new. It is not up to bdrv_open_common to fix the mess. Also, backing_format was not being re-initialized. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: protect path_has_protocol from filenames with colonsPaolo Bonzini2012-05-101-1/+6
| | | | | | | | | | | | path_has_protocol will erroneously return "true" if the colon is part of a filename. These names are common with stable device names produced by udev. We cannot fully protect against this in case the filename does not have a path component (e.g. if the current directory is /dev/disk/by-path), but in the common case there will be a slash before and path_has_protocol can easily detect that and return false. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: simplify path_is_absolutePaolo Bonzini2012-05-101-11/+4Star
| | | | | | | | | | | | | | | | | | | On Windows, all the logic is already in is_windows_drive and is_windows_drive_prefix. On POSIX, there is no need to look out for colons. The win32 code changes the behaviour in some cases, we could have something like "d:foo.img". The old code would treat it as relative path, the new one as absolute. Now the path is absolute, because to go from c:/program files/blah to d:foo.img you cannot say c:/program files/blah/d:foo.img. You have to say d:foo.img. But you could also say it's relative because (I think, at least it was like that in DOS 15 years ago) d:foo.img is relative to the current path of drive D. Considering how path_is_absolute is used by path_combine, I think it's better to treat it as absolute. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: wait for job callback in block_job_cancel_syncPaolo Bonzini2012-05-101-2/+34
| | | | | | | | | | | | | | | | | | | | | | | | | | | | The limitation on not having I/O after cancellation cannot really be kept. Even streaming has a very small race window where you could cancel a job and have it report completion. If this window is hit, bdrv_change_backing_file() will yield and possibly cause accesses to dangling pointers etc. So, let's just assume that we cannot know exactly what will happen after the coroutine has set busy to false. We can set a very lax condition: - if we cancel the job, the coroutine won't set it to false again (and hence will not call co_sleep_ns again). - block_job_cancel_sync will wait for the coroutine to exit, which pretty much ensures no race. Instead, we track the coroutine that executes the job and put very strict conditions on what to do while it is quiescent (busy = false). First of all, the coroutine must never set busy = false while the job has been cancelled. Second, the coroutine can be reentered arbitrarily while it is quiescent, so you cannot really do anything but co_sleep_ns at that time. This condition is obeyed by the block_job_sleep_ns function. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: add block_job_sleep_nsPaolo Bonzini2012-05-101-0/+11
| | | | | | | | This function abstracts the pretty complex semantics of the "busy" member of BlockJob. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: fully delete bs->file when closingPaolo Bonzini2012-05-101-4/+2Star
| | | | | | | | | We are reusing bs->file across close/open, which may not cause any known bugs but is a recipe for trouble. Prefer bdrv_delete, and enjoy the new invariant in the implementation of bdrv_delete. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: do not reuse the backing file across bdrv_close/bdrv_openPaolo Bonzini2012-05-101-0/+2
| | | | | | | | | This is another bug caused by not doing a full cleanup of the BDS across close/open. This was found with mirroring by Shaolong Hu, but it can probably be reproduced also with eject or change. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* block: another bdrv_append fixPaolo Bonzini2012-05-101-0/+1
| | | | | | | | | | bdrv_append must also copy open_flags to the top, because the snapshot has BDRV_O_NO_BACKING set. This causes interesting results if you later use drive-reopen (not upstream) to reopen the image, and lose the backing file in the process. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>