The Report supported operation codes command is very closely integrated
into the table driven parser and very useful for testing it. Its cdb
masks form the basis of the 'strict' parameter's checks. The Report
supported TMFs command is a simple extension. The Compare and write
command may even be useful, as it should be atomic due to the read-write
lock that the driver uses on its backing store (ram).
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

The existing 'big switch' parser in queuecommand() is changed to
a table driven parser. The old and new queuecommand() were moved
in the source so diff would not shuffle them. Apart from the new
tables most other changes are refactoring existing response code
to be more easily called out of the table parser. The 'strict'
parameter is added so that cdb_s can be checked for non-zero
values in parts of the cdb that are reserved. Some other changes
include: tweak request sense response when D_SENSE differs; support
NDOB in Write Same(16); and fix crash in Get LBA Status when LBP
was inactive.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Via sysfs the virtual_gb scsi_debug parameter can be changed while
LUs are in use. If that changes, the 'Capacity data has changed'
Unit Attention is queued on all LUs.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

The way the existing scsi_debug command parser associated various
inject error flags to a command was difficult to replicate in the
table driven parser. This patch adds infrastructure to append those
flags to the end of a scsi_cmnd object with the cmd_size host
template option.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Use Sense Key Specific field in the sense data of an ILLEGAL REQUEST
to optionally pinpoint the location of the problem field. This may
be either in the cdb or the associated parameter list.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Drop the now unused reason argument from the ->change_queue_depth method.
Also add a return value to scsi_adjust_queue_depth, and rename it to
scsi_change_queue_depth now that it can be used as the default
->change_queue_depth implementation.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NMike Christie <michaelc@cs.wisc.edu>
Reviewed-by: NHannes Reinecke <hare@suse.de>

All drivers use the implementation for ramping the queue up and down, so
instead of overloading the change_queue_depth method call the
implementation diretly if the driver opts into it by setting the
track_queue_depth flag in the host template.

Note that a few drivers validated the new queue depth in their
change_queue_depth method, but as we never go over the queue depth
set during slave_configure or the sysfs file this isn't nessecary
and can safely be removed.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NMike Christie <michaelc@cs.wisc.edu>
Reviewed-by: NHannes Reinecke <hare@suse.de>
Reviewed-by: NVenkatesh Srinivas <venkateshs@google.com>

Signed-off-by: NFinn Thain <fthain@telegraphics.com.au>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Now that we also get proper values in cmd->request->tag for untagged
commands, there is no need to force tagged_supported to on in drivers
that need host-wide tags.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NMike Christie <michaelc@cs.wisc.edu>
Reviewed-by: NHannes Reinecke <hare@suse.de>

Remove the tagged argument from scsi_adjust_queue_depth, and just let it
handle the queue depth.  For most drivers those two are fairly separate,
given that most modern drivers don't care about the SCSI "tagged" status
of a command at all, and many old drivers allow queuing of multiple
untagged commands in the driver.

Instead we start out with the ->simple_tags flag set before calling
->slave_configure, which is how all drivers actually looking at
->simple_tags except for one worke anyway.  The one other case looks
broken, but I've kept the behavior as-is for now.

Except for that we only change ->simple_tags from the ->change_queue_type,
and when rejecting a tag message in a single driver, so keeping this
churn out of scsi_adjust_queue_depth is a clear win.

Now that the usage of scsi_adjust_queue_depth is more obvious we can
also remove all the trivial instances in ->slave_alloc or ->slave_configure
that just set it to the cmd_per_lun default.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NMike Christie <michaelc@cs.wisc.edu>
Reviewed-by: NHannes Reinecke <hare@suse.de>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>

Most drivers use exactly the same implementation, so provide it as a
library function.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NBart Van Assche <bvanassche@acm.org>
Reviewed-by: NMike Christie <michaelc@cs.wisc.edu>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: NHannes Reinecke <hare@suse.de>

The kernel used to contain two functions for length-delimited,
case-insensitive string comparison, strnicmp with correct semantics and
a slightly buggy strncasecmp.  The latter is the POSIX name, so strnicmp
was renamed to strncasecmp, and strnicmp made into a wrapper for the new
strncasecmp to avoid breaking existing users.

To allow the compat wrapper strnicmp to be removed at some point in the
future, and to avoid the extra indirection cost, do
s/strnicmp/strncasecmp/g.
Signed-off-by: NRasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Signed-off-by: NAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>

A deadlock has been reported when the completion
of SCSI commands (simulated by a timer) was surprised
by a module removal. This patch removes one half of
the offending locks around timer deletions. This fix
is applied both to stop_all_queued() which is were
the deadlock was discovered and stop_queued_cmnd()
which has very similar logic.

This patch should be applied both to the lk 3.17 tree
and Christoph's drivers-for-3.18 tree.
Tested-and-reported-by: NMilan Broz <gmazyland@gmail.com>
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Since a lot of functionality from SPC-4 is supported by this
driver (e.g. LBP and PI) then bump the default INQUIRY version
from SPC-3 to SPC-4. Also update the INQUIRY version
descriptors.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Give existing errors priority over the generation of Task
Set Full (TSF) errors. So that max_queue is not exceeded,
existing errors may be sent back in the invocation thread.
This is done so errors like Unit Attentions are not hidden
and lost by either max_queue exceeded or real/injected
TSFs.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

This patch removes a NULL check for the scsi_cmnd::cmnd pointer
since many other instances in this driver and elsewhere assume
it is valid. Also redundant casts to 'unsigned char *' are removed
as the pointer has that type.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

 - add host_lock option whose default value is 0 which removes the
   host_lock around all queued commands
 - accept delay=-1 (_hi_) or -2 which use a tasklet to invoke
   the scsi_done callback into the mid-layer. The default
   is still delay=1 which uses a timer to delay 1 jiffy
 - wire .change_queue_depth and .change_queue_type
   functions to better simulate queueing in a modern LLD
 - add SCSI_DEBUG_OPT_Q_NOISE (0x200) mask to only produce
   debug output associated with queue full, plus from
   .change_queue_depth and .change_queue_type functions
 - add SCSI_DEBUG_OPT_ALL_TSF (0x400) mask which reports
   all queued_arr fulls at TASK_SET_FULL, otherwise
   SCSI_MLQUEUE_HOST_BUSY is returned
 - add SCSI_DEBUG_OPT_RARE_TSF (0x800) mask which works
   together with the every_nth option (> 0) to count
   occurrences of num_in_q==queue_depth. When every_nth
   is reached the victim (a command) yields TASK SET FULL
 - clean up many debug messages.
 - add ndelay=<nanosecs> option that uses high resolution
   timers; active if > 0 and then overrides delay= option
 - expand Unit Attention handling: POR, BUS_RESET and
   MODE PARAMETERS CHANGED
 - support .eh_target_reset_handler and drop .bios_param
 - add OPT_N_WCE mask so caching page yields WCE=0
 - add OPT_RESET_NOISE mask to log aborts and resets
 - add OPT_NO_CDB_NOISE mask to not log each cdb
 - MODE SELECT support for changing caching page's WCE
 - name common ioctls in log
 - when fake_rw=1, do not vmalloc fake store; make
   UNMAP and WRITE SAME obey fake_rw
 - more logging and code improvements including better
   sense buffer handling

With fio and four (pseudo) devices I have observed 1.2 M IOPS
on my equipment. Rob Elliott who has done much testing and made
numerous suggestions, has better IOPS results than mine.
Signed-off-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NRobert Elliott <elliott@hp.com>
Tested-by: NRobert Elliott <elliott@hp.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

The SCSI standard defines 64-bit values for LUNs, and large arrays
employing large or hierarchical LUN numbers become more and more
common.

So update the linux SCSI stack to use 64-bit LUN numbers.
Signed-off-by: NHannes Reinecke <hare@suse.de>
Reviewed-by: NChristoph Hellwig <hch@infradead.org>
Reviewed-by: NEwan Milne <emilne@redhat.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

This change enables to test read/write commands with huge transfer
length such as 1GB.  For example:

	# modprobe scsi_debug dev_size_mb=1024 clustering=1 opts=1
	# cat /sys/block/$DEV/queue/max_hw_sectors_kb > \
		/sys/block/$DEV/queue/max_sectors_kb
	# fio --name=test --rw=write --bs=1g --size=1g --filename=/dev/$DEV \
		--mem=mmaphuge  --direct=1

The data type of max_sectors in scsi_host_template has been extended
to unsigned int by the previous change.  So we can increase it from
0xffff to 0xffffffff to allow such huge transfer length.

Also, this increases sg_tablesize and max_segment_size, otherwise the
maximum transfer length is limited to 64MB.
(sg_tablesize * max_segment_size = 256 * 256KB)
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NChristoph Hellwig <hch@lst.de>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Acked by: Douglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NChristoph Hellwig <hch@lst.de>

Add an option to only transfer half the data for every n-th command.
Signed-off-by: NChristoph Hellwig <hch@lst.de>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NNicholas Bellinger <nab@linux-iscsi.org>

This adds a module parameter to enable clustering.

Without enabling clustering support, the transfer length for read and
write scsi commands is limited upto 8MB when page size is 4KB and
sg_tablesize is 2048 (= SCSI_MAX_SG_CHAIN_SEGMENTS).  I would like to
test commands with more than that transfer length.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

This change ensures that concurrent device access including ramdisk
storage, protection info, and provisioning map by read, write, and
unmap commands are protected with atomic_rw spinlock.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

Currently, clustering support for scsi_debug is disabled.  This is
because there are for_each_sg() loops which assume that each sg list
element is consisted with a single page.  But enabling clustering
support, each sg list element for scsi commands can be consisted with
multiple pages.

This replaces these for_each_sg() loops with sg mapping iterator which
is capable of handling each sg list element is consisted with multiple
pages.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

When resp_xdwriteread() can't allocate temporary buffer, it returns -1.
But the return value is used as scsi status code and -1 is not
interpreted as correct code.

target_core_mod has similar xdwriteread emulation code. So this mimics
what target_core_mod does for xdwriteread when running out of memory.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

It is unnecessary to increase dif_errors in dif_verify(), because the
caller will increment it when dif_verify() detects failure.

This bug was introduced by commit beb40ea4 ("[SCSI] scsi_debug:
reduce duplication between prot_verify_read and prot_verify_write")
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

As pseudo_primary is only used in scsi_debug.c, it should be static.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Reviewed-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

Reading partially unwritten sectors generates a false positive logical
block reference tag check failure when DIF is enabled.

This bug is caused by missing ei_lba increment in loop of dif_verify()
when unwritten sector is skipped.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Reviewed-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

Instead of repeatedly calling driver_create_file() to create driver
attribute files, This achieves the same thing by constructing an array
of driver_attribute and setting it to bus_type->drv_groups.

This change simplifies both creation and destruction of the attribute
files, and also removes sparse warning caused by driver_attributes which
are unnecessarily declared as global.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

Each member in data integrity field tuple is big-endian.  But the
endianness of the values being compared with these members are not
annotated.  So this fixes these sparse warnings.
Reported-by: Nkbuild test robot <fengguang.wu@intel.com>
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

In the module initialization, invalid value for guard module parameter
is detected by the following check:

        if (scsi_debug_guard > 1) {
                printk(KERN_ERR "scsi_debug_init: guard must be 0 or 1\n");
                return -EINVAL;
        }

But this check isn't enough, because the type of scsi_debug_guard is
'int' and scsi_debug_guard could be a negative value.

This fixes it by changing the type of scsi_debug_guard to 'unsigned int'
instead of adding extra check for a negative value.
Reported-by: NJoe Perches <joe@perches.com>
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

If data integrity support is enabled, prot_verify_write() is called in
response to WRITE commands and it verifies protection info from
prot_sglist by comparing against data sglist, and copies protection info
to dif_storep.

When multiple blocks are transfered by a WRITE command, it verifies and
copies these blocks one by one.  So if it fails to verify protection
info in the middle of blocks, the actual data transfer to fake_storep
isn't proceeded at all although protection info for some blocks are
already copied to dif_storep.  Therefore, it breaks the data integrity
between fake_storep and dif_storep.

This fixes it by ensuring that copying protection info to dif_storep is
done after all blocks are successfully verified.  Reusing dif_copy_prot()
with supporting the opposite direction simplifies this fix.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

If data integrity support is enabled, prot_verify_read() is called in
response to READ commands and it verifies protection info from dif_storep
by comparing against fake_storep, and copies protection info to
prot_sglist.

This factors out the portion of copying protection info into a separate
function.  It will also be reused in the next change after supporting
the opposite direction (copying prot_sglist to dif_storep).
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

If the module parameter virtual_gb is greater than 0, the READ command
may request the blocks which exceed actual ramdisk storage (fake_storep).
prot_verify_read() should treat those blocks as wrap around the end of
fake_storep.  But it actually causes fake_storep and dif_storep buffer
overruns.

This fixes these buffer overruns.  In order to simplify the fix,
this also introduces fake_store() and dif_store() which return
corresponding wrap around addresses.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NMartin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

Commit b90ebc3d ("[SCSI] scsi_debug:
fix logical block provisioning support") fixed several issues with
logical block provisioning support, but it still doesn't properly fix
the cases when unmap_alignment > 0.

For example, load scsi_debug module with the following module parameters
and make all blocks mapped by filling the storage with zero.

        # modprobe scsi_debug lbpu=1 unmap_alignment=1 unmap_granularity=4
        # dd if=/dev/zero of=$DEV

Then, try to unmap the first unmappable blocks at lba=1, but GET LBA STATUS
unexpectedly reports that the last UNMAP has done nothing.

        # sg_unmap --lba=1 --num=4 $DEV
        # sg_get_lba_status --lba=1 $DEV
        descriptor LBA: 0x0000000000000001  blocks: 16383  mapped

The problem is in map_index_to_lba(), which should return the first
LBA which is corresponding to a given index of provisioning map
(map_storep).
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: N"Martin K. Petersen" <martin.petersen@oracle.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

With module parameter num_parts > 0, partition table is built on the
ramdisk storage when loading the driver.  Unfortunately, there is an
endianness bug in sdebug_build_parts().  So the partition table is not
correctly initialized on big-endian systems.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Tested-by: NMartin Peschke <mpeschke@linux.vnet.ibm.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

do_device_access() is a function that abstracts copying SG list from/to
ramdisk storage (fake_storep).

It must deal with the ranges exceeding actual fake_storep size, because
such ranges are valid if virtual_gb is set greater than zero, and they
should be treated as fake_storep is repeatedly mirrored up to virtual
size.

Unfortunately, it can't deal with the range which wraps around the end of
fake_storep.  A wrap around range is copied by two
sg_copy_{from,to}_buffer() calls, but sg_copy_{from,to}_buffer() can't
copy from/to in the middle of SG list, therefore the second call can't
copy correctly.

This fixes it by using sg_pcopy_{from,to}_buffer() that can copy from/to
the middle of SG list.

This also simplifies the assignment of sdb->resid in
fill_from_dev_buffer().  Because fill_from_dev_buffer() is now only called
once per command execution cycle.  So it is not necessary to take care to
decrease sdb->resid if fill_from_dev_buffer() is called more than once.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: "James E.J. Bottomley" <JBottomley@parallels.com>
Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Horia Geanta <horia.geanta@freescale.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: NAndrew Morton <akpm@linux-foundation.org>
Signed-off-by: NLinus Torvalds <torvalds@linux-foundation.org>

In order to reduce code duplication between prot_verify_read() and
prot_verify_write(), this moves common code into the new functions.

[jejb: fix unitialised variable warning]
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Acked-by: N"Martin K. Petersen" <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

dif_storep is declared as pointer to unsigned char type.  But it is
actually used to store vmalloced array of struct sd_dif_tuple.

This changes the type of dif_storep to the pointer to struct sd_dif_tuple.
It simplifies offset calculation for dif_storep and enables to remove
hardcoded size of struct sd_dif_tuple.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Acked-by: N"Martin K. Petersen" <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

When UNMAP command is issued with the data integrity support enabled,
the protection info for the unmapped region is remain unchanged.
So READ command for the region later on causes data integrity failure.

This fixes it by invalidating protection info for the unmapped region
by filling with 0xff pattern.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Acked-by: N"Martin K. Petersen" <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>

The protection info dif_storep is allocated only when parameter dif is
not zero.  But it will be accessed when reading or writing to the storage
installed with parameter dix is not zero.

So kernel crashes if scsi_debug module is loaded with parameters dix=1 and
dif=0.

This fixes it by making dif_storep available if parameter dix is not zero
instead of checking if parameter dif is not zero.
Signed-off-by: NAkinobu Mita <akinobu.mita@gmail.com>
Acked-by: NDouglas Gilbert <dgilbert@interlog.com>
Acked-by: N"Martin K. Petersen" <martin.petersen@oracle.com>
Signed-off-by: NJames Bottomley <JBottomley@Parallels.com>