bs->total_sectors is not yet updated at this point. resulting
in memory corruption if the volume has grown and data is written
to the newly availble areas.

CC: qemu-stable@nongnu.org
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the iscsi block driver.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Acked-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NBenoit Canet <benoit@irqsave.net>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

* accomodate -> accommodate
* aquiring -> acquiring
* beacuse -> because
* loosing -> losing
* prefering -> preferring
* threshhold -> threshold
Signed-off-by: NStefan Weil <sw@weilnetz.de>
Signed-off-by: NMichael Tokarev <mjt@tls.msk.ru>

Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

during rebasing the changed init value for the
retry counter was missed. This resulted in no retries
being performed at all.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

This patch lifts the minimum supported libiscsi version from 1.4.0 to
1.9.0 since the BUSY patch required that change.

On one this allows us to remove all #ifdefs from the code which
makes the code easier to maintain and read. On the other hand
I would not recommend libiscsi prior to 1.8.0 for production use
because the following important libiscsi fixes for deadlocks and
protocol errors are missing prior to 1.8.0:

dbe9a1e SOCKET queue cmd PDUs directly in waitpdu queue
30df192 DATA-OUT set pdu->cmdsn appropriately
548bd22 ISCSI fix broken send logic in iscsi_scsi_async_command
14bee10 RECONNECT do not increase CmdSN for immediate PDUs
1f4a66a PDU queue out PDUs in order of itt.
562dd46 PDU avoid incrementing itt to 0xffffffff
cd09c0f PDU use serial32 arithmetic for cmdsn, maxcmdsn and expcmdsn.
89e918e SOCKET validate data_size in in_pdu header
91267f5 Limit immediate and unsolicited data to FirstBurstLength

Note that libiscsi 1.9.0 was released on Feb 24th, 2013, about
one month after 1.8.0.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

this patch changes the driver to uses 16 Byte CDBs for
READ/WRITE only if the target requires 64bit lba addressing.

On one hand this saves 6 bytes in each PDU on the other
hand it seems that 10 Byte CDBs seems to be much better
supported and tested as a recent issue I had with a
major storage supplier lined out.

For WRITESAME the logic is a bit more tricky as WRITESAME10
with UNMAP was added really late. Thus a fallback to WRITESAME16
is possible if it supports UNMAP and WRITESAME10 not.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

it might happen in the future that a function directly invokes its callback.
In this case we end up in a segfault because the iTask is gone when the BH
is scheduled.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

this patch adds handling of BUSY status reponse from an iSCSI target.
Currently, we fail with -EIO in case of SCSI_STATUS_BUSY while the
obvious reaction would be to retry the operation after some time.
The retry time is randomly choosen from a range with exponential
growth increasing with each retry.

This patch includes most of the changes by a an upcoming patch
from Stefan Hajnoczi:

 iscsi: implement .bdrv_detach/attach_aio_context()

because I also need the reference to the aio_context for
the retry timer to work. I included the changes to maintain
better mergeability.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Now that all backend drivers are using QemuOpts, remove all
QEMUOptionParameter related codes.
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: NChunyan Liu <cyliu@suse.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NDong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: NChunyan Liu <cyliu@suse.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

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

Drop the assumption that we're using the main AioContext for Linux
AIO.  Convert qemu_aio_set_fd_handler() to aio_set_fd_handler() and
timer_new_ms() to aio_timer_new().

The .bdrv_detach/attach_aio_context() interfaces also need to be
implemented to move the fd and timer from the old to the new AioContext.

Cc: Peter Lieven <pl@kamp.de>
Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: NPeter Lieven <pl@kamp.de>

Commit b03c3805 introduced the function
iscsi_allocationmap_is_allocated(), however it is only used within a
code block that is conditionally compiled.  This produces a warning
(error with -werror) of "defined but not used" for the the function, if
LIBISCSI_FEATURE_IOVECTOR is not defined.

This wraps iscsi_allocationmap_is_allocated() in the same conditional.
Signed-off-by: NJeff Cody <jcody@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

Signed-off-by: NPeter Lieven <pl@kamp.de>

depending on the target the opt_unmap_gran might be as low
as 4K. As we know use this also as a knob to activate the allocationmap
feature lower the barrier. The limit 4K (and not 512) is choosen
to avoid a potentially too big allocationmap.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

this patch implements a cache that tracks if a page on the
iscsi target is allocated or not. The cache is implemented in
a way that it allows for false positives
(e.g. pretending a page is allocated, but it isn't), but
no false negatives.

The cached allocation info is then used to speed up the
read process for unallocated sectors by issueing a GET_LBA_STATUS
request for all sectors that are not yet known to be allocated.
If the read request is confirmed to fall into an unallocated
range we directly return zeroes and do not transfer the
data over the wire.

Tests have shown that a relatively small amount of GET_LBA_STATUS
requests happens a vServer boot time to fill the allocation cache
(all those blocks are not queried again).

Not to transfer all the data of unallocated sectors saves a lot
of time, bandwidth and storage I/O load during block jobs or storage
migration and it saves a lot of bandwidth as well for any big sequential
read of the whole disk (e.g. block copy or speed tests) if a significant
number of blocks is unallocated.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

if the iscsi driver receives a write zeroes request with
the BDRV_REQ_MAY_UNMAP flag set it fails with -ENOTSUP
if the iscsi target does not support WRITE SAME with
UNMAP. However, the BDRV_REQ_MAY_UNMAP is only a hint
and writing zeroes with WRITE SAME will still be
better than falling back to writing zeroes with WRITE16.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Using error_is_set(errp) that way can sweep programming errors under
the carpet when we get called incorrectly with an error set.

Commit 24d3bd67 added a broken error path to iscsi_do_inquiry(): it
first calls error_setg(), then jumps to the preexisting error label,
where error_setg() gets called again, triggering an assertion failure.

Commit cbee81f6 fixed this by guarding the second error_setg() with an
error_is_set().

Replace this fix by a simpler and safer one: jump right behind the
second error_setg().
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NKevin Wolf <kwolf@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

This patch adds an errp parameter to bdrv_new() and updates all its
callers. The next patches will make use of this in order to check for
duplicate IDs. Most of the callers know that their ID is fine, so they
can simply assert that there is no error.

Behaviour doesn't change with this patch yet as bdrv_new() doesn't
actually assign errors to errp.
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

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

This eliminates the possible assertion failure in error_setg().
Signed-off-by: NFam Zheng <famz@redhat.com>
Reviewed-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Max WRITE SAME length is also used when the UNMAP bit is zero, so it
should be queried even if LBPWS=0.  Same for the optimal transfer
length.

However, the write_zeroes_alignment only matters for UNMAP=1 so we
still restrict it to LBPWS=1.
Reviewed-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Non-block SCSI devices do not support flushing, but we may still send
them requests via bdrv_flush_all.  Just ignore them.
Reviewed-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Some targets may return "invalid field" as the ASCQ from WRITE SAME
if they support the command only without the UNMAP field.  Recognize
that, and return ENOTSUP just like for "invalid operation code".
Reviewed-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

The current iscsi block driver code makes the rather arbitrary decision
that TYPE_MEDIUM_CHANGER and TYPE_TAPE devices have bs->sg = 1 and all
other device types are disks.

Instead of this, check for TYPE_DISK to expose the disk interface and
make everything else bs->sg = 1. In particular, this includes devices
with TYPE_STORAGE_ARRAY, which is what LUN 0 of an iscsi target is.
(See https://bugzilla.redhat.com/show_bug.cgi?id=1067784 for the exact
scenario.)
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NBenoit Canet <benoit@irqsave.net>
Acked-by: NPaolo Bonzini <pbonzini@redhat.com>

commit fa6252b0 introduced a segfault because it tries
to read iTask.task->sense after iTask.task has been
freed.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

this patch ensures that we only query for block provisioning and
block limits vpd pages if they are advertised. It also cleans
up the inquiry code and eliminates some redundant code.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

the retry logic was broken because the complete status
of the task structure was not reset. this resulted in
an infinite loop retrying the command over and over.

CC: qemu-stable@nongnu.org
Signed-off-by: NPeter Lieven <pl@kamp.de>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Before:
    $ ./qemu-io-old
    qemu-io-old> open -r -o file.driver=iscsi,file.filename=foo
    Failed to parse URL : foo
    qemu-io-old: can't open device (null): Could not open 'foo': Invalid argument

After:
    $ ./qemu-io
    qemu-io> open -r -o file.driver=iscsi,file.filename=foo
    qemu-io: can't open device (null): Failed to parse URL : foo
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Reviewed-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

error_is_set(&var) is the same as var != NULL, but it takes
whole-program analysis to figure that out.  Unnecessarily hard for
optimizers, static checkers, and human readers.  Dumb it down to
obvious.

Gets rid of several dozen Coverity false positives.

Note that the obvious form is already used in many places.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NAndreas Färber <afaerber@suse.de>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

iSCSI currently does not need to do any actions to support the
current usage of bdrv_reopen().  However, it is important to note
a couple of things: 1.) A connection will not be re-established to
an iSCSI target, and 2.) If iscsi_open() is changed to parse 'flags',
then iscsi_reopen_prepare() may need to be more than a stub.

In light of the above, this commit adds comments above both of the
functions to bring attention to these facts.
Signed-off-by: NJeff Cody <jcody@redhat.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>

the opt_transfer_length has nothing to do with logical
block provisioning stuff so always copy it from
the block limits VPD page.
Reported-By: NBenoit Canet <benoit@irqsave.net>
Signed-off-by: NPeter Lieven <pl@kamp.de>
Reviewed-by: NBenoit Canet <benoit@irqsave.net>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>

The iSCSI backend already gets the block size from the READ CAPACITY
command it sends.  Save it so that the generic block layer gets it
too.
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>

This function separates filling the BlockLimits from bdrv_open(), which
allows it to call it from other operations which may change the limits
(e.g. modifications to the backing file chain or bdrv_reopen)
Signed-off-by: NKevin Wolf <kwolf@redhat.com>
Reviewed-by: NMax Reitz <mreitz@redhat.com>
Reviewed-by: NBenoit Canet <benoit@irqsave.net>

if an async libiscsi call fails directly it can only be due
to an out of memory condition. All other errors are returned
through the callback.
Signed-off-by: NPeter Lieven <pl@kamp.de>
Reviewed-by: NRonnie Sahlberg <ronniesahlberg@gmail.com>
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NKevin Wolf <kwolf@redhat.com>