a7b78fc9 | 18-Feb-2019 |
Kevin Wolf <kwolf@redhat.com> |
nbd: Move nbd_read_eof() to nbd/client.c
The only caller of nbd_read_eof() is nbd_receive_reply(), so it doesn't have to live in the header file, but can move next to its caller.
Also add the missi
nbd: Move nbd_read_eof() to nbd/client.c
The only caller of nbd_read_eof() is nbd_receive_reply(), so it doesn't have to live in the header file, but can move next to its caller.
Also add the missing coroutine_fn to the function and its caller.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
show more ...
|
269ee27e | 07-Feb-2019 |
Eric Blake <eblake@redhat.com> |
nbd/server: Kill pointless shadowed variable
lgtm.com pointed out that commit 678ba275 introduced a shadowed declaration of local variable 'bs'; thankfully, the inner 'bs' obtained by 'blk_bs(blk)'
nbd/server: Kill pointless shadowed variable
lgtm.com pointed out that commit 678ba275 introduced a shadowed declaration of local variable 'bs'; thankfully, the inner 'bs' obtained by 'blk_bs(blk)' matches the outer one given that we had 'blk_insert_bs(blk, bs, errp)' a few lines earlier, and there are no later uses of 'bs' beyond the scope of the 'if (bitmap)' to care if we change the value stored in 'bs' while traveling the backing chain to find a bitmap. So simply get rid of the extra declaration.
Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20190207191357.6665-1-eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
e6798f06 | 28-Jan-2019 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
nbd: generalize usage of nbd_read
We generally do very similar things around nbd_read: error_prepend specifying what we have tried to read, and be_to_cpu conversion of integers.
So, it seems reason
nbd: generalize usage of nbd_read
We generally do very similar things around nbd_read: error_prepend specifying what we have tried to read, and be_to_cpu conversion of integers.
So, it seems reasonable to move common things to helper functions, which: 1. simplify code a bit 2. generalize nbd_read error descriptions, all starting with "Failed to read" 3. make it more difficult to forget to convert things from BE
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20190128165830.165170-1-vsementsov@virtuozzo.com> [eblake: rename macro to DEF_NBD_READ_N and formatting tweaks; checkpatch has false positive complaint] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
7c6f5ddc | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Work around 3.0 bug for listing meta contexts
Commit 3d068aff forgot to advertise available qemu: contexts when the client requests a list with 0 queries. Furthermore, 3.0 shipped with a
nbd/client: Work around 3.0 bug for listing meta contexts
Commit 3d068aff forgot to advertise available qemu: contexts when the client requests a list with 0 queries. Furthermore, 3.0 shipped with a qemu-img hack of x-dirty-bitmap (commit 216ee365) that _silently_ acts as though the entire image is clean if a requested bitmap is not present. Both bugs have been recently fixed, so that a modern qemu server gives full context output right away, and the client refuses a connection if a requested x-dirty-bitmap was not found.
Still, it is likely that there will be users that have to work with a mix of old and new qemu versions, depending on which features get backported where, at which point being able to rely on 'qemu-img --list' output to know for sure whether a given NBD export has the desired dirty bitmap is much nicer than blindly connecting and risking that the entire image may appear clean. We can make our --list code smart enough to work around buggy servers by tracking whether we've seen any qemu: replies in the original 0-query list; if not, repeat with a single query on "qemu:" (which may still have no replies, but then we know for sure we didn't trip up on the server bug).
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-21-eblake@redhat.com>
show more ...
|
0b576b6b | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Add meta contexts to nbd_receive_export_list()
We want to be able to detect whether a given qemu NBD server is exposing the right export(s) and dirty bitmaps, at least for regression tes
nbd/client: Add meta contexts to nbd_receive_export_list()
We want to be able to detect whether a given qemu NBD server is exposing the right export(s) and dirty bitmaps, at least for regression testing. We could use 'nbd-client -l' from the upstream NBD project to list exports, but it's annoying to rely on out-of-tree binaries; furthermore, nbd-client doesn't necessarily know about all of the qemu NBD extensions. Thus, we plan on adding a new mode to qemu-nbd that merely sniffs all possible information from the server during handshake phase, then disconnects and dumps the information.
This patch continues the work of the previous patch, by adding the ability to track the list of available meta contexts into NBDExportInfo. It benefits from the recent refactoring patches with a new nbd_list_meta_contexts() that reuses much of the same framework as setting a meta context.
Note: a malicious server could exhaust memory of a client by feeding an unending loop of contexts; perhaps we could place a limit on how many we are willing to receive. But this is no different from our earlier analysis on a server sending an unending list of exports, and the death of a client due to memory exhaustion when the client was going to exit soon anyways is not really a denial of service attack.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-19-eblake@redhat.com>
show more ...
|
d21a2d34 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Add nbd_receive_export_list()
We want to be able to detect whether a given qemu NBD server is exposing the right export(s) and dirty bitmaps, at least for regression testing. We could u
nbd/client: Add nbd_receive_export_list()
We want to be able to detect whether a given qemu NBD server is exposing the right export(s) and dirty bitmaps, at least for regression testing. We could use 'nbd-client -l' from the upstream NBD project to list exports, but it's annoying to rely on out-of-tree binaries; furthermore, nbd-client doesn't necessarily know about all of the qemu NBD extensions. Thus, we plan on adding a new mode to qemu-nbd that merely sniffs all possible information from the server during handshake phase, then disconnects and dumps the information.
This patch adds the low-level client code for grabbing the list of exports. It benefits from the recent refactoring patches, in order to share as much code as possible when it comes to doing validation of server replies. The resulting information is stored in an array of NBDExportInfo which has been expanded to any description string, along with a convenience function for freeing the list.
Note: a malicious server could exhaust memory of a client by feeding an unending loop of exports; perhaps we should place a limit on how many we are willing to receive. But note that a server could reasonably be serving an export for every file in a large directory, where an arbitrary limit in the client means we can't list anything from such a server; the same happens if we just run until the client fails to malloc() and thus dies by an abort(), where the limit is no longer arbitrary but determined by available memory. Since the client is already planning on being short-lived, it's hard to call this a denial of service attack that would starve off other uses, so it does not appear to be a security issue.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Message-Id: <20190117193658.16413-18-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
138796d0 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO
Rename the function to nbd_opt_info_or_go() with an added parameter and slight changes to comments and trace messages, in order to reuse the
nbd/client: Refactor nbd_opt_go() to support NBD_OPT_INFO
Rename the function to nbd_opt_info_or_go() with an added parameter and slight changes to comments and trace messages, in order to reuse the function for NBD_OPT_INFO.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20190117193658.16413-17-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
b3c9d33b | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Pull out oldstyle size determination
Another refactoring creating nbd_negotiate_finish_oldstyle() for further reuse during 'qemu-nbd --list'.
Signed-off-by: Eric Blake <eblake@redhat.co
nbd/client: Pull out oldstyle size determination
Another refactoring creating nbd_negotiate_finish_oldstyle() for further reuse during 'qemu-nbd --list'.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Message-Id: <20190117193658.16413-16-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
10b89988 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Split handshake into two functions
An upcoming patch will add the ability for qemu-nbd to list the services provided by an NBD server. Share the common code of the TLS handshake by spli
nbd/client: Split handshake into two functions
An upcoming patch will add the ability for qemu-nbd to list the services provided by an NBD server. Share the common code of the TLS handshake by splitting the initial exchange into a separate function, leaving only the export handling in the original function. Functionally, there should be no change in behavior in this patch, although some of the code motion may be difficult to follow due to indentation changes (view with 'git diff -w' for a smaller changeset).
I considered an enum for the return code coordinating state between the two functions, but in the end just settled with ample comments.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-15-eblake@redhat.com>
show more ...
|
2b8d0954 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Refactor return of nbd_receive_negotiate()
The function could only ever return 0 or -EINVAL; make this clearer by dropping a useless 'fail:' label.
Signed-off-by: Eric Blake <eblake@red
nbd/client: Refactor return of nbd_receive_negotiate()
The function could only ever return 0 or -EINVAL; make this clearer by dropping a useless 'fail:' label.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-14-eblake@redhat.com>
show more ...
|
0182c1ae | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Split out nbd_receive_one_meta_context()
Extract portions of nbd_negotiate_simple_meta_context() to a new function nbd_receive_one_meta_context() that copies the pattern of nbd_receive_l
nbd/client: Split out nbd_receive_one_meta_context()
Extract portions of nbd_negotiate_simple_meta_context() to a new function nbd_receive_one_meta_context() that copies the pattern of nbd_receive_list() for performing the argument validation of one reply. The error message when the server replies with more than one context changes slightly, but that shouldn't happen in the common case.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-13-eblake@redhat.com>
show more ...
|
757b3ab9 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Split out nbd_send_meta_query()
Refactor nbd_negotiate_simple_meta_context() to pull out the code that can be reused to send a LIST request for 0 or 1 query. No semantic change. The old
nbd/client: Split out nbd_send_meta_query()
Refactor nbd_negotiate_simple_meta_context() to pull out the code that can be reused to send a LIST request for 0 or 1 query. No semantic change. The old comment about 'sizeof(uint32_t)' being equivalent to '/* number of queries */' is no longer needed, now that we are computing 'sizeof(queries)' instead.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Message-Id: <20190117193658.16413-12-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
2df94eb5 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Change signature of nbd_negotiate_simple_meta_context()
Pass 'info' instead of three separate parameters related to info, when requesting the server to set the meta context. Update the
nbd/client: Change signature of nbd_negotiate_simple_meta_context()
Pass 'info' instead of three separate parameters related to info, when requesting the server to set the meta context. Update the NBDExportInfo struct to rename the received id field to match the fact that we are currently overloading the field to match whatever context the user supplied through the x-dirty-bitmap hack, as well as adding a TODO comment to remind future patches about a desire to request two contexts at once.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-11-eblake@redhat.com>
show more ...
|
6dc1667d | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Move export name into NBDExportInfo
Refactor the 'name' parameter of nbd_receive_negotiate() from being a separate parameter into being part of the in-out 'info'. This also spills over t
nbd/client: Move export name into NBDExportInfo
Refactor the 'name' parameter of nbd_receive_negotiate() from being a separate parameter into being part of the in-out 'info'. This also spills over to a simplification of nbd_opt_go().
The main driver for this refactoring is that an upcoming patch would like to add support to qemu-nbd to list information about all exports available on a server, where the name(s) will be provided by the server instead of the client. But another benefit is that we can now allow the client to explicitly specify the empty export name "" even when connecting to an oldstyle server (even if qemu is no longer such a server after commit 7f7dfe2a).
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-10-eblake@redhat.com>
show more ...
|
091d0bf3 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/client: Refactor nbd_receive_list()
Right now, nbd_receive_list() is only called by nbd_receive_query_exports(), which in turn is only called if the server lacks NBD_OPT_GO but has working optio
nbd/client: Refactor nbd_receive_list()
Right now, nbd_receive_list() is only called by nbd_receive_query_exports(), which in turn is only called if the server lacks NBD_OPT_GO but has working option negotiation, and is merely used as a quality-of-implementation trick since servers can't give decent errors for NBD_OPT_EXPORT_NAME. However, servers that lack NBD_OPT_GO are becoming increasingly rare (nbdkit was a latecomer, in Aug 2018, but qemu has been such a server since commit f37708f6 in July 2017 and released in 2.10), so it no longer makes sense to micro-optimize that function for performance.
Furthermore, when debugging a server's implementation, tracing the full reply (both names and descriptions) is useful, not to mention that upcoming patches adding 'qemu-nbd --list' will want to collect that data. And when you consider that a server can send an export name up to the NBD protocol length limit of 4k; but our current NBD_MAX_NAME_SIZE is only 256, we can't trace all valid server names without more storage, but 4k is large enough that the heap is better than the stack for long names.
Thus, I'm changing the division of labor, with nbd_receive_list() now always malloc'ing a result on success (the malloc is bounded by the fact that we reject servers with a reply length larger than 32M), and moving the comparison to 'wantname' to the caller.
There is a minor change in behavior where a server with 0 exports (an immediate NBD_REP_ACK reply) is now no longer distinguished from a server without LIST support (NBD_REP_ERR_UNSUP); this information could be preserved with a complication to the calling contract to provide a bit more information, but I didn't see the point. After all, the worst that can happen if our guess at a match is wrong is that the caller will get a cryptic disconnect when NBD_OPT_EXPORT_NAME fails (which is no different from what would happen if we had not tried LIST), while treating an empty list as immediate failure would prevent connecting to really old servers that really did lack LIST. Besides, NBD servers with 0 exports are rare (qemu can do it when using QMP nbd-server-start without nbd-server-add - but qemu understands NBD_OPT_GO and thus won't tickle this change in behavior).
Fix the spelling of foundExport to match coding standards while in the area.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190117193658.16413-9-eblake@redhat.com>
show more ...
|
9d26dfcb | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/server: Favor [u]int64_t over off_t
Although our compile-time environment is set up so that we always support long files with 64-bit off_t, we have no guarantee whether off_t is the same type as
nbd/server: Favor [u]int64_t over off_t
Although our compile-time environment is set up so that we always support long files with 64-bit off_t, we have no guarantee whether off_t is the same type as int64_t. This requires casts when printing values, and prevents us from directly using qemu_strtoi64() (which will be done in the next patch). Let's just flip to uint64_t where possible, and stick to int64_t for detecting failure of blk_getlength(); we also keep the assertions added in the previous patch that the resulting values fit in 63 bits. The overflow check in nbd_co_receive_request() was already sane (request->from is validated to fit in 63 bits, and request->len is 32 bits, so the addition can't overflow 64 bits), but rewrite it in a form easier to recognize as a typical overflow check.
Rename the variable 'description' to keep line lengths reasonable.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20190117193658.16413-7-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
7596bbb3 | 17-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd/server: Hoist length check to qmp_nbd_server_add
We only had two callers to nbd_export_new; qemu-nbd.c always passed a valid offset/length pair (because it already checked the file length, to en
nbd/server: Hoist length check to qmp_nbd_server_add
We only had two callers to nbd_export_new; qemu-nbd.c always passed a valid offset/length pair (because it already checked the file length, to ensure that offset was in bounds), while blockdev-nbd.c always passed 0/-1. Then nbd_export_new reduces the size to a multiple of BDRV_SECTOR_SIZE (can only happen when offset is not sector-aligned, since bdrv_getlength() currently rounds up) (someday, it would be nice to have byte-accurate lengths - but not today).
However, I'm finding it easier to work with the code if we are consistent on having both callers pass in a valid length, and just assert that things are sane in nbd_export_new, meaning that no negative values were passed, and that offset+size does not exceed 63 bits (as that really is a fundamental limit to later operations, whether we use off_t or uint64_t).
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20190117193658.16413-6-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
76d570dc | 15-Jan-2019 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
dirty-bitmap: improve bdrv_dirty_bitmap_next_zero
Add bytes parameter to the function, to limit searched range.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
678ba275 | 11-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd: Merge nbd_export_bitmap into nbd_export_new
We only have one caller that wants to export a bitmap name, which it does right after creation of the export. But there is still a brief window of ti
nbd: Merge nbd_export_bitmap into nbd_export_new
We only have one caller that wants to export a bitmap name, which it does right after creation of the export. But there is still a brief window of time where an NBD client could see the export but not the dirty bitmap, which a robust client would have to interpret as meaning the entire image should be treated as dirty. Better is to eliminate the window entirely, by inlining nbd_export_bitmap() into nbd_export_new(), and refusing to create the bitmap in the first place if the requested bitmap can't be located.
We also no longer need logic for setting a different bitmap name compared to the bitmap being exported.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190111194720.15671-8-eblake@redhat.com>
show more ...
|
3fa4c765 | 11-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd: Merge nbd_export_set_name into nbd_export_new
The existing NBD code had a weird split where nbd_export_new() created an export but did not add it to the list of exported names until a later nbd
nbd: Merge nbd_export_set_name into nbd_export_new
The existing NBD code had a weird split where nbd_export_new() created an export but did not add it to the list of exported names until a later nbd_export_set_name() came along and grabbed a second reference on the object; later, the first call to nbd_export_close() drops the second reference while removing the export from the list. This is in part because the QAPI NbdServerRemoveNode enum documents the possibility of adding a mode where we could do a soft disconnect: preventing new clients, but waiting for existing clients to gracefully quit, based on the mode used when calling nbd_export_close().
But in spite of all that, note that we never change the name of an NBD export while it is exposed, which means it is easier to just inline the process of setting the name as part of creating the export.
Inline the contents of nbd_export_set_name() and nbd_export_set_description() into the two points in an export lifecycle where they matter, then adjust both callers to pass the name up front. Note that for creation, all callers pass a non-NULL name, (passing NULL at creation was for old style servers, but we removed support for that in commit 7f7dfe2a), so we can add an assert and do things unconditionally; but for cleanup, because of the dual nature of nbd_export_close(), we still have to be careful to avoid use-after-free. Along the way, add a comment reminding ourselves of the potential of adding a middle mode disconnect.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20190111194720.15671-5-eblake@redhat.com>
show more ...
|
702aa50d | 11-Jan-2019 |
Eric Blake <eblake@redhat.com> |
nbd: Only require disabled bitmap for read-only exports
Our initial implementation of x-nbd-server-add-bitmap put in a restriction because of incremental backups: in that usage, we are exporting one
nbd: Only require disabled bitmap for read-only exports
Our initial implementation of x-nbd-server-add-bitmap put in a restriction because of incremental backups: in that usage, we are exporting one qcow2 file (the temporary overlay target of a blockdev-backup sync:none job) and a dirty bitmap owned by a second qcow2 file (the source of the blockdev-backup, which is the backing file of the temporary). While both qcow2 files are still writable (the target in order to capture copy-on-write of old contents, and the source in order to track live guest writes in the meantime), the NBD client expects to see constant data, including the dirty bitmap. An enabled bitmap in the source would be modified by guest writes, which is at odds with the NBD export being a read-only constant view, hence the initial code choice of enforcing a disabled bitmap (the intent is that the exposed bitmap was disabled in the same transaction that started the blockdev-backup job, although we don't want to track enough state to actually enforce that).
However, consider the case of a bitmap contained in a read-only node (including when the bitmap is found in a backing layer of the active image). Because the node can't be modified, the bitmap won't change due to writes, regardless of whether it is still enabled. Forbidding the export unless the bitmap is disabled is awkward, paritcularly since we can't change the bitmap to be disabled (because the node is read-only).
Alternatively, consider the case of live storage migration, where management directs the destination to create a writable NBD server, then performs a drive-mirror from the source to the target, prior to doing the rest of the live migration. Since storage migration can be time-consuming, it may be wise to let the destination include a dirty bitmap to track which portions it has already received, where even if the migration is interrupted and restarted, the source can query the destination block status in order to potentially minimize re-sending data that has not changed in the meantime on a second attempt. Such code has not been written, and might not be trivial (after all, a cluster being marked dirty in the bitmap does not necessarily guarantee it has the desired contents), but it makes sense that letting an active dirty bitmap be exposed and changing alongside writes may prove useful in the future.
Solve both issues by gating the restriction against a disabled bitmap to only happen when the caller has requested a read-only export, and where the BDS that owns the bitmap (whether or not it is the BDS handed to nbd_export_new() or from its backing chain) is still writable. We could drop the check altogether (if management apps are prepared to deal with a changing bitmap even on a read-only image), but for now keeping a check for the read-only case still stands a chance of preventing management errors.
Update iotest 223 to show the looser behavior by leaving a bitmap enabled the whole run; note that we have to tear down and re-export a node when handling an error.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20190111194720.15671-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
ef2e35fc | 15-Dec-2018 |
Eric Blake <eblake@redhat.com> |
nbd/client: Drop pointless buf variable
There's no need to read into a temporary buffer (oversized since commit 7d3123e1) followed by a byteswap into a uint64_t to check for a magic number via memcm
nbd/client: Drop pointless buf variable
There's no need to read into a temporary buffer (oversized since commit 7d3123e1) followed by a byteswap into a uint64_t to check for a magic number via memcmp(), when the code immediately below demonstrates reading into the uint64_t then byteswapping in place and checking for a magic number via integer math. What's more, having a different error message when the server's first reply byte is 0 is unusual - it's no different from any other wrong magic number, and we already detected short reads. That whole strlen() issue has been present and useless since commit 1d45f8b5 in 2010; perhaps it was leftover debugging (since the correct magic number happens to be ASCII)? Make the error messages more consistent and detailed while touching things.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181215135324.152629-9-eblake@redhat.com>
show more ...
|
3c1fa35d | 15-Dec-2018 |
Eric Blake <eblake@redhat.com> |
qemu-nbd: Fail earlier for -c/-d on non-linux
Connecting to a /dev/nbdN device is a Linux-specific action. We were already masking -c and -d from 'qemu-nbd --help' on non-linux. However, while -d f
qemu-nbd: Fail earlier for -c/-d on non-linux
Connecting to a /dev/nbdN device is a Linux-specific action. We were already masking -c and -d from 'qemu-nbd --help' on non-linux. However, while -d fails with a sensible error message, it took hunting through a couple of files to prove that. What's more, the code for -c doesn't fail until after it has created a pthread and tried to open a device - possibly even printing an error message with %m on a non-Linux platform in spite of the comment that %m is glibc-specific. Make the failure happen sooner, then get rid of stubs that are no longer needed because of the early exits.
While at it: tweak the blank newlines in --help output to be consistent, whether or not built on Linux.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181215135324.152629-7-eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|
6c5c0351 | 15-Dec-2018 |
Eric Blake <eblake@redhat.com> |
nbd/client: More consistent error messages
Consolidate on using decimal (not hex), on outputting the option reply name (not just value), and a consistent comma between clauses, when the client repor
nbd/client: More consistent error messages
Consolidate on using decimal (not hex), on outputting the option reply name (not just value), and a consistent comma between clauses, when the client reports protocol discrepancies from the server. While it won't affect normal operation, it makes debugging additions easier.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20181215135324.152629-6-eblake@redhat.com>
show more ...
|
bee21ef0 | 18-Dec-2018 |
Eric Blake <eblake@redhat.com> |
nbd/client: Trace all server option error messages
Not all servers send free-form text alongside option error replies, but for servers that do (such as qemu), we pass the server's message as a hint
nbd/client: Trace all server option error messages
Not all servers send free-form text alongside option error replies, but for servers that do (such as qemu), we pass the server's message as a hint alongside our own error reporting. However, it would also be useful to trace such server messages, since we can't guarantee how the hint may be consumed.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <20181218225714.284495-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
show more ...
|