This reverts commit 79368c81.

Conflicts:

	block.c

I haven't been able to come up with a solution yet for the corruption caused by
unaligned requests from the IDE disk so revert until a solution can be written.
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

Arguably we should re-open the backing file with the backing file format and
not with the format of the snapshot image.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

bdrv_eject() gets called when a device model opens or closes the tray.

If the block driver implements method bdrv_eject(), that method gets
called.  Drivers host_cdrom implements it, and it opens and closes the
physical tray, and nothing else.  When a device model opens, then
closes the tray, media changes only if the user actively changes the
physical media while the tray is open.  This is matches how physical
hardware behaves.

If the block driver doesn't implement method bdrv_eject(), we do
something quite different: opening the tray severs the connection to
the image by calling bdrv_close(), and closing the tray does nothing.
When the device model opens, then closes the tray, media is gone,
unless the user actively inserts another one while the tray is open,
with a suitable change command in the monitor.  This isn't how
physical hardware behaves.  Rather inconvenient when programs
"helpfully" eject media to give you a chance to change it.  The way
bdrv_eject() behaves here turns that chance into a must, which is not
what these programs or their users expect.

Change the default action not to call bdrv_close().  Instead, note the
tray status in new BlockDriverState member tray_open.  Use it in
bdrv_is_inserted().

Arguably, the device models should keep track of tray status
themselves.  But this is less invasive.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Assuming that any image on a block device is not properly zero-initialized is
actually wrong: Only raw images have this problem. Any other image format
shouldn't care about it, they initialize everything properly themselves.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

bdrv_commit copies the image to its backing file sector by sector, which
is (surprise!) relatively slow. Let's take a larger buffer and handle more
sectors at once if possible.

With a 1G qcow2 file, this brought the time bdrv_commit takes down from
5:06 min to 1:14 min for me.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Block device change command did not copy BDRV_O_SNAPSHOT flag. Thus
the new image did not have this flag and the file got deleted during
opening.

Fix by copying BDRV_O_SNAPSHOT flag.
Signed-off-by: NBlue Swirl <blauwirbel@gmail.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

"No such file or directory" is a misleading error message
when a user tries to open a file with wrong permissions.

Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: NStefan Weil <weil@mail.berlios.de>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

CVE-2008-2004 described a vulnerability in QEMU whereas a malicious user could
trick the block probing code into accessing arbitrary files in a guest.  To
mitigate this, we added an explicit format parameter to -drive which disabling
block probing.

Fast forward to today, and the vast majority of users do not use this parameter.
libvirt does not use this by default nor does virt-manager.

Most users want block probing so we should try to make it safer.

This patch adds some logic to the raw device which attempts to detect a write
operation to the beginning of a raw device.  If the first 4 bytes happen to
match an image file that has a backing file that we support, it scrubs the
signature to all zeros.  If a user specifies an explicit format parameter, this
behavior is disabled.

I contend that while a legitimate guest could write such a signature to the
header, we would behave incorrectly anyway upon the next invocation of QEMU.
This simply changes the incorrect behavior to not involve a security
vulnerability.

I've tested this pretty extensively both in the positive and negative case.  I'm
not 100% confident in the block layer's ability to deal with zero sized writes
particularly with respect to the aio functions so some additional eyes would be
appreciated.

Even in the case of a single sector write, we have to make sure to invoked the
completion from a bottom half so just removing the zero sized write is not an
option.
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

This distinguishes between harmless leaks and real corruption. Hopefully users
better understand what qemu-img check wants to tell them.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

People think that their images are corrupted when in fact there are just some
leaked clusters. Differentiating several error cases should make the messages
more comprehensible.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Don't try to be clever by freeing all temporary data and calling all callbacks
when the return value (an error) is certain. Doing so has at least two
important problems:

* The temporary data that is freed (qiov, possibly zero buffer) is still used
  by the requests that have not yet completed.
* Calling the callbacks for all requests in the multiwrite means for the caller
  that it may free buffers etc. which are still in use.

Just remember the error value and do the cleanup when all requests have
completed.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

bdrv_aio_writev may call the callback immediately (and it will commonly do so
in error cases). Current code doesn't consider this. For details see the
comment added by this patch.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

BlockDriverState member removable controls whether virtual media
change (monitor commands change, eject) is allowed.  It is set when
the "type hint" is BDRV_TYPE_CDROM or BDRV_TYPE_FLOPPY.

The type hint is only set by drive_init().  It sets BDRV_TYPE_FLOPPY
for if=floppy.  It sets BDRV_TYPE_CDROM for media=cdrom and if=ide,
scsi, xen, or none.

if=ide and if=scsi work, because the type hint makes it a CD-ROM.
if=xen likewise, I think.

For the same reason, if=none works when it's used by ide-drive or
scsi-disk.  For other guest devices, there are problems:

* fdc: you can't change virtual media

    $ qemu [...] -drive if=none,id=foo,... -global isa-fdc.driveA=foo
    QEMU 0.12.50 monitor - type 'help' for more information
    (qemu) eject foo
    Device 'foo' is not removable

  unless you add media=cdrom, but that makes it readonly.

* virtio: if you add media=cdrom, you can change virtual media.  If
  you eject, the guest gets I/O errors.  If you change, the guest sees
  the drive's contents suddenly change.

* scsi-generic: if you add media=cdrom, you can change virtual media.
  I didn't test what that does to the guest or the physical device,
  but it can't be pretty.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

savevm.c keeps a pointer to the snapshot block device.  If you manage
to get that device deleted, the pointer dangles, and the next snapshot
operation will crash & burn.  Unplugging a guest device that uses it
does the trick:

    $ MALLOC_PERTURB_=234 qemu-system-x86_64 [...]
    QEMU 0.12.50 monitor - type 'help' for more information
    (qemu) info snapshots
    No available block device supports snapshots
    (qemu) drive_add auto if=none,file=tmp.qcow2
    OK
    (qemu) device_add usb-storage,id=foo,drive=none1
    (qemu) info snapshots
    Snapshot devices: none1
    Snapshot list (from none1):
    ID        TAG                 VM SIZE                DATE       VM CLOCK
    (qemu) device_del foo
    (qemu) info snapshots
    Snapshot devices:
    Segmentation fault (core dumped)

Move management of that pointer to block.c, and zap it when the device
it points becomes unusable.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

For instance, -device scsi-disk,drive=foo -device scsi-disk,drive=foo
happily creates two SCSI disks connected to the same block device.
It's all downhill from there.

Device usb-storage deliberately attaches twice to the same blockdev,
which fails with the fix in place.  Detach before the second attach
there.

Also catch attempt to delete while a guest device model is attached.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

To fix https://bugs.launchpad.net/qemu/+bug/597402 where qemu fails to
call unlink() on temporary snapshots due to bs->is_temporary getting clobbered
in bdrv_open_common() after being set in bdrv_open() which calls the former.

We don't need to initialize bs->is_temporary in bdrv_open_common().
Signed-off-by: NRyan Harper <ryanh@us.ibm.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Before the raw/file split we used to allow filenames with colons for host
device only.  While this was more by accident than by design people rely
on it, so we need to bring it back.

So move the host device probing to be before the protocol detection
again.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Add new functions that write and flush the written data to disk immediately.
This is what needs to be used for image format metadata to maintain integrity
for cache=... modes that don't use O_DSYNC. (Actually, we only need barriers,
and therefore the functions are defined as such, but flushes is what is
implemented in this patch - we can try to change that later)
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Fix a warning from OpenBSD gcc (3.3.5 (propolice)):
/src/qemu/block.c: In function `bdrv_info_stats_bs':
/src/qemu/block.c:1548: warning: long long int format, long unsigned
int arg (arg 6)

There may be also truncation effects.
Signed-off-by: NBlue Swirl <blauwirbel@gmail.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This is a more flexible alternative to bdrv_iterate().
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

do_commit() and mux_proc_byte() iterate over the list of drives
defined with drive_init().  This misses host block devices defined by
other means.  Such means don't exist now, but will be introduced later
in this series.

Change them to use new bdrv_commit_all(), which iterates over all host
block devices.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

That's where they belong semantically (block device host part), even
though the actions are actually executed by guest device code.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Both bdrv_can_snapshot() and bdrv_has_snapshot() does not work as advertized.

First issue: Their names implies different porpouses, but they do the same thing
and have exactly the same code. Maybe copied and pasted and forgotten?
bdrv_has_snapshot() is called in various places for actually checking if there
is snapshots or not.

Second issue: the way bdrv_can_snapshot() verifies if a block driver supports or
not snapshots does not catch all cases. E.g.: a raw image.

So when do_savevm() is called, first thing it does is to set a global
BlockDriverState to save the VM memory state calling get_bs_snapshots().

static BlockDriverState *get_bs_snapshots(void)
{
    BlockDriverState *bs;
    DriveInfo *dinfo;

    if (bs_snapshots)
        return bs_snapshots;
    QTAILQ_FOREACH(dinfo, &drives, next) {
        bs = dinfo->bdrv;
        if (bdrv_can_snapshot(bs))
            goto ok;
    }
    return NULL;
 ok:
    bs_snapshots = bs;
    return bs;
}

bdrv_can_snapshot() may return a BlockDriverState that does not support
snapshots and do_savevm() goes on.

Later on in do_savevm(), we find:

    QTAILQ_FOREACH(dinfo, &drives, next) {
        bs1 = dinfo->bdrv;
        if (bdrv_has_snapshot(bs1)) {
            /* Write VM state size only to the image that contains the state */
            sn->vm_state_size = (bs == bs1 ? vm_state_size : 0);
            ret = bdrv_snapshot_create(bs1, sn);
            if (ret < 0) {
                monitor_printf(mon, "Error while creating snapshot on '%s'\n",
                               bdrv_get_device_name(bs1));
            }
        }
    }

bdrv_has_snapshot(bs1) is not checking if the device does support or has
snapshots as explained above. Only in bdrv_snapshot_create() the device is
actually checked for snapshot support.

So, in cases where the first device supports snapshots, and the second does not,
the snapshot on the first will happen anyways. I believe this is not a good
behavior. It should be an all or nothing process.

This patch addresses these issues by making bdrv_can_snapshot() actually do
what it must do and enforces better tests to avoid errors in the middle of
do_savevm(). bdrv_has_snapshot() is removed and replaced by bdrv_can_snapshot()
where appropriate.

bdrv_can_snapshot() was moved from savevm.c to block.c. It makes more sense to me.

The loadvm_state() function was updated too to enforce that when loading a VM at
least all writable devices must support snapshots too.
Signed-off-by: NMiguel Di Ciurcio Filho <miguel.filho@gmail.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

When snapshot handlers are not defined in the format driver, it is
better to call the ones of the protocol driver.  This enables us to
implement snapshot support in the protocol driver.

We need to call bdrv_close() and bdrv_open() handlers of the format
driver before and after bdrv_snapshot_goto() call of the protocol.  It is
because the contents of the block driver state may need to be changed
after loading vmstate.
Signed-off-by: NMORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This patch calls the close handler of the block driver before the qemu
process exits.

This is necessary because the sheepdog block driver releases the lock
of VM images in the close handler.
Signed-off-by: NMORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

qemu -cdrom /dev/cdrom with an empty CD-ROM drive doesn't work any more because
we try to guess the format and when this fails (because there is no medium) we
exit with an error message.

This patch should restore the old behaviour by assuming raw format for such
drives.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Clean up block.c and use BDRV_SECTOR_SIZE rather than hard coded
numbers (512) when referring to sector size throughout the code.
Signed-off-by: NJes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

In bdrv_open() there is no need to shift total_size >> 9 just to
multiply it by 512 again just a few lines later, since this is the
only place the variable is used.

Mask with BDRV_SECTOR_MASK to protect against case where we are
passed a corrupted image.
Signed-off-by: NJes Sorensen <Jes.Sorensen@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Previous commit added QMP documentation to the qemu-monitor.hx
file, it's is a copy of this information.

While it's good to keep it near code, maintaining two copies of
the same information is too hard and has little benefit as we
don't expect client writers to consult the code to find how to
use a QMP command.
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

This patch adds a missing bdrv_delete() call in find_image_format() so that a
SG_IO BlockDriver properly releases the temporary BlockDriverState *bs created
from bdrv_file_open()
Signed-off-by: NNicholas A. Bellinger <nab@linux-iscsi.org>
Reported-by: NChris Krumme <chris.krumme@windriver.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This patch enables protocol drivers to use their create options which
are not supported by the format.  For example, protcol drivers can use
a backing_file option with raw format.
Signed-off-by: NMORITA Kazutaka <morita.kazutaka@lab.ntt.co.jp>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

With overlapping requests, the total number of sectors is smaller than the sum
of the nb_sectors of both requests.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Usually the guest can tell the host to flush data to disk. In some cases we
don't want to flush though, but try to keep everything in cache.

So let's add a new cache value to -drive that allows us to set the cache
policy to most aggressive, disabling flushes. We call this mode "unsafe",
as guest data is not guaranteed to survive host crashes anymore.

This patch also adds a noop function for aio, so we can do nothing in AIO
fashion.
Signed-off-by: NAlexander Graf <agraf@suse.de>
Signed-off-by: NAurelien Jarno <aurelien@aurel32.net>

This patch adds a special case check for scsi-generic devices in
refresh_total_sectors() to skip the subsequent BlockDriver->bdrv_getlength()
that will be returning -ESPIPE from block/raw-posic.c:raw_getlength() for
BlockDriverState->sg=1 devices.
Signed-off-by: NNicholas A. Bellinger <nab@linux-iscsi.org>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

This patch adds a special BlockDriverState->sg check in block.c:find_image_format()
after bdrv_file_open() -> block/raw-posix.c:hdev_open() has been called to determine
if we are dealing with a Linux host scsi-generic device.

The patch then returns the BlockDriver * from bdrv_find_format("raw"), skipping the
subsequent bdrv_read() and rest of find_image_format().
Signed-off-by: NNicholas A. Bellinger <nab@linux-iscsi.org>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The difference between the start sectors of two requests can be larger
than the size of the "int" type, which can lead to a not correctly
sorted multiwrite array and thus spurious I/O errors and filesystem
corruption due to incorrect request merges.

So instead of doing the cute sector arithmetics trick spell out the
exact comparisms.

Spotted by Kevin Wolf based on a testcase from Michael Tokarev.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The special case doesn't really us buy anything. Without it vvfat works more
consistently as a protocol. We get raw on top of vvfat now, which works just
as well as using vvfat directly.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The 'parent' field in the 'query-blockstats' monitor command is
part of the top level block device QDict, not part of the 2nd
level 'stats' QDict.

* block.c: Fix docs for 'parent' field in block stats monitor
  command output
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

There is a call to free() where qemu_free() should instead be used.
Signed-off-by: NBruce Rogers <brogers@novell.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>