[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-commits] [qemu/qemu] 2058c2: qemu-img: Report bdrv_block_status fa
|
From: |
Peter Maydell |
|
Subject: |
[Qemu-commits] [qemu/qemu] 2058c2: qemu-img: Report bdrv_block_status failures |
|
Date: |
Mon, 01 Apr 2019 22:26:25 -0700 |
Branch: refs/heads/master
Home: https://github.com/qemu/qemu
Commit: 2058c2ad261de7f58fae01d63d3d0efa484caf2a
https://github.com/qemu/qemu/commit/2058c2ad261de7f58fae01d63d3d0efa484caf2a
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M qemu-img.c
Log Message:
-----------
qemu-img: Report bdrv_block_status failures
If bdrv_block_status_above() fails, we are aborting the convert
process but failing to print an error message. Broken in commit
690c7301 (v2.4) when rewriting convert's logic.
Discovered when teaching nbdkit to support NBD_CMD_BLOCK_STATUS, and
accidentally violating the protocol by returning more than one extent
in spite of qemu asking for NBD_CMD_FLAG_REQ_ONE. The qemu NBD code
should probably handle the server's non-compliance more gracefully
than failing with EINVAL, but qemu-img shouldn't be silently
squelching any block status failures. It doesn't help that qemu 3.1
masks the qemu-img bug with extra noise that the nbd code is dumping
to stderr (that noise was cleaned up in d8b4bad8).
Reported-by: Richard W.M. Jones <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Kevin Wolf <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: a39286dd61e455014694cb6aa44cfa9e5c86d101
https://github.com/qemu/qemu/commit/a39286dd61e455014694cb6aa44cfa9e5c86d101
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd-client.c
M block/trace-events
Log Message:
-----------
nbd: Tolerate some server non-compliance in NBD_CMD_BLOCK_STATUS
The NBD spec states that NBD_CMD_FLAG_REQ_ONE (which we currently
always use) should not reply with an extent larger than our request,
and that the server's response should be exactly one extent. Right
now, that means that if a server sends more than one extent, we treat
the server as broken, fail the block status request, and disconnect,
which prevents all further use of the block device. But while good
software should be strict in what it sends, it should be tolerant in
what it receives.
While trying to implement NBD_CMD_BLOCK_STATUS in nbdkit, we
temporarily had a non-compliant server sending too many extents in
spite of REQ_ONE. Oddly enough, 'qemu-img convert' with qemu 3.1
failed with a somewhat useful message:
qemu-img: Protocol error: invalid payload for NBD_REPLY_TYPE_BLOCK_STATUS
which then disappeared with commit d8b4bad8, on the grounds that an
error message flagged only at the time of coroutine teardown is
pointless, and instead we should rely on the actual failed API to
report an error - in other words, the 3.1 behavior was masking the
fact that qemu-img was not reporting an error. That has since been
fixed in the previous patch, where qemu-img convert now fails with:
qemu-img: error while reading block status of sector 0: Invalid argument
But even that is harsh. Since we already partially relaxed things in
commit acfd8f7a to tolerate a server that exceeds the cap (although
that change was made prior to the NBD spec actually putting a cap on
the extent length during REQ_ONE - in fact, the NBD spec change was
BECAUSE of the qemu behavior prior to that commit), it's not that much
harder to argue that we should also tolerate a server that sends too
many extents. But at the same time, it's nice to trace when we are
being tolerant of server non-compliance, in order to help server
writers fix their implementations to be more portable (if they refer
to our traces, rather than just stderr).
Reported-by: Richard W.M. Jones <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: b29f3a3d2a5fab40dbb4a65fa2f91821ebffae51
https://github.com/qemu/qemu/commit/b29f3a3d2a5fab40dbb4a65fa2f91821ebffae51
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd-client.c
Log Message:
-----------
nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS
When the server replies with a (structured [*]) error to
NBD_CMD_BLOCK_STATUS, without any extent information sent first, the
client code was blindly throwing away the server's error code and
instead telling the caller that EIO occurred. This has been broken
since its introduction in 78a33ab5 (v2.12, where we should have called:
error_setg(&local_err, "Server did not reply with any status extents");
nbd_iter_error(&iter, false, -EIO, &local_err);
to declare the situation as a non-fatal error if no earlier error had
already been flagged, rather than just blindly slamming iter.err and
iter.ret), although it is more noticeable since commit 7f86068d, which
actually tries hard to preserve the server's code thanks to a separate
iter.request_ret.
[*] The spec is clear that the server is also permitted to reply with
a simple error, but that's a separate fix.
I was able to provoke this scenario with a hack to the server, then
seeing whether ENOMEM makes it back to the caller:
| diff --git a/nbd/server.c b/nbd/server.c
| index fd013a2817a..29c7995de02 100644
| --- a/nbd/server.c
| +++ b/nbd/server.c
| @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
| "discard failed", errp);
|
| case NBD_CMD_BLOCK_STATUS:
| + return nbd_send_generic_reply(client, request->handle, -ENOMEM,
| + "no status for you today", errp);
| if (!request->len) {
| return nbd_send_generic_reply(client, request->handle, -EINVAL,
| "need non-zero length", errp);
| --
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: ebd82cd872726549d0a55d329d22c731e2e660ff
https://github.com/qemu/qemu/commit/ebd82cd872726549d0a55d329d22c731e2e660ff
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd-client.c
Log Message:
-----------
nbd: Permit simple error to NBD_CMD_BLOCK_STATUS
The NBD spec is clear that when structured replies are active, a
simple error reply is acceptable to any command except for
NBD_CMD_READ. However, we were mistakenly requiring structured errors
for NBD_CMD_BLOCK_STATUS, and hanging up on a server that gave a
simple error (since qemu does not behave as such a server, we didn't
notice the problem until now). Broken since its introduction in
commit 78a33ab5 (v2.12).
Noticed while debugging a separate failure reported by nbdkit while
working out its initial implementation of BLOCK_STATUS, although it
turns out that nbdkit also chose to send structured error replies for
BLOCK_STATUS, so I had to manually provoke the situation by hacking
qemu's server to send a simple error reply:
| diff --git i/nbd/server.c w/nbd/server.c
| index fd013a2817a..833288d7c45 100644
| 00--- i/nbd/server.c
| +++ w/nbd/server.c
| @@ -2269,6 +2269,8 @@ static coroutine_fn int nbd_handle_request(NBDClient
*client,
| "discard failed", errp);
|
| case NBD_CMD_BLOCK_STATUS:
| + return nbd_co_send_simple_reply(client, request->handle, ENOMEM,
| + NULL, 0, errp);
| if (!request->len) {
| return nbd_send_generic_reply(client, request->handle, -EINVAL,
| "need non-zero length", errp);
|
Signed-off-by: Eric Blake <address@hidden>
Acked-by: Richard W.M. Jones <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: 30065d142443981924786da72828ba683da35e8f
https://github.com/qemu/qemu/commit/30065d142443981924786da72828ba683da35e8f
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M qemu-img.c
Log Message:
-----------
qemu-img: Gracefully shutdown when map can't finish
Trying 'qemu-img map -f raw nbd://localhost:10809' causes the
NBD server to output a scary message:
qemu-nbd: Disconnect client, due to: Failed to read request: Unexpected
end-of-file before all bytes were read
This is because the NBD client, being remote, has no way to expose a
human-readable map (the --output=json data is fine, however). But
because we exit(1) right after the message, causing the client to
bypass all block cleanup, the server sees the abrupt exit and warns,
whereas it would be silent had the client had a chance to send
NBD_CMD_DISC. Other protocols may have similar cleanup issues, where
failure to blk_unref() could cause unintended effects.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: John Snow <address@hidden>
Reviewed-by: Kevin Wolf <address@hidden>
Commit: 737d3f524481bb2ef68d3eba1caa636ff143e16a
https://github.com/qemu/qemu/commit/737d3f524481bb2ef68d3eba1caa636ff143e16a
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd-client.c
Log Message:
-----------
nbd-client: Work around server BLOCK_STATUS misalignment at EOF
The NBD spec is clear that a server that advertises a minimum block
size should reply to NBD_CMD_BLOCK_STATUS with extents aligned
accordingly. However, we know that the qemu NBD server implementation
has had a corner-case bug where it is not compliant with the spec,
present since the introduction of NBD_CMD_BLOCK_STATUS in qemu 2.12
(and unlikely to be patched in time for 4.0). Namely, when qemu is
serving a file that is not a multiple of 512 bytes, it rounds the size
advertised over NBD up to the next sector boundary (someday, I'd like
to fix that to be byte-accurate, but it's a much bigger audit not
appropriate for this release); yet if the final sector contains data
prior to EOF, lseek(SEEK_HOLE) will point to the implicit hole
mid-sector which qemu then reported over NBD.
We are well within our rights to hang up on a server that can't follow
the spec, but it is more useful to try and keep the connection alive
in spite of the problem. Do so by tracing a message about the problem,
and then either truncating the request back to an aligned boundary (if
it covered more than the final sector) or widening it out to the full
boundary with a forced status of data (since truncating would result
in 0 bytes, but we have to make progress, and valid since data is a
default-safe answer). And in practice, since the problem only happens
on a sector that starts with data and ends with a hole, we are going
to want to read that full sector anyway (where qemu as the server
fills in the tail beyond EOF with appropriate NUL bytes).
Easy reproduction:
$ printf %1000d 1 > file
$ qemu-nbd -f raw -t file & pid=$!
$ qemu-img map --output=json -f raw nbd://localhost:10809
qemu-img: Could not read file metadata: Invalid argument
$ kill $pid
where the patched version instead succeeds with:
[{ "start": 0, "length": 1024, "depth": 0, "zero": false, "data": true}]
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: e9dce9cb6eae57834cd80324ff43069299198bab
https://github.com/qemu/qemu/commit/e9dce9cb6eae57834cd80324ff43069299198bab
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
A tests/qemu-iotests/241
A tests/qemu-iotests/241.out
M tests/qemu-iotests/group
Log Message:
-----------
iotests: Add 241 to test NBD on unaligned images
Add a test for the NBD client workaround in the previous patch. It's
not really feasible for an iotest to assume a specific tracing engine,
so we can't really probe trace_nbd_parse_blockstatus_compliance to see
if the server was fixed vs. whether the client just worked around the
server (other than by rearranging order between code patches and this
test). But having a successful exchange sure beats the previous state
of an error message. Since format probing can change alignment, we can
use that as an easy way to test several configurations.
Not tested yet, but worth adding to this test in future patches: an
NBD server that can advertise a non-sector-aligned size (such as
nbdkit) causes qemu as the NBD client to misbehave when it rounds the
size up and accesses beyond the advertised size. Qemu as NBD server
never advertises a non-sector-aligned size (since bdrv_getlength()
currently rounds up to sector boundaries); until qemu can act as such
a server, testing that flaw will have to rely on external binaries.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Tested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
[eblake: add forced-512 alignment, and nbdkit reproducer comment]
Commit: 7da537f70d929800ba9c657b8a47a7b827695ccc
https://github.com/qemu/qemu/commit/7da537f70d929800ba9c657b8a47a7b827695ccc
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd.c
Log Message:
-----------
nbd/client: Lower min_block for block-status, unaligned size
We have a latent bug in our NBD client code, tickled by the brand new
nbdkit 1.11.10 block status support:
$ nbdkit --filter=log --filter=truncate -U - \
data data="1" size=511 truncate=64K logfile=/dev/stdout \
--run 'qemu-img convert $nbd /var/tmp/out'
...
qemu-img: block/io.c:2122: bdrv_co_block_status: Assertion `*pnum &&
QEMU_IS_ALIGNED(*pnum, align) && align > offset - aligned_offset' failed.
The culprit? Our implementation of .bdrv_co_block_status can return
unaligned block status for any server that operates with a lower
actual alignment than what we tell the block layer in
request_alignment, in violation of the block layer's constraints. To
date, we've been unable to trip the bug, because qemu as NBD server
always advertises block sizing (at which point it is a server bug if
the server sends unaligned status - although qemu 3.1 is such a server
and I've sent separate patches for 4.0 both to get the server to obey
the spec, and to let the client to tolerate server oddities at EOF).
But nbdkit does not (yet) advertise block sizing, and therefore is not
in violation of the spec for returning block status at whatever
boundaries it wants, and those unaligned results can occur anywhere
rather than just at EOF. While we are still wise to avoid sending
sub-sector read/write requests to a server of unknown origin, we MUST
consider that a server telling us block status without an advertised
block size is correct. So, we either have to munge unaligned answers
from the server into aligned ones that we hand back to the block
layer, or we have to tell the block layer about a smaller alignment.
Similarly, if the server advertises an image size that is not
sector-aligned, we might as well assume that the server intends to let
us access those tail bytes, and therefore supports a minimum block
size of 1, regardless of whether the server supports block status
(although we still need more patches to fix the problem that with an
unaligned image, we can send read or block status requests that exceed
EOF to the server). Again, qemu as server cannot trip this problem
(because it rounds images to sector alignment), but nbdkit advertised
unaligned size even before it gained block status support.
Solve both alignment problems at once by using better heuristics on
what alignment to report to the block layer when the server did not
give us something to work with. Note that very few NBD servers
implement block status (to date, only qemu and nbdkit are known to do
so); and as the NBD spec mentioned block sizing constraints prior to
documenting block status, it can be assumed that any future
implementations of block status are aware that they must advertise
block size if they want a minimum size other than 1.
We've had a long history of struggles with picking the right alignment
to use in the block layer, as evidenced by the commit message of
fd8d372d (v2.12) that introduced the current choice of forced 512-byte
alignment.
There is no iotest coverage for this fix, because qemu can't provoke
it, and I didn't want to make test 241 dependent on nbdkit.
Fixes: fd8d372d
Reported-by: Richard W.M. Jones <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Tested-by: Richard W.M. Jones <address@hidden>
Commit: a62a85ef5ccd764d03d72d6c3cd558f9755b3457
https://github.com/qemu/qemu/commit/a62a85ef5ccd764d03d72d6c3cd558f9755b3457
Author: Eric Blake <address@hidden>
Date: 2019-03-30 (Sat, 30 Mar 2019)
Changed paths:
M block/nbd-client.c
M tests/qemu-iotests/209.out
M tests/qemu-iotests/223.out
M tests/qemu-iotests/241.out
Log Message:
-----------
nbd/client: Report offsets in bdrv_block_status
It is desirable for 'qemu-img map' to have the same output for a file
whether it is served over file or nbd protocols. However, ever since
we implemented block status for NBD (2.12), the NBD protocol forgot to
inform the block layer that as the final layer in the chain, the
offset is valid; without an offset, the human-readable form of
qemu-img map gives up with the unhelpful:
$ nbdkit -U - data data="1" size=512 --run 'qemu-img map $nbd'
Offset Length Mapped to File
qemu-img: File contains external, encrypted or compressed clusters.
The --output=json form always works, because it is reporting the
lower-level bdrv_block_status results directly rather than trying to
filter out sparse ranges for human consumption - but now it also
shows the offset member.
With this patch, the human output changes to:
Offset Length Mapped to File
0 0x200 0
nbd+unix://?socket=/tmp/nbdkitOxeoLa/socket
This change is observable to several iotests.
Fixes: 78a33ab5
Reported-by: Richard W.M. Jones <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: 3add3ab78247fd347fd6f377a4b951022ac35d35
https://github.com/qemu/qemu/commit/3add3ab78247fd347fd6f377a4b951022ac35d35
Author: Eric Blake <address@hidden>
Date: 2019-04-01 (Mon, 01 Apr 2019)
Changed paths:
M nbd/client.c
Log Message:
-----------
nbd/client: Reject inaccessible tail of inconsistent server
The NBD spec suggests that a server should never advertise a size
inconsistent with its minimum block alignment, as that tail is
effectively inaccessible to a compliant client obeying those block
constraints. Since we have a habit of rounding up rather than
truncating, to avoid losing the last few bytes of user input, and we
cannot access the tail when the server advertises bogus block sizing,
abort the connection to alert the server to fix their bug. And
rejecting such servers matches what we already did for a min_block
that was not a power of 2 or which was larger than max_block.
Does not impact either qemu (which always sends properly aligned
sizes) or nbdkit (which does not send minimum block requirements yet);
so this is mostly aimed at new NBD server implementations, and ensures
that the rest of our code can assume the size is aligned.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: 9cf638508c0090b33ada4155c7cbb684e08e5ee9
https://github.com/qemu/qemu/commit/9cf638508c0090b33ada4155c7cbb684e08e5ee9
Author: Eric Blake <address@hidden>
Date: 2019-04-01 (Mon, 01 Apr 2019)
Changed paths:
M block/nbd-client.c
Log Message:
-----------
nbd/client: Support qemu-img convert from unaligned size
If an NBD server advertises a size that is not a multiple of a sector,
the block layer rounds up that size, even though we set info.size to
the exact byte value sent by the server. The block layer then proceeds
to let us read or query block status on the hole that it added past
EOF, which the NBD server is unlikely to be happy with. Fortunately,
qemu as a server never advertizes an unaligned size, so we generally
don't run into this problem; but the nbdkit server makes it easy to
test:
$ printf %1000d 1 > f1
$ ~/nbdkit/nbdkit -fv file f1 & pid=$!
$ qemu-img convert -f raw nbd://localhost:10809 f2
$ kill $pid
$ qemu-img compare f1 f2
Pre-patch, the server attempts a 1024-byte read, which nbdkit
rightfully rejects as going beyond its advertised 1000 byte size; the
conversion fails and the output files differ (not even the first
sector is copied, because qemu-img does not follow ddrescue's habit of
trying smaller reads to get as much information as possible in spite
of errors). Post-patch, the client's attempts to read (and query block
status, for new enough nbdkit) are properly truncated to the server's
length, with sane handling of the hole the block layer forced on
us. Although f2 ends up as a larger file (1024 bytes instead of 1000),
qemu-img compare shows the two images to have identical contents for
display to the guest.
I didn't add iotests coverage since I didn't want to add a dependency
on nbdkit in iotests. I also did NOT patch write, trim, or write
zeroes - these commands continue to fail (usually with ENOSPC, but
whatever the server chose), because we really can't write to the end
of the file, and because 'qemu-img convert' is the most common case
where we care about being tolerant (which is read-only). Perhaps we
could truncate the request if the client is writing zeros to the tail,
but that seems like more work, especially if the block layer is fixed
in 4.1 to track byte-accurate sizing (in which case this patch would
be reverted as unnecessary).
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Tested-by: Richard W.M. Jones <address@hidden>
Commit: 4841211e0d1628cd386b35835676d7f6f9a4fa9d
https://github.com/qemu/qemu/commit/4841211e0d1628cd386b35835676d7f6f9a4fa9d
Author: Eric Blake <address@hidden>
Date: 2019-04-01 (Mon, 01 Apr 2019)
Changed paths:
M block/block-backend.c
M include/sysemu/block-backend.h
Log Message:
-----------
block: Add bdrv_get_request_alignment()
The next patch needs access to a device's minimum permitted
alignment, since NBD wants to advertise this to clients. Add
an accessor function, borrowing from blk_get_max_transfer()
for accessing a backend's block limits.
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Message-Id: <address@hidden>
Commit: b0245d6478ea5906e3d7a542244d5c015fd47bc7
https://github.com/qemu/qemu/commit/b0245d6478ea5906e3d7a542244d5c015fd47bc7
Author: Eric Blake <address@hidden>
Date: 2019-04-01 (Mon, 01 Apr 2019)
Changed paths:
M nbd/server.c
M tests/qemu-iotests/223.out
M tests/qemu-iotests/233.out
M tests/qemu-iotests/241.out
Log Message:
-----------
nbd/server: Advertise actual minimum block size
Both NBD_CMD_BLOCK_STATUS and structured NBD_CMD_READ will split their
reply according to bdrv_block_status() boundaries. If the block device
has a request_alignment smaller than 512, but we advertise a block
alignment of 512 to the client, then this can result in the server
reply violating client expectations by reporting a smaller region of
the export than what the client is permitted to address (although this
is less of an issue for qemu 4.0 clients, given recent client patches
to overlook our non-compliance at EOF). Since it's always better to
be strict in what we send, it is worth advertising the actual minimum
block limit rather than blindly rounding it up to 512.
Note that this patch is not foolproof - it is still possible to
provoke non-compliant server behavior using:
$ qemu-nbd --image-opts
driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file
That is arguably a bug in the blkdebug driver (it should never pass
back block status smaller than its alignment, even if it has to make
multiple bdrv_get_status calls and determine the
least-common-denominator status among the group to return). It may
also be possible to observe issues with a backing layer with smaller
alignment than the active layer, although so far I have been unable to
write a reliable iotest for that scenario (but again, an issue like
that could be argued to be a bug in the block layer, or something
where we need a flag to bdrv_block_status() to state whether the
result must be aligned to the current layer's limits or can be
subdivided for accuracy when chasing backing files).
Anyways, as blkdebug is not normally used, and as this patch makes our
server more interoperable with qemu 3.1 clients, it is worth applying
now, even while we still work on a larger patch series for the 4.1
timeframe to have byte-accurate file lengths.
Note that the iotests output changes - for 223 and 233, we can see the
server's better granularity advertisement; and for 241, the three test
cases have the following effects:
- natural alignment: the server's smaller alignment is now advertised,
and the hole reported at EOF is now the right result; we've gotten rid
of the server's non-compliance
- forced server alignment: the server still advertises 512 bytes, but
still sends a mid-sector hole. This is still a server compliance bug,
which needs to be fixed in the block layer in a later patch; output
does not change because the client is already being tolerant of the
non-compliance
- forced client alignment: the server's smaller alignment means that
the client now sees the server's status change mid-sector without any
protocol violations, but the fact that the map shows an unaligned
mid-sector hole is evidence of the block layer problems with aligned
block status, to be fixed in a later patch
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
[eblake: rebase to enhanced iotest 241 coverage]
Commit: 75d34eb98ca0bb2f49d607fcaefd9c482e56b15d
https://github.com/qemu/qemu/commit/75d34eb98ca0bb2f49d607fcaefd9c482e56b15d
Author: Eric Blake <address@hidden>
Date: 2019-04-01 (Mon, 01 Apr 2019)
Changed paths:
M block/nbd-client.c
M block/trace-events
Log Message:
-----------
nbd/client: Trace server noncompliance on structured reads
Just as we recently added a trace for a server sending block status
that doesn't match the server's advertised minimum block alignment,
let's do the same for read chunks. But since qemu 3.1 is such a
server (because it advertised 512-byte alignment, but when serving a
file that ends in data but is not sector-aligned, NBD_CMD_READ would
detect a mid-sector change between data and hole at EOF and the
resulting read chunks are unaligned), we don't want to change our
behavior of otherwise tolerating unaligned reads.
Note that even though we fixed the server for 4.0 to advertise an
actual block alignment (which gets rid of the unaligned reads at EOF
for posix files), we can still trigger it via other means:
$ qemu-nbd --image-opts
driver=blkdebug,align=512,image.driver=file,image.filename=/path/to/non-aligned-file
Arguably, that is a bug in the blkdebug block status function, for
leaking a block status that is not aligned. It may also be possible to
observe issues with a backing layer with smaller alignment than the
active layer, although so far I have been unable to write a reliable
iotest for that scenario.
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
Commit: 47175951a691a8a2d483f0cbc7f31b4e3814bffa
https://github.com/qemu/qemu/commit/47175951a691a8a2d483f0cbc7f31b4e3814bffa
Author: Peter Maydell <address@hidden>
Date: 2019-04-02 (Tue, 02 Apr 2019)
Changed paths:
M block/block-backend.c
M block/nbd-client.c
M block/nbd.c
M block/trace-events
M include/sysemu/block-backend.h
M nbd/client.c
M nbd/server.c
M qemu-img.c
M tests/qemu-iotests/209.out
M tests/qemu-iotests/223.out
M tests/qemu-iotests/233.out
A tests/qemu-iotests/241
A tests/qemu-iotests/241.out
M tests/qemu-iotests/group
Log Message:
-----------
Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2019-04-01' into
staging
nbd patches for 2019-04-01
- Better behavior of qemu-img map on NBD images
- Fixes for NBD protocol alignment corner cases:
- the server has fewer places where it sends reads or block status
not aligned to its advertised block size
- the client has more cases where it can work around server
non-compliance present in qemu 3.1
- the client now avoids non-compliant requests when interoperating
with nbdkit or other servers not advertising block size
# gpg: Signature made Mon 01 Apr 2019 15:06:54 BST
# gpg: using RSA key A7A16B4A2527436A
# gpg: Good signature from "Eric Blake <address@hidden>" [full]
# gpg: aka "Eric Blake (Free Software Programmer)
<address@hidden>" [full]
# gpg: aka "[jpeg image of size 6874]" [full]
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A
* remotes/ericb/tags/pull-nbd-2019-04-01:
nbd/client: Trace server noncompliance on structured reads
nbd/server: Advertise actual minimum block size
block: Add bdrv_get_request_alignment()
nbd/client: Support qemu-img convert from unaligned size
nbd/client: Reject inaccessible tail of inconsistent server
nbd/client: Report offsets in bdrv_block_status
nbd/client: Lower min_block for block-status, unaligned size
iotests: Add 241 to test NBD on unaligned images
nbd-client: Work around server BLOCK_STATUS misalignment at EOF
qemu-img: Gracefully shutdown when map can't finish
nbd: Permit simple error to NBD_CMD_BLOCK_STATUS
nbd: Don't lose server's error to NBD_CMD_BLOCK_STATUS
nbd: Tolerate some server non-compliance in NBD_CMD_BLOCK_STATUS
qemu-img: Report bdrv_block_status failures
Signed-off-by: Peter Maydell <address@hidden>
Compare: https://github.com/qemu/qemu/compare/230ce19814ec...47175951a691
| [Prev in Thread] |
Current Thread |
[Next in Thread] |
- [Qemu-commits] [qemu/qemu] 2058c2: qemu-img: Report bdrv_block_status failures,
Peter Maydell <=