Revision tags: v8.2.3, v7.2.11, v9.0.0, v9.0.0-rc4, v9.0.0-rc3, v9.0.0-rc2 |
|
#
2d9cbbea |
| 28-Mar-2024 |
Philippe Mathieu-Daudé <philmd@linaro.org> |
block/gluster: Remove deprecated RDMA protocol handling
GlusterFS+RDMA has been deprecated 8 years ago in commit 0552ff2465 ("block/gluster: deprecate rdma support"):
gluster volfile server fetch
block/gluster: Remove deprecated RDMA protocol handling
GlusterFS+RDMA has been deprecated 8 years ago in commit 0552ff2465 ("block/gluster: deprecate rdma support"):
gluster volfile server fetch happens through unix and/or tcp, it doesn't support volfile fetch over rdma. The rdma code may actually mislead, so to make sure things do not break, for now we fallback to tcp when requested for rdma, with a warning.
If you are wondering how this worked all these days, its the gluster libgfapi code which handles anything other than unix transport as socket/tcp, sad but true.
Besides, the whole RDMA subsystem was deprecated in commit e9a54265f5 ("hw/rdma: Deprecate the pvrdma device and the rdma subsystem") released in v8.2.
Cc: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-Id: <20240328130255.52257-4-philmd@linaro.org>
show more ...
|
Revision tags: v9.0.0-rc1, v9.0.0-rc0, v8.2.2, v7.2.10, v8.2.1, v8.1.5, v7.2.9, v8.1.4, v7.2.8, v8.2.0, v8.2.0-rc4, v8.2.0-rc3, v8.2.0-rc2, v8.2.0-rc1, v7.2.7, v8.1.3, v8.2.0-rc0, v8.1.2 |
|
#
018f9dea |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_apply_auto_read_only() need to hold a reader lock for the graph be
block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_apply_auto_read_only() need to hold a reader lock for the graph because it calls bdrv_can_set_read_only(), which indirectly accesses the parents list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-19-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v9.0.0-rc1, v9.0.0-rc0, v8.2.2, v7.2.10, v8.2.1, v8.1.5, v7.2.9, v8.1.4, v7.2.8, v8.2.0, v8.2.0-rc4, v8.2.0-rc3, v8.2.0-rc2, v8.2.0-rc1, v7.2.7, v8.1.3, v8.2.0-rc0, v8.1.2 |
|
#
018f9dea |
| 29-Sep-2023 |
Kevin Wolf <kwolf@redhat.com> |
block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_apply_auto_read_only() need to hold a reader lock for the graph be
block: Mark bdrv_apply_auto_read_only() and callers GRAPH_RDLOCK
This adds GRAPH_RDLOCK annotations to declare that callers of bdrv_apply_auto_read_only() need to hold a reader lock for the graph because it calls bdrv_can_set_read_only(), which indirectly accesses the parents list of a node.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20230929145157.45443-19-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v8.1.1, v7.2.6, v8.0.5, v8.1.0, v8.1.0-rc4, v8.1.0-rc3, v7.2.5, v8.0.4, v8.1.0-rc2, v8.1.0-rc1, v8.1.0-rc0, v8.0.3, v7.2.4, v8.0.2, v8.0.1, v7.2.3 |
|
#
bd1386cc |
| 22-May-2023 |
Eric Blake <eblake@redhat.com> |
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permi
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permit leading '-'. Adjust the signature of parse_uint() and parse_uint_full() to be like all of qemu_strto*(): put the result parameter last, use the same types (uint64_t and unsigned long long have the same width, but are not always the same type), and mark endptr const (this latter change only affects the rare caller of parse_uint). Adjust all callers in the tree.
While at it, note that since cutils.c already includes:
QEMU_BUILD_BUG_ON(sizeof(int64_t) != sizeof(long long));
we are guaranteed that the result of parse_uint* cannot exceed UINT64_MAX (or the build would have failed), so we can drop pre-existing dead comparisons in opts-visitor.c that were never false.
Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-8-eblake@redhat.com> [eblake: Drop dead code spotted by Markus] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
Revision tags: v8.1.1, v7.2.6, v8.0.5, v8.1.0, v8.1.0-rc4, v8.1.0-rc3, v7.2.5, v8.0.4, v8.1.0-rc2, v8.1.0-rc1, v8.1.0-rc0, v8.0.3, v7.2.4, v8.0.2, v8.0.1, v7.2.3 |
|
#
bd1386cc |
| 22-May-2023 |
Eric Blake <eblake@redhat.com> |
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permi
cutils: Adjust signature of parse_uint[_full]
It's already confusing that we have two very similar functions for wrapping the parse of a 64-bit unsigned value, differing mainly on whether they permit leading '-'. Adjust the signature of parse_uint() and parse_uint_full() to be like all of qemu_strto*(): put the result parameter last, use the same types (uint64_t and unsigned long long have the same width, but are not always the same type), and mark endptr const (this latter change only affects the rare caller of parse_uint). Adjust all callers in the tree.
While at it, note that since cutils.c already includes:
QEMU_BUILD_BUG_ON(sizeof(int64_t) != sizeof(long long));
we are guaranteed that the result of parse_uint* cannot exceed UINT64_MAX (or the build would have failed), so we can drop pre-existing dead comparisons in opts-visitor.c that were never false.
Reviewed-by: Hanna Czenczek <hreitz@redhat.com> Message-Id: <20230522190441.64278-8-eblake@redhat.com> [eblake: Drop dead code spotted by Markus] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
Revision tags: v7.2.2, v8.0.0, v8.0.0-rc4, v8.0.0-rc3, v7.2.1, v8.0.0-rc2, v8.0.0-rc1, v8.0.0-rc0 |
|
#
82618d7b |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a gra
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since it traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-10-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c86422c5 |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since the callback traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Because now this function creates a new coroutine and polls, we need to take the AioContext lock where it is missing, for the only reason that internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls in bdrv_open_driver, because this function has so many callers in so many context that it can easily lead to deadlocks. Therefore the new rule for bdrv_open_driver is that the caller must always hold the AioContext lock of the given bs (except if it is a coroutine), because the function calls bdrv_refresh_total_sectors() which is now a co_wrapper.
Once the rwlock is ultimated and placed in every place it needs to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext lock.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-7-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.2.2, v8.0.0, v8.0.0-rc4, v8.0.0-rc3, v7.2.1, v8.0.0-rc2, v8.0.0-rc1, v8.0.0-rc0 |
|
#
82618d7b |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a gra
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since it traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-10-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c86422c5 |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since the callback traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Because now this function creates a new coroutine and polls, we need to take the AioContext lock where it is missing, for the only reason that internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls in bdrv_open_driver, because this function has so many callers in so many context that it can easily lead to deadlocks. Therefore the new rule for bdrv_open_driver is that the caller must always hold the AioContext lock of the given bs (except if it is a coroutine), because the function calls bdrv_refresh_total_sectors() which is now a co_wrapper.
Once the rwlock is ultimated and placed in every place it needs to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext lock.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-7-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.2.2, v8.0.0, v8.0.0-rc4, v8.0.0-rc3, v7.2.1, v8.0.0-rc2, v8.0.0-rc1, v8.0.0-rc0 |
|
#
82618d7b |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a gra
block: Convert bdrv_get_allocated_file_size() to co_wrapper
bdrv_get_allocated_file_size() is categorized as an I/O function, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since it traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-10-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
c86422c5 |
| 13-Jan-2023 |
Emanuele Giuseppe Esposito <eesposit@redhat.com> |
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph
block: Convert bdrv_refresh_total_sectors() to co_wrapper_mixed
BlockDriver->bdrv_getlength is categorized as IO callback, and it currently doesn't run in a coroutine. We should let it take a graph rdlock since the callback traverses the block nodes graph, which however is only possible in a coroutine.
Therefore turn it into a co_wrapper to move the actual function into a coroutine where the lock can be taken.
Because now this function creates a new coroutine and polls, we need to take the AioContext lock where it is missing, for the only reason that internally co_wrapper calls AIO_WAIT_WHILE and it expects to release the AioContext lock.
This is especially messy when a co_wrapper creates a coroutine and polls in bdrv_open_driver, because this function has so many callers in so many context that it can easily lead to deadlocks. Therefore the new rule for bdrv_open_driver is that the caller must always hold the AioContext lock of the given bs (except if it is a coroutine), because the function calls bdrv_refresh_total_sectors() which is now a co_wrapper.
Once the rwlock is ultimated and placed in every place it needs to be, we will poll using AIO_WAIT_WHILE_UNLOCKED and remove the AioContext lock.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230113204212.359076-7-kwolf@redhat.com> Reviewed-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
#
e2c1c34f |
| 21-Dec-2022 |
Markus Armbruster <armbru@redhat.com> |
include/block: Untangle inclusion loops
We have two inclusion loops:
block/block.h -> block/block-global-state.h -> block/block-common.h -> block/blockjob.h -> block/block.h
include/block: Untangle inclusion loops
We have two inclusion loops:
block/block.h -> block/block-global-state.h -> block/block-common.h -> block/blockjob.h -> block/block.h
block/block.h -> block/block-io.h -> block/block-common.h -> block/blockjob.h -> block/block.h
I believe these go back to Emanuele's reorganization of the block API, merged a few months ago in commit d7e2fe4aac8.
Fortunately, breaking them is merely a matter of deleting unnecessary includes from headers, and adding them back in places where they are now missing.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221221133551.3967339-2-armbru@redhat.com>
show more ...
|
#
e2c1c34f |
| 21-Dec-2022 |
Markus Armbruster <armbru@redhat.com> |
include/block: Untangle inclusion loops
We have two inclusion loops:
block/block.h -> block/block-global-state.h -> block/block-common.h -> block/blockjob.h -> block/block.h
include/block: Untangle inclusion loops
We have two inclusion loops:
block/block.h -> block/block-global-state.h -> block/block-common.h -> block/blockjob.h -> block/block.h
block/block.h -> block/block-io.h -> block/block-common.h -> block/blockjob.h -> block/block.h
I believe these go back to Emanuele's reorganization of the block API, merged a few months ago in commit d7e2fe4aac8.
Fortunately, breaking them is merely a matter of deleting unnecessary includes from headers, and adding them back in places where they are now missing.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221221133551.3967339-2-armbru@redhat.com>
show more ...
|
Revision tags: v7.2.0, v7.2.0-rc4, v7.2.0-rc3, v7.2.0-rc2, v7.2.0-rc1, v7.2.0-rc0 |
|
#
54fde4ff |
| 04-Nov-2022 |
Markus Armbruster <armbru@redhat.com> |
qapi block: Elide redundant has_FOO in generated C
The has_FOO for pointer-valued FOO are redundant, except for arrays. They are also a nuisance to work with. Recent commit "qapi: Start to elide re
qapi block: Elide redundant has_FOO in generated C
The has_FOO for pointer-valued FOO are redundant, except for arrays. They are also a nuisance to work with. Recent commit "qapi: Start to elide redundant has_FOO in generated C" provided the means to elide them step by step. This is the step for qapi/block*.json.
Said commit explains the transformation in more detail.
There is one instance of the invariant violation mentioned there: qcow2_signal_corruption() passes false, "" when node_name is an empty string. Take care to pass NULL then.
The previous two commits cleaned up two more.
Additionally, helper bdrv_latency_histogram_stats() loses its output parameters and returns a value instead.
Cc: Kevin Wolf <kwolf@redhat.com> Cc: Hanna Reitz <hreitz@redhat.com> Cc: qemu-block@nongnu.org Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20221104160712.3005652-11-armbru@redhat.com> [Fixes for #ifndef LIBRBD_SUPPORTS_ENCRYPTION and MacOS squashed in]
show more ...
|
#
e8b65355 |
| 13-Oct-2022 |
Stefan Hajnoczi <stefanha@redhat.com> |
block: add BDRV_REQ_REGISTERED_BUF request flag
Block drivers may optimize I/O requests accessing buffers previously registered with bdrv_register_buf(). Checking whether all elements of a request's
block: add BDRV_REQ_REGISTERED_BUF request flag
Block drivers may optimize I/O requests accessing buffers previously registered with bdrv_register_buf(). Checking whether all elements of a request's QEMUIOVector are within previously registered buffers is expensive, so we need a hint from the user to avoid costly checks.
Add a BDRV_REQ_REGISTERED_BUF request flag to indicate that all QEMUIOVector elements in an I/O request are known to be within previously registered buffers.
Always pass the flag through to driver read/write functions. There is little harm in passing the flag to a driver that does not use it. Passing the flag to drivers avoids changes across many block drivers. Filter drivers would need to explicitly support the flag and pass through to their children when the children support it. That's a lot of code changes and it's hard to remember to do that everywhere, leading to silent reduced performance when the flag is accidentally dropped.
The only problematic scenario with the approach in this patch is when a driver passes the flag through to internal I/O requests that don't use the same I/O buffer. In that case the hint may be set when it should actually be clear. This is a rare case though so the risk is low.
Some drivers have assert(!flags), which no longer works when BDRV_REQ_REGISTERED_BUF is passed in. These assertions aren't very useful anyway since the functions are called almost exclusively by bdrv_driver_preadv/pwritev() so if we get flags handling right there then the assertion is not needed.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20221013185908.1297568-7-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
show more ...
|
#
e8b65355 |
| 13-Oct-2022 |
Stefan Hajnoczi <stefanha@redhat.com> |
block: add BDRV_REQ_REGISTERED_BUF request flag
Block drivers may optimize I/O requests accessing buffers previously registered with bdrv_register_buf(). Checking whether all elements of a request's
block: add BDRV_REQ_REGISTERED_BUF request flag
Block drivers may optimize I/O requests accessing buffers previously registered with bdrv_register_buf(). Checking whether all elements of a request's QEMUIOVector are within previously registered buffers is expensive, so we need a hint from the user to avoid costly checks.
Add a BDRV_REQ_REGISTERED_BUF request flag to indicate that all QEMUIOVector elements in an I/O request are known to be within previously registered buffers.
Always pass the flag through to driver read/write functions. There is little harm in passing the flag to a driver that does not use it. Passing the flag to drivers avoids changes across many block drivers. Filter drivers would need to explicitly support the flag and pass through to their children when the children support it. That's a lot of code changes and it's hard to remember to do that everywhere, leading to silent reduced performance when the flag is accidentally dropped.
The only problematic scenario with the approach in this patch is when a driver passes the flag through to internal I/O requests that don't use the same I/O buffer. In that case the hint may be set when it should actually be clear. This is a rare case though so the risk is low.
Some drivers have assert(!flags), which no longer works when BDRV_REQ_REGISTERED_BUF is passed in. These assertions aren't very useful anyway since the functions are called almost exclusively by bdrv_driver_preadv/pwritev() so if we get flags handling right there then the assertion is not needed.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-id: 20221013185908.1297568-7-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
show more ...
|
Revision tags: v7.1.0, v7.1.0-rc4, v7.1.0-rc3 |
|
#
9a891a91 |
| 11-Aug-2022 |
Stefan Hajnoczi <stefanha@redhat.com> |
gluster: stop using .bdrv_needs_filename
The gluster protocol driver used to parse URIs (filenames) but was extended with a richer JSON syntax in commit 6c7189bb29de ("block/gluster: add support for
gluster: stop using .bdrv_needs_filename
The gluster protocol driver used to parse URIs (filenames) but was extended with a richer JSON syntax in commit 6c7189bb29de ("block/gluster: add support for multiple gluster servers"). The gluster drivers that have JSON parsing set .bdrv_needs_filename to false.
The gluster+unix and gluster+rdma drivers still to require a filename even though the JSON parser is equipped to parse the same volume/path/sockaddr details as the URI parser. Let's allow JSON parsing for these drivers too.
Note that the gluster+rdma driver actually uses TCP because RDMA support is not available, so the JSON server.type field must be "inet".
Drop .bdrv_needs_filename since both the filename and the JSON parsers can handle gluster+unix and gluster+rdma. This change is in preparation for eventually removing .bdrv_needs_filename across the entire codebase.
Cc: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220811164905.430834-1-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.1.0, v7.1.0-rc4, v7.1.0-rc3 |
|
#
9a891a91 |
| 11-Aug-2022 |
Stefan Hajnoczi <stefanha@redhat.com> |
gluster: stop using .bdrv_needs_filename
The gluster protocol driver used to parse URIs (filenames) but was extended with a richer JSON syntax in commit 6c7189bb29de ("block/gluster: add support for
gluster: stop using .bdrv_needs_filename
The gluster protocol driver used to parse URIs (filenames) but was extended with a richer JSON syntax in commit 6c7189bb29de ("block/gluster: add support for multiple gluster servers"). The gluster drivers that have JSON parsing set .bdrv_needs_filename to false.
The gluster+unix and gluster+rdma drivers still to require a filename even though the JSON parser is equipped to parse the same volume/path/sockaddr details as the URI parser. Let's allow JSON parsing for these drivers too.
Note that the gluster+rdma driver actually uses TCP because RDMA support is not available, so the JSON server.type field must be "inet".
Drop .bdrv_needs_filename since both the filename and the JSON parsers can handle gluster+unix and gluster+rdma. This change is in preparation for eventually removing .bdrv_needs_filename across the entire codebase.
Cc: Prasanna Kumar Kalever <prasanna.kalever@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20220811164905.430834-1-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.1.0-rc2, v7.1.0-rc1, v7.1.0-rc0 |
|
#
9b38fc56 |
| 20-May-2022 |
Fabian Ebner <f.ebner@proxmox.com> |
block/gluster: correctly set max_pdiscard
On 64-bit platforms, assigning SIZE_MAX to the int64_t max_pdiscard results in a negative value, and the following assertion would trigger down the line (it
block/gluster: correctly set max_pdiscard
On 64-bit platforms, assigning SIZE_MAX to the int64_t max_pdiscard results in a negative value, and the following assertion would trigger down the line (it's not the same max_pdiscard, but computed from the other one): qemu-system-x86_64: ../block/io.c:3166: bdrv_co_pdiscard: Assertion `max_pdiscard >= bs->bl.request_alignment' failed.
On 32-bit platforms, it's fine to keep using SIZE_MAX.
The assertion in qemu_gluster_co_pdiscard() is checking that the value of 'bytes' can safely be passed to glfs_discard_async(), which takes a size_t for the argument in question, so it is kept as is. And since max_pdiscard is still <= SIZE_MAX, relying on max_pdiscard is still fine.
Fixes: 0c8022876f ("block: use int64_t instead of int in driver discard handlers") Cc: qemu-stable@nongnu.org Signed-off-by: Fabian Ebner <f.ebner@proxmox.com> Message-Id: <20220520075922.43972-1-f.ebner@proxmox.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.1.0-rc2, v7.1.0-rc1, v7.1.0-rc0 |
|
#
9b38fc56 |
| 20-May-2022 |
Fabian Ebner <f.ebner@proxmox.com> |
block/gluster: correctly set max_pdiscard
On 64-bit platforms, assigning SIZE_MAX to the int64_t max_pdiscard results in a negative value, and the following assertion would trigger down the line (it
block/gluster: correctly set max_pdiscard
On 64-bit platforms, assigning SIZE_MAX to the int64_t max_pdiscard results in a negative value, and the following assertion would trigger down the line (it's not the same max_pdiscard, but computed from the other one): qemu-system-x86_64: ../block/io.c:3166: bdrv_co_pdiscard: Assertion `max_pdiscard >= bs->bl.request_alignment' failed.
On 32-bit platforms, it's fine to keep using SIZE_MAX.
The assertion in qemu_gluster_co_pdiscard() is checking that the value of 'bytes' can safely be passed to glfs_discard_async(), which takes a size_t for the argument in question, so it is kept as is. And since max_pdiscard is still <= SIZE_MAX, relying on max_pdiscard is still fine.
Fixes: 0c8022876f ("block: use int64_t instead of int in driver discard handlers") Cc: qemu-stable@nongnu.org Signed-off-by: Fabian Ebner <f.ebner@proxmox.com> Message-Id: <20220520075922.43972-1-f.ebner@proxmox.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
Revision tags: v7.0.0, v7.0.0-rc4, v7.0.0-rc3, v7.0.0-rc2, v7.0.0-rc1, v7.0.0-rc0, v6.1.1, v6.2.0, v6.2.0-rc4, v6.2.0-rc3, v6.2.0-rc2, v6.2.0-rc1, v6.2.0-rc0, v6.0.1 |
|
#
0c802287 |
| 03-Sep-2021 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
block: use int64_t instead of int in driver discard handlers
We are generally moving to int64_t for both offset and bytes parameters on all io paths.
Main motivation is realization of 64-bit write_
block: use int64_t instead of int in driver discard handlers
We are generally moving to int64_t for both offset and bytes parameters on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and with possibility for signed return type (where negative value means error).
So, convert driver discard handlers bytes parameter to int64_t.
The only caller of all updated function is bdrv_co_pdiscard in block/io.c. It is already prepared to work with 64bit requests, but pass at most max(bs->bl.max_pdiscard, INT_MAX) to the driver.
Let's look at all updated functions:
blkdebug: all calculations are still OK, thanks to bdrv_check_qiov_request(). both rule_check and bdrv_co_pdiscard are 64bit
blklogwrites: pass to blk_loc_writes_co_log which is 64bit
blkreplay, copy-on-read, filter-compress: pass to bdrv_co_pdiscard, OK
copy-before-write: pass to bdrv_co_pdiscard which is 64bit and to cbw_do_copy_before_write which is 64bit
file-posix: one handler calls raw_account_discard() is 64bit and both handlers calls raw_do_pdiscard(). Update raw_do_pdiscard, which pass to RawPosixAIOData::aio_nbytes, which is 64bit (and calls raw_account_discard())
gluster: somehow, third argument of glfs_discard_async is size_t. Let's set max_pdiscard accordingly.
iscsi: iscsi_allocmap_set_invalid is 64bit, !is_byte_request_lun_aligned is 64bit. list.num is uint32_t. Let's clarify max_pdiscard and pdiscard_alignment.
mirror_top: pass to bdrv_mirror_top_do_write() which is 64bit
nbd: protocol limitation. max_pdiscard is alredy set strict enough, keep it as is for now.
nvme: buf.nlb is uint32_t and we do shift. So, add corresponding limits to nvme_refresh_limits().
preallocate: pass to bdrv_co_pdiscard() which is 64bit.
rbd: pass to qemu_rbd_start_co() which is 64bit.
qcow2: calculations are still OK, thanks to bdrv_check_qiov_request(), qcow2_cluster_discard() is 64bit.
raw-format: raw_adjust_offset() is 64bit, bdrv_co_pdiscard too.
throttle: pass to bdrv_co_pdiscard() which is 64bit and to throttle_group_co_io_limits_intercept() which is 64bit as well.
test-block-iothread: bytes argument is unused
Great! Now all drivers are prepared to handle 64bit discard requests, or else have explicit max_pdiscard limits.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210903102807.27127-11-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
#
f34b2bcf |
| 03-Sep-2021 |
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
block: use int64_t instead of int in driver write_zeroes handlers
We are generally moving to int64_t for both offset and bytes parameters on all io paths.
Main motivation is realization of 64-bit w
block: use int64_t instead of int in driver write_zeroes handlers
We are generally moving to int64_t for both offset and bytes parameters on all io paths.
Main motivation is realization of 64-bit write_zeroes operation for fast zeroing large disk chunks, up to the whole disk.
We chose signed type, to be consistent with off_t (which is signed) and with possibility for signed return type (where negative value means error).
So, convert driver write_zeroes handlers bytes parameter to int64_t.
The only caller of all updated function is bdrv_co_do_pwrite_zeroes().
bdrv_co_do_pwrite_zeroes() itself is of course OK with widening of callee parameter type. Also, bdrv_co_do_pwrite_zeroes()'s max_write_zeroes is limited to INT_MAX. So, updated functions all are safe, they will not get "bytes" larger than before.
Still, let's look through all updated functions, and add assertions to the ones which are actually unprepared to values larger than INT_MAX. For these drivers also set explicit max_pwrite_zeroes limit.
Let's go:
blkdebug: calculations can't overflow, thanks to bdrv_check_qiov_request() in generic layer. rule_check() and bdrv_co_pwrite_zeroes() both have 64bit argument.
blklogwrites: pass to blk_log_writes_co_log() with 64bit argument.
blkreplay, copy-on-read, filter-compress: pass to bdrv_co_pwrite_zeroes() which is OK
copy-before-write: Calls cbw_do_copy_before_write() and bdrv_co_pwrite_zeroes, both have 64bit argument.
file-posix: both handler calls raw_do_pwrite_zeroes, which is updated. In raw_do_pwrite_zeroes() calculations are OK due to bdrv_check_qiov_request(), bytes go to RawPosixAIOData::aio_nbytes which is uint64_t. Check also where that uint64_t gets handed: handle_aiocb_write_zeroes_block() passes a uint64_t[2] to ioctl(BLKZEROOUT), handle_aiocb_write_zeroes() calls do_fallocate() which takes off_t (and we compile to always have 64-bit off_t), as does handle_aiocb_write_zeroes_unmap. All look safe.
gluster: bytes go to GlusterAIOCB::size which is int64_t and to glfs_zerofill_async works with off_t.
iscsi: Aha, here we deal with iscsi_writesame16_task() that has uint32_t num_blocks argument and iscsi_writesame16_task() has uint16_t argument. Make comments, add assertions and clarify max_pwrite_zeroes calculation. iscsi_allocmap_() functions already has int64_t argument is_byte_request_lun_aligned is simple to update, do it.
mirror_top: pass to bdrv_mirror_top_do_write which has uint64_t argument
nbd: Aha, here we have protocol limitation, and NBDRequest::len is uint32_t. max_pwrite_zeroes is cleanly set to 32bit value, so we are OK for now.
nvme: Again, protocol limitation. And no inherent limit for write-zeroes at all. But from code that calculates cdw12 it's obvious that we do have limit and alignment. Let's clarify it. Also, obviously the code is not prepared to handle bytes=0. Let's handle this case too. trace events already 64bit
preallocate: pass to handle_write() and bdrv_co_pwrite_zeroes(), both 64bit.
rbd: pass to qemu_rbd_start_co() which is 64bit.
qcow2: offset + bytes and alignment still works good (thanks to bdrv_check_qiov_request()), so tail calculation is OK qcow2_subcluster_zeroize() has 64bit argument, should be OK trace events updated
qed: qed_co_request wants int nb_sectors. Also in code we have size_t used for request length which may be 32bit. So, let's just keep INT_MAX as a limit (aligning it down to pwrite_zeroes_alignment) and don't care.
raw-format: Is OK. raw_adjust_offset and bdrv_co_pwrite_zeroes are both 64bit.
throttle: Both throttle_group_co_io_limits_intercept() and bdrv_co_pwrite_zeroes() are 64bit.
vmdk: pass to vmdk_pwritev which is 64bit
quorum: pass to quorum_co_pwritev() which is 64bit
Hooray!
At this point all block drivers are prepared to support 64bit write-zero requests, or have explicitly set max_pwrite_zeroes.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210903102807.27127-8-vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> [eblake: use <= rather than < in assertions relying on max_pwrite_zeroes] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
Revision tags: v6.1.0, v6.1.0-rc4 |
|
#
72b4cabe |
| 12-Aug-2021 |
Hanna Reitz <hreitz@redhat.com> |
block/gluster: Do not force-cap *pnum
bdrv_co_block_status() does it for us, we do not need to do it here.
The advantage of not capping *pnum is that bdrv_co_block_status() can cache larger data re
block/gluster: Do not force-cap *pnum
bdrv_co_block_status() does it for us, we do not need to do it here.
The advantage of not capping *pnum is that bdrv_co_block_status() can cache larger data regions than requested by its caller.
Signed-off-by: Hanna Reitz <hreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20210812084148.14458-6-hreitz@redhat.com>
show more ...
|
Revision tags: v6.1.0-rc3 |
|
#
e24154d8 |
| 05-Aug-2021 |
Max Reitz <mreitz@redhat.com> |
gluster: Align block-status tail
gluster's block-status implementation is basically a copy of that in block/file-posix.c, there is only one thing missing, and that is aligning trailing data extents
gluster: Align block-status tail
gluster's block-status implementation is basically a copy of that in block/file-posix.c, there is only one thing missing, and that is aligning trailing data extents to the request alignment (as added by commit 9c3db310ff0).
Note that 9c3db310ff0 mentions that "there seems to be no other block driver that sets request_alignment and [...]", but while block/gluster.c does indeed not set request_alignment, block/io.c's bdrv_refresh_limits() will still default to an alignment of 512 because block/gluster.c does not provide a byte-aligned read function. Therefore, unaligned tails can conceivably occur, and so we should apply the change from 9c3db310ff0 to gluster's block-status implementation.
Reported-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Message-Id: <20210805143603.59503-1-mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Hanna Reitz <hreitz@redhat.com>
show more ...
|
Revision tags: v6.1.0-rc2, v6.1.0-rc1, v6.1.0-rc0, v6.0.0, v6.0.0-rc5, v6.0.0-rc4, v6.0.0-rc3, v6.0.0-rc2, v6.0.0-rc1, v6.0.0-rc0 |
|
#
95b3a8c8 |
| 13-Jan-2021 |
Eric Blake <eblake@redhat.com> |
qapi: More complex uses of QAPI_LIST_APPEND
These cases require a bit more thought to review; in each case, the code was appending to a list, but not with a FOOList **tail variable.
Signed-off-by:
qapi: More complex uses of QAPI_LIST_APPEND
These cases require a bit more thought to review; in each case, the code was appending to a list, but not with a FOOList **tail variable.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20210113221013.390592-6-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> [Flawed change to qmp_guest_network_get_interfaces() dropped] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|