On 06/19/2015 12:06 AM, Stefan Hajnoczi wrote:
> On Thu, Jun 18, 2015 at 10:36:39PM +0800, Wen Congyang wrote:
>> At 2015/6/18 20:55, Stefan Hajnoczi Wrote:
>>> On Thu, Jun 18, 2015 at 04:49:12PM +0800, Wen Congyang wrote:
+void bdrv_connect(BlockDriverState *bs, Error **errp)
+{
+
Am 22.06.2015 um 23:54 schrieb John Snow:
On 06/22/2015 09:09 AM, Peter Lieven wrote:
Am 22.06.2015 um 11:25 schrieb Stefan Hajnoczi:
On Fri, Jun 19, 2015 at 2:14 PM, Peter Lieven wrote:
Am 18.06.2015 um 11:36 schrieb Stefan Hajnoczi:
On Thu, Jun 18, 2015 at 10:29 AM, Peter Lieven wrote:
The PIO Setup FIS specification in the SATA 3.2 spec is an odd one for me:
It specifies that, due to the stringent timing of the ATA/PIO protocols
that the Setup FIS is always sent immediately prior to the Data FIS.
The Setup FIS in this case contains both the value of the register
before the dat
There are two things to fix here:
The first one is subtle: the PxSACT register in the AHCI HBA has different
semantics from the field it is shadowing, the ACT field in the
Set Device Bits FIS.
In the HBA register, PxSACT acts as a bitfield indicating outstanding
NCQ commands where a set bit indic
The Register D2H FIS should copy the current values of
the registers instead of just parroting back the same
values the guest sent back to it.
In this case, the SECTOR COUNT variables are actually
not generally meaningful in terms of standard commands
(See ATA8-AC3 Section 9.2 Normal Outputs), so
Migrate the NCQ queue. This is solely for the benefit of halted commands,
since anything else should have completed and had any relevant status
flushed to the HBA registers already.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 49 -
1 file changed,
cur_cmd is an internal bookmark that points to the
current AHCI Command Header being processed by the
AHCI state machine. With NCQ needing to occasionally
rely on some of the same AHCI helpers, we cannot use
cur_cmd and will need to grab explicit pointers instead.
In an attempt to begin relying on
Signed-off-by: John Snow
---
tests/ahci-test.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 3f06fd9..c30837b 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1200,13 +1200,13 @@ static void test_migr
While the rest of the AHCI device can rely on a single bookmarked
pointer for the AHCI Command Header currently being processed, NCQ
is asynchronous and may have many commands in flight simultaneously.
Add a cmdh pointer to the ncq_tfs object and make the sglist prepare
function take an AHCICmdHea
uint16_t isn't enough to hold the real sector count, since a value of
zero implies a full 64K sectors, so we need a uint32_t here.
We *could* cheat and pretend that this value is 0-based and fit it in
a uint16_t, but I'd rather waste 2 bytes instead of a future dev's
10 minutes when they forget to
prepare_buf should not always grab as many descriptors
as it can, sometimes it should self-limit.
For example, an NCQ transfer of 1 sector with a PRDT that
describes 4GiB of data should not copy 4GiB of data, it
should just transfer that first 512 bytes.
PIO is not affected, because the dma_buf_r
Signed-off-by: John Snow
---
tests/ahci-test.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index c30837b..87d7691 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1269,13 +1269,13 @@ static void test_halt
Signed-off-by: John Snow
---
hw/ide/ahci.c | 10 +-
hw/ide/ahci.h | 4 ++--
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6233059..7fcc6a2 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -45,7 +45,7 @@ do { \
} while (0)
static v
Upon a VM state change, retry the halted NCQ commands.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 18 ++
hw/ide/core.c | 7 +++
hw/ide/internal.h | 1 +
3 files changed, 26 insertions(+)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index a838317..6233059 100644
---
For migration and werror=stop/rerror=stop resume purposes,
it will be convenient to have the command handy inside of
ncq_tfs.
Eventually, we'd like to avoid reading from the FIS entirely
after the initial read, so this is a byte (hah!) sized step
in that direction.
Signed-off-by: John Snow
---
Handle NCQ failures for cases where we want to halt the VM on IO errors.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 17 +++--
hw/ide/ahci.h | 1 +
hw/ide/internal.h | 1 +
3 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index
When we add werror=stop or rerror=stop support to NCQ,
we'll want to take a codepath where we don't actually
complete the command, so factor that out into a new routine.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 32 +++-
1 file changed, 19 insertions(+), 13 deletio
We already checked this in the handle_cmd phase, so just
change this to an assertion and simplify the error logic.
(Also, fix the switch indent, because checkpatch.pl yelled.)
((Sorry for churn.))
Signed-off-by: John Snow
---
hw/ide/ahci.c | 60 ++
Split off execute_ncq_command so that we can call
it separately later if we desire.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 73 ++-
1 file changed, 42 insertions(+), 31 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 2d7
requires: 1434470575-21625-1-git-send-email-js...@redhat.com
1435016308-6150-1-git-send-email-js...@redhat.com
[PATCH v2 0/4] ahci: misc fixes/tests for 2.4
[PATCH v2 00/16] ahci: ncq cleanup, part 1
This chunk gets NCQ migration and and resume support working.
There
The wait command should check to make sure SACT is clear as well
as the Command Issue register.
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 9a4d510..da02e2e 100644
NCQ commands are LBA48 by definition.
See SATA 3.2 13.6.4.1 "READ FPDMA QUEUED", or
SATA 3.2 13.6.5.1 "WRITE FPDMA QUEUED."
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/libqos/ahci.c b/tests/libqos/ahci.c
index 7c
NCQ commands have the concept of a "TAG" that they need to set,
but in the AHCI world, it is mandated that the TAG always match
the command slot that you executed the NCQ from.
See AHCI 9.3.1.1.5.2 "Native Queued Commands".
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 5 +
1 file chan
Signed-off-by: John Snow
---
tests/ahci-test.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 0d117fe..3f06fd9 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1146,9 +1146,9 @@ static void test_migrat
If you try to execute an NCQ command before trying to engage with the
device by issuing an IDENTIFY command, the error bits that are part of
the signature will fool the test suite into thinking there was a failure.
Issue IDENTIFY first on "boot", which will clear the signature out of
the registers
This value should not be size-corrected, 0 sectors does not imply
1 sector(s). This is just debug information, but it's misleading!
Signed-off-by: John Snow
---
hw/ide/ahci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index c2e6642..95
NCQ commands will expect the SDBS interrupt,
and in the normative case, do not expect to see
a D2H Register FIS unless something went wrong.
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/libqos/ahci.c b/tests/l
Test the NCQ pathways for a simple IO RW test.
Also, test that libqos doesn't explode when
running NCQ commands :)
Signed-off-by: John Snow
---
tests/ahci-test.c | 13 +
tests/libqos/ahci.c | 46 +-
tests/libqos/ahci.h | 27 ++
NCQ frames are generated a little differently than
their non-NCQ cousins. Add support for them.
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 44 +++-
tests/libqos/ahci.h | 29 -
2 files changed, 63 insertions(+), 10 deleti
NCQ commands should not / do not update the byte count
in the command header post command, so this field is
meaningless for NCQ tests.
Signed-off-by: John Snow
---
tests/libqos/ahci.c | 46 --
tests/libqos/ahci.h | 3 +--
2 files changed, 25 insertion
Several fields of the NCQFIS structure are ambiguously named. This patch
clarifies the intended (if unsupported) usage of the NCQ fields to aid
in creating more meaningful debug messages through the NCQ codepaths.
Signed-off-by: John Snow
---
hw/ide/ahci.h | 35 +-
Set some appropriate error bits for NCQ for us.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 14eccb8..375aa44 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -922,6 +922,15 @@
Most of the time, these bits can be safely ignored. For the purposes
of debugging however, it's nice to know that they're not being used.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 14
There's no real reason to have it bundled together, and this way
is a little nicer to follow if you have the AHCI spec pulled up.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 23 ---
hw/ide/ahci.h | 3 ++-
2 files changed, 14 insertions(+), 12 deletions(-)
diff --git a/hw/i
Trivial cleanup that I didn't want to tack-on to anything else.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 26df2ca..14eccb8 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@
Don't attempt the NCQ transfer if the PRDT we were given is not big
enough to perform the entire transfer.
Signed-off-by: John Snow
---
hw/ide/ahci.c | 20 +++-
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 375aa44..24c4832 100
requires: 1434470575-21625-1-git-send-email-js...@redhat.com
[PATCH v2 0/4] ahci: misc fixes/tests for 2.4
This series adds a couple of tests to exercise the NCQ pathways
and establish a baseline for us.
Most of these patches are fairly short and should be relatively trivial
to review.
From: Jeff Cody
Currently, node_name is only filled in when done so explicitly by the
user. If no node_name is specified, then the node name field is not
populated.
If node_names are automatically generated when not specified, that means
that all block job operations can be done by reference to
LGTM
It is good to finally have timeouts that work in libiscsi, and a consumer
that can use and benefit from it.
On Tue, Jun 16, 2015 at 4:45 AM, Peter Lieven wrote:
> libiscsi starting with 1.15 will properly support timeout of iscsi
> commands. The default will remain no timeout, but this ca
On 06/22/2015 09:09 AM, Peter Lieven wrote:
> Am 22.06.2015 um 11:25 schrieb Stefan Hajnoczi:
>> On Fri, Jun 19, 2015 at 2:14 PM, Peter Lieven wrote:
>>> Am 18.06.2015 um 11:36 schrieb Stefan Hajnoczi:
On Thu, Jun 18, 2015 at 10:29 AM, Peter Lieven wrote:
> Am 18.06.2015 um 10:42 schri
On 06/22/2015 10:43 AM, John Snow wrote:
>
>
> On 06/22/2015 10:38 AM, Stefan Hajnoczi wrote:
>> On Fri, Jun 19, 2015 at 09:50:39PM -0400, John Snow wrote:
>>> The legacy ide command execution layer will clear any errors
>>> outstanding before execution, but the NCQ layer doesn't. Even on
>>> s
v8:
- Rebased on top of Stefan's block branch (0a35bce416)
- The loop that pauses the block jobs in bdrv_drain_all() is now split
in two: one that iterates the list of block jobs to stop them, and
one that iterates the root bds in order to get the aio contexts.
v7: https://lists.gnu.org/archiv
This test case checks that it's possible to launch several stream
operations in parallel in the same snapshot chain, each one involving
a different set of nodes.
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
tests/qemu-iotests/030 | 80
This test case checks that it's not possible to perform two
block-stream operations if there are nodes involved in both.
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
tests/qemu-iotests/030 | 27 +++
tests/qemu-iotests/030.out | 4 ++--
2 files changed, 2
This test is streaming to the top layer using the intermediate image
as the base. This is a mistake since block-stream never copies data
from the base image and its backing chain, so this is effectively a
no-op.
In addition to fixing the base parameter, this patch also writes some
data to the inte
This adds test_stream_intermediate(), similar to test_stream() but
streams to the intermediate image instead.
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
tests/qemu-iotests/030 | 18 +-
tests/qemu-iotests/030.out | 4 ++--
2 files changed, 19 insertions(+), 3 d
This patch makes the 'device' parameter of the 'block-stream' command
accept a node name as well as a device name.
In addition to that, operation blockers will be checked in all
intermediate nodes between the top and the base node.
Since qmp_block_stream() now uses the error from bdrv_lookup_bs()
The current way to obtain the list of existing block jobs is to
iterate over all root nodes and check which ones own a job.
Since we want to be able to support block jobs in other nodes as well,
this patch keeps a list of jobs that is updated every time one is
created or destroyed.
This also upda
Currently, block jobs can only be owned by root nodes. This patch
allows block jobs to be in any arbitrary node, by making the following
changes:
- Block jobs can now be identified by the node name of their
BlockDriverState in addition to the device name. Since both device
and node names live
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
Reviewed-by: Eric Blake
---
docs/live-block-ops.txt | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/docs/live-block-ops.txt b/docs/live-block-ops.txt
index a257087..a05d869 100644
--- a/do
This makes sure that the image we are steaming into is open in
read-write mode during the operation.
Operation blockers are also set in all intermediate nodes, since they
will be removed from the chain afterwards.
Finally, this also unblocks the stream operation in backing files.
Signed-off-by:
This patch tests that in a partial block-stream operation, no data is
ever copied from the base image.
Signed-off-by: Alberto Garcia
Reviewed-by: Max Reitz
---
tests/qemu-iotests/030 | 18 ++
tests/qemu-iotests/030.out | 4 ++--
2 files changed, 20 insertions(+), 2 deletion
We need to call stream_complete() in order to do all the necessary
clean-ups, even if there's an early failure. At the moment it's only
useful to make sure that s->backing_file_str is not leaked, but it
will become more important as we introduce support for streaming to
any intermediate node.
Sign
On Wed, May 13, 2015 at 04:27:30PM +0300, Alberto Garcia wrote:
> v7:
> - Rebased against the current master
> - Updated bdrv_drain_all() to use the new block_job_next() API.
Please rebase onto https://github.com/stefanha/qemu 'block' branch and
check that qemu-iotests 015 030 032 038 046 pass:
On 22 June 2015 at 10:59, Markus Armbruster wrote:
> What about this instead:
>
> 1. When -device creation connects a qdev_prop_drive property to a
> backend, fail when the backend has a DriveInfo and the DriveInfo has
> type != IF_NONE. Note: the connection is made in parse_drive().
>
> 2. This
On Fri, Jun 19, 2015 at 09:50:31PM -0400, John Snow wrote:
> requires: 1434470575-21625-1-git-send-email-js...@redhat.com
> [PATCH v2 0/4] ahci: misc fixes/tests for 2.4
>
> This series adds a couple of tests to exercise the NCQ pathways
> and establish a baseline for us.
>
> Most of th
Peter Maydell writes:
> On 22 June 2015 at 10:59, Markus Armbruster wrote:
>> What about this instead:
>>
>> 1. When -device creation connects a qdev_prop_drive property to a
>> backend, fail when the backend has a DriveInfo and the DriveInfo has
>> type != IF_NONE. Note: the connection is made
On 06/22/2015 10:38 AM, Stefan Hajnoczi wrote:
> On Fri, Jun 19, 2015 at 09:50:39PM -0400, John Snow wrote:
>> The legacy ide command execution layer will clear any errors
>> outstanding before execution, but the NCQ layer doesn't. Even on
>> success, this register will remain clogged.
>>
>> Cle
On Fri, Jun 19, 2015 at 09:50:39PM -0400, John Snow wrote:
> The legacy ide command execution layer will clear any errors outstanding
> before execution, but the NCQ layer doesn't.
> Even on success, this register will remain clogged.
>
> Clear it out before each NCQ command so the guest can tell
On Fri, Jun 19, 2015 at 09:50:35PM -0400, John Snow wrote:
> -/* Note: We calculate the sector count, but don't currently rely on it.
> - * The total size of the DMA buffer tells us the transfer size instead.
> */
> ncq_tfs->sector_count = ((uint16_t)ncq_fis->sector_count_high << 8) |
On Fri, Jun 19, 2015 at 09:50:37PM -0400, John Snow wrote:
> @@ -1003,6 +1003,27 @@ static void process_ncq_command(AHCIState *s, int
> port, uint8_t *cmd_fis,
> (uint64_t)ncq_fis->lba0;
> ncq_tfs->tag = tag;
>
> +#ifdef DEBUG_AHCI
These sorts of debug ifdefs have a ten
On 06/22/2015 10:06 AM, Stefan Hajnoczi wrote:
> On Fri, Jun 19, 2015 at 09:50:35PM -0400, John Snow wrote:
>> @@ -999,20 +1000,28 @@ static void process_ncq_command(AHCIState
>> *s, int port, uint8_t *cmd_fis, ((uint64_t)ncq_fis->lba2 << 16)
>> | ((uint64_t)ncq_fis->lba1 << 8) | (uint64_t)ncq_fi
On Fri, Jun 19, 2015 at 09:50:35PM -0400, John Snow wrote:
> @@ -999,20 +1000,28 @@ static void process_ncq_command(AHCIState *s, int
> port, uint8_t *cmd_fis,
> ((uint64_t)ncq_fis->lba2 << 16) |
> ((uint64_t)ncq_fis->lba1 << 8) |
> (uint
At 2015/6/22 20:39, Stefan Hajnoczi Wrote:
On Sat, Jun 20, 2015 at 11:31:52AM +0800, Wen Congyang wrote:
At 2015/6/19 18:49, Stefan Hajnoczi Wrote:
On Fri, Jun 19, 2015 at 08:54:56AM +0800, Wen Congyang wrote:
On 06/19/2015 12:06 AM, Stefan Hajnoczi wrote:
On Thu, Jun 18, 2015 at 10:36:39PM +
On 22 June 2015 at 10:59, Markus Armbruster wrote:
> What about this instead:
>
> 1. When -device creation connects a qdev_prop_drive property to a
> backend, fail when the backend has a DriveInfo and the DriveInfo has
> type != IF_NONE. Note: the connection is made in parse_drive().
So, the rea
Am 22.06.2015 um 11:25 schrieb Stefan Hajnoczi:
On Fri, Jun 19, 2015 at 2:14 PM, Peter Lieven wrote:
Am 18.06.2015 um 11:36 schrieb Stefan Hajnoczi:
On Thu, Jun 18, 2015 at 10:29 AM, Peter Lieven wrote:
Am 18.06.2015 um 10:42 schrieb Kevin Wolf:
Am 18.06.2015 um 10:30 hat Peter Lieven gesch
On Sat, Jun 20, 2015 at 11:31:52AM +0800, Wen Congyang wrote:
> At 2015/6/19 18:49, Stefan Hajnoczi Wrote:
> >On Fri, Jun 19, 2015 at 08:54:56AM +0800, Wen Congyang wrote:
> >>On 06/19/2015 12:06 AM, Stefan Hajnoczi wrote:
> >>>On Thu, Jun 18, 2015 at 10:36:39PM +0800, Wen Congyang wrote:
> At
On Mon, Jun 22, 2015 at 01:18:43PM +0300, Dimitris Aragiorgis wrote:
> So I suggest we go with the submitted patch taking into account Eric's
> proposal: The code first stat()s the given filename to ensure it is a
> character device node, and then it issues the SG_GET_VERSION_NUM and
> SG_GET_SCSI_
Peter Maydell writes:
> On 22 June 2015 at 10:39, Markus Armbruster wrote:
>> Peter Maydell writes:
>>
>>> Instead of having set_pointer() call a parse callback which returns
>>> an error number that we then convert to an Error string with
>>> error_set_from_qdev_prop_error(), make the parse ca
Peter Maydell writes:
> On 22 June 2015 at 10:12, Markus Armbruster wrote:
>> We generally do not end error messages with a period.
>>
>> The message for auto_claimed drives is of the form
>>
>> LOCATION: This went wrong. Advice on how to fix it.
>>
>> All in one awfully long line. A frien
Hello Stefan,
Yes, you are right. Using realpath() is a workaround for supporting
symlinks, as long as they point to a path starting with "/dev/sg". I
will remove this reference in the revised version of this patch.
However, it still holds that determining whether a filename is an SG
device or no
On 22 June 2015 at 10:12, Markus Armbruster wrote:
> I think we should just bite the bullet and extend Error to support
> additional helpful information for humans.
...I thought all of the string was already just helpful information
for humans? I certainly hope we aren't expecting anybody to pars
On 22 June 2015 at 10:39, Markus Armbruster wrote:
> Peter Maydell writes:
>
>> Instead of having set_pointer() call a parse callback which returns
>> an error number that we then convert to an Error string with
>> error_set_from_qdev_prop_error(), make the parse callback take an
>> Error** and s
Peter Maydell writes:
> Improve the diagnosis of command line errors where the user requested
> an automatic connection of a drive (via if=, or by not
> setting if= and using the board-default-if). We already fail this
> case if the board actually handles if=, but if the board
> did not auto-conn
On Fri, Jun 19, 2015 at 2:14 PM, Peter Lieven wrote:
> Am 18.06.2015 um 11:36 schrieb Stefan Hajnoczi:
>> On Thu, Jun 18, 2015 at 10:29 AM, Peter Lieven wrote:
>>> Am 18.06.2015 um 10:42 schrieb Kevin Wolf:
Am 18.06.2015 um 10:30 hat Peter Lieven geschrieben:
> Am 18.06.2015 um 09:45 sch
Peter Maydell writes:
> Instead of having set_pointer() call a parse callback which returns
> an error number that we then convert to an Error string with
> error_set_from_qdev_prop_error(), make the parse callback take an
> Error** and set the error itself. This will allow parse routines
> to pr
Peter Maydell writes:
> If the user forgot if=none on their drive specification they're likely
> to get an error message because the drive is assigned once automatically
> by QEMU and once by the manual id=/drive= user command line specification.
> Improve the error message produced in this case
77 matches
Mail list logo
