* [PATCH 0/3] block: remove separate bdrv_file_open callback
@ 2022-12-12 13:16 Paolo Bonzini
2022-12-12 13:16 ` [PATCH 1/3] block: apply assertion more widely Paolo Bonzini
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Paolo Bonzini @ 2022-12-12 13:16 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-block, kwolf
The presence of the bdrv_file_open callback is used in some parts of the
code to distinguish protocol and format drivers. Use the existing
.protocol_name field instead, and unify .bdrv_open with .bdrv_file_open.
Paolo
Paolo Bonzini (3):
block: apply assertion more widely
block: do not check bdrv_file_open
block: remove separate bdrv_file_open callback
block.c | 17 +++++++----------
block/blkdebug.c | 2 +-
block/blkio.c | 2 +-
block/blkverify.c | 2 +-
block/curl.c | 8 ++++----
block/file-posix.c | 8 ++++----
block/file-win32.c | 4 ++--
block/gluster.c | 8 ++++----
block/iscsi.c | 4 ++--
block/nbd.c | 6 +++---
block/nfs.c | 2 +-
block/null.c | 4 ++--
block/nvme.c | 2 +-
block/rbd.c | 3 ++-
block/ssh.c | 2 +-
block/vvfat.c | 2 +-
include/block/block_int-common.h | 3 ---
17 files changed, 37 insertions(+), 42 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 1/3] block: apply assertion more widely 2022-12-12 13:16 [PATCH 0/3] block: remove separate bdrv_file_open callback Paolo Bonzini @ 2022-12-12 13:16 ` Paolo Bonzini 2022-12-12 13:16 ` [PATCH 2/3] block: do not check bdrv_file_open Paolo Bonzini 2022-12-12 13:17 ` [PATCH 3/3] block: remove separate bdrv_file_open callback Paolo Bonzini 2 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2022-12-12 13:16 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-block, kwolf .bdrv_needs_filename is only set for drivers that also set bdrv_file_open, i.e. protocol drivers. So we can make the assertion always, it will always pass for those drivers that use bdrv_open. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block.c b/block.c index 676bbe0529b0..0a625a489a6e 100644 --- a/block.c +++ b/block.c @@ -1617,8 +1617,8 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, bs->drv = drv; bs->opaque = g_malloc0(drv->instance_size); + assert(!drv->bdrv_needs_filename || bs->filename[0]); if (drv->bdrv_file_open) { - assert(!drv->bdrv_needs_filename || bs->filename[0]); ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); } else if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); -- 2.38.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] block: do not check bdrv_file_open 2022-12-12 13:16 [PATCH 0/3] block: remove separate bdrv_file_open callback Paolo Bonzini 2022-12-12 13:16 ` [PATCH 1/3] block: apply assertion more widely Paolo Bonzini @ 2022-12-12 13:16 ` Paolo Bonzini 2023-01-19 13:17 ` Kevin Wolf 2022-12-12 13:17 ` [PATCH 3/3] block: remove separate bdrv_file_open callback Paolo Bonzini 2 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2022-12-12 13:16 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-block, kwolf The set of BlockDrivers that have .bdrv_file_open coincides with those that have .protocol_name and guess what---checking drv->bdrv_file_open is done to see if the driver is a protocol. So check drv->protocol_name instead. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/block.c b/block.c index 0a625a489a6e..7a66cc2ea23a 100644 --- a/block.c +++ b/block.c @@ -911,7 +911,6 @@ BlockDriver *bdrv_find_protocol(const char *filename, int i; GLOBAL_STATE_CODE(); - /* TODO Drivers without bdrv_file_open must be specified explicitly */ /* * XXX(hch): we really should not let host device detection @@ -1618,7 +1617,7 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, bs->opaque = g_malloc0(drv->instance_size); assert(!drv->bdrv_needs_filename || bs->filename[0]); - if (drv->bdrv_file_open) { + if (drv->bdrv_open) { ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); } else if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); @@ -1930,7 +1929,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file, open_flags = bdrv_open_flags(bs, bs->open_flags); node_name = qemu_opt_get(opts, "node-name"); - assert(!drv->bdrv_file_open || file == NULL); + assert(!drv->protocol_name || file == NULL); ret = bdrv_open_driver(bs, drv, node_name, options, open_flags, errp); if (ret < 0) { goto fail_opts; @@ -2030,7 +2029,7 @@ static int bdrv_fill_options(QDict **options, const char *filename, } /* If the user has explicitly specified the driver, this choice should * override the BDRV_O_PROTOCOL flag */ - protocol = drv->bdrv_file_open; + protocol = drv->protocol_name; } if (protocol) { @@ -3932,7 +3931,7 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, } /* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */ - assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open); + assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->protocol_name); /* file must be NULL if a protocol BDS is about to be created * (the inverse results in an error message from bdrv_open_common()) */ assert(!(flags & BDRV_O_PROTOCOL) || !file); @@ -5671,7 +5670,7 @@ int64_t bdrv_get_allocated_file_size(BlockDriverState *bs) return drv->bdrv_get_allocated_file_size(bs); } - if (drv->bdrv_file_open) { + if (drv->protocol_name) { /* * Protocol drivers default to -ENOTSUP (most of their data is * not stored in any of their children (if they even have any), @@ -7772,7 +7771,7 @@ void bdrv_refresh_filename(BlockDriverState *bs) * Both of these conditions are represented by generate_json_filename. */ if (primary_child_bs->exact_filename[0] && - primary_child_bs->drv->bdrv_file_open && + primary_child_bs->drv->protocol_name && !drv->is_filter && !generate_json_filename) { strcpy(bs->exact_filename, primary_child_bs->exact_filename); -- 2.38.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] block: do not check bdrv_file_open 2022-12-12 13:16 ` [PATCH 2/3] block: do not check bdrv_file_open Paolo Bonzini @ 2023-01-19 13:17 ` Kevin Wolf 2023-04-21 9:35 ` Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Kevin Wolf @ 2023-01-19 13:17 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel, qemu-block Am 12.12.2022 um 14:16 hat Paolo Bonzini geschrieben: > The set of BlockDrivers that have .bdrv_file_open coincides with those > that have .protocol_name and guess what---checking drv->bdrv_file_open > is done to see if the driver is a protocol. So check drv->protocol_name > instead. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > block.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/block.c b/block.c > index 0a625a489a6e..7a66cc2ea23a 100644 > --- a/block.c > +++ b/block.c > @@ -911,7 +911,6 @@ BlockDriver *bdrv_find_protocol(const char *filename, > int i; > > GLOBAL_STATE_CODE(); > - /* TODO Drivers without bdrv_file_open must be specified explicitly */ > > /* > * XXX(hch): we really should not let host device detection > @@ -1618,7 +1617,7 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, > bs->opaque = g_malloc0(drv->instance_size); > > assert(!drv->bdrv_needs_filename || bs->filename[0]); > - if (drv->bdrv_file_open) { > + if (drv->bdrv_open) { > ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); > } else if (drv->bdrv_open) { > ret = drv->bdrv_open(bs, options, open_flags, &local_err); I suppose you mean drv->protocol_name for the first if condition? The bug will disappear again after patch 3, but this intermediate state is very broken. Kevin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] block: do not check bdrv_file_open 2023-01-19 13:17 ` Kevin Wolf @ 2023-04-21 9:35 ` Paolo Bonzini 0 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2023-04-21 9:35 UTC (permalink / raw) To: Kevin Wolf; +Cc: qemu-devel, qemu-block On Thu, Jan 19, 2023 at 2:17 PM Kevin Wolf <kwolf@redhat.com> wrote: > > assert(!drv->bdrv_needs_filename || bs->filename[0]); > > - if (drv->bdrv_file_open) { > > + if (drv->bdrv_open) { > > ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); > > } else if (drv->bdrv_open) { > > ret = drv->bdrv_open(bs, options, open_flags, &local_err); > > I suppose you mean drv->protocol_name for the first if condition? > > The bug will disappear again after patch 3, but this intermediate state > is very broken. Yep, I split the patch wrong. Will resend after you merge block-next. Paolo ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] block: remove separate bdrv_file_open callback 2022-12-12 13:16 [PATCH 0/3] block: remove separate bdrv_file_open callback Paolo Bonzini 2022-12-12 13:16 ` [PATCH 1/3] block: apply assertion more widely Paolo Bonzini 2022-12-12 13:16 ` [PATCH 2/3] block: do not check bdrv_file_open Paolo Bonzini @ 2022-12-12 13:17 ` Paolo Bonzini 2 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2022-12-12 13:17 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-block, kwolf bdrv_file_open and bdrv_open are completely equivalent, they are never checked except to see which one to invoke. So merge them into a single callback. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 2 -- block/blkdebug.c | 2 +- block/blkio.c | 2 +- block/blkverify.c | 2 +- block/curl.c | 8 ++++---- block/file-posix.c | 8 ++++---- block/file-win32.c | 4 ++-- block/gluster.c | 8 ++++---- block/iscsi.c | 4 ++-- block/nbd.c | 6 +++--- block/nfs.c | 2 +- block/null.c | 4 ++-- block/nvme.c | 2 +- block/rbd.c | 3 ++- block/ssh.c | 2 +- block/vvfat.c | 2 +- include/block/block_int-common.h | 3 --- 17 files changed, 30 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index 7a66cc2ea23a..48efbf94106e 100644 --- a/block.c +++ b/block.c @@ -1618,8 +1618,6 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, assert(!drv->bdrv_needs_filename || bs->filename[0]); if (drv->bdrv_open) { - ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); - } else if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); } else { ret = 0; diff --git a/block/blkdebug.c b/block/blkdebug.c index 4265ca125e25..a4445a352876 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -1071,7 +1071,7 @@ static BlockDriver bdrv_blkdebug = { .is_filter = true, .bdrv_parse_filename = blkdebug_parse_filename, - .bdrv_file_open = blkdebug_open, + .bdrv_open = blkdebug_open, .bdrv_close = blkdebug_close, .bdrv_reopen_prepare = blkdebug_reopen_prepare, .bdrv_child_perm = blkdebug_child_perm, diff --git a/block/blkio.c b/block/blkio.c index 5eae3adfaf66..45ae5780152c 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -994,7 +994,7 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) .format_name = name, \ .protocol_name = name, \ .instance_size = sizeof(BDRVBlkioState), \ - .bdrv_file_open = blkio_file_open, \ + .bdrv_open = blkio_open, \ .bdrv_close = blkio_close, \ .bdrv_getlength = blkio_getlength, \ .bdrv_co_truncate = blkio_truncate, \ diff --git a/block/blkverify.c b/block/blkverify.c index c60a2dc624dc..e5b5d5c4a426 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -310,7 +310,7 @@ static BlockDriver bdrv_blkverify = { .instance_size = sizeof(BDRVBlkverifyState), .bdrv_parse_filename = blkverify_parse_filename, - .bdrv_file_open = blkverify_open, + .bdrv_open = blkverify_open, .bdrv_close = blkverify_close, .bdrv_child_perm = bdrv_default_perms, .bdrv_getlength = blkverify_getlength, diff --git a/block/curl.c b/block/curl.c index cba4c4cac768..398b28163e00 100644 --- a/block/curl.c +++ b/block/curl.c @@ -999,7 +999,7 @@ static BlockDriver bdrv_http = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_getlength = curl_getlength, @@ -1018,7 +1018,7 @@ static BlockDriver bdrv_https = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_getlength = curl_getlength, @@ -1037,7 +1037,7 @@ static BlockDriver bdrv_ftp = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_getlength = curl_getlength, @@ -1056,7 +1056,7 @@ static BlockDriver bdrv_ftps = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_getlength = curl_getlength, diff --git a/block/file-posix.c b/block/file-posix.c index b9647c5ffc26..91c4a2d74687 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -3301,7 +3301,7 @@ BlockDriver bdrv_file = { .bdrv_needs_filename = true, .bdrv_probe = NULL, /* no probe for protocols */ .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, @@ -3675,7 +3675,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_probe_device = hdev_probe_device, .bdrv_parse_filename = hdev_parse_filename, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -3803,7 +3803,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -3934,7 +3934,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, diff --git a/block/file-win32.c b/block/file-win32.c index ec9d64d0e4e2..4207e521f22a 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -748,7 +748,7 @@ BlockDriver bdrv_file = { .instance_size = sizeof(BDRVRawState), .bdrv_needs_filename = true, .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_refresh_limits = raw_probe_alignment, .bdrv_close = raw_close, .bdrv_co_create_opts = raw_co_create_opts, @@ -921,7 +921,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_parse_filename = hdev_parse_filename, .bdrv_probe_device = hdev_probe_device, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_refresh_limits = hdev_refresh_limits, diff --git a/block/gluster.c b/block/gluster.c index 7c90f7ba4b80..0ab196b35229 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1554,7 +1554,7 @@ static BlockDriver bdrv_gluster = { .format_name = "gluster", .protocol_name = "gluster", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1583,7 +1583,7 @@ static BlockDriver bdrv_gluster_tcp = { .format_name = "gluster", .protocol_name = "gluster+tcp", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1612,7 +1612,7 @@ static BlockDriver bdrv_gluster_unix = { .format_name = "gluster", .protocol_name = "gluster+unix", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1647,7 +1647,7 @@ static BlockDriver bdrv_gluster_rdma = { .format_name = "gluster", .protocol_name = "gluster+rdma", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, diff --git a/block/iscsi.c b/block/iscsi.c index a316d46d9625..dc6cbe0f74a5 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2425,7 +2425,7 @@ static BlockDriver bdrv_iscsi = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, @@ -2464,7 +2464,7 @@ static BlockDriver bdrv_iser = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, diff --git a/block/nbd.c b/block/nbd.c index 7d485c86d285..a332372a5380 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -2114,7 +2114,7 @@ static BlockDriver bdrv_nbd = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2142,7 +2142,7 @@ static BlockDriver bdrv_nbd_tcp = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2170,7 +2170,7 @@ static BlockDriver bdrv_nbd_unix = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, diff --git a/block/nfs.c b/block/nfs.c index ece22353ac53..126e3898f966 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -888,7 +888,7 @@ static BlockDriver bdrv_nfs = { #endif .bdrv_co_truncate = nfs_file_co_truncate, - .bdrv_file_open = nfs_file_open, + .bdrv_open = nfs_file_open, .bdrv_close = nfs_file_close, .bdrv_co_create = nfs_file_co_create, .bdrv_co_create_opts = nfs_file_co_create_opts, diff --git a/block/null.c b/block/null.c index 75f7d0db40c4..f3a427b6c69a 100644 --- a/block/null.c +++ b/block/null.c @@ -281,7 +281,7 @@ static BlockDriver bdrv_null_co = { .protocol_name = "null-co", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_co_parse_filename, .bdrv_getlength = null_getlength, .bdrv_get_allocated_file_size = null_allocated_file_size, @@ -302,7 +302,7 @@ static BlockDriver bdrv_null_aio = { .protocol_name = "null-aio", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_aio_parse_filename, .bdrv_getlength = null_getlength, .bdrv_get_allocated_file_size = null_allocated_file_size, diff --git a/block/nvme.c b/block/nvme.c index 656624c585a2..5083b36b89bc 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -1640,7 +1640,7 @@ static BlockDriver bdrv_nvme = { .create_opts = &bdrv_create_opts_simple, .bdrv_parse_filename = nvme_parse_filename, - .bdrv_file_open = nvme_file_open, + .bdrv_open = nvme_file_open, .bdrv_close = nvme_close, .bdrv_getlength = nvme_getlength, .bdrv_probe_blocksizes = nvme_probe_blocksizes, diff --git a/block/rbd.c b/block/rbd.c index f826410f40f9..4c759dd51167 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1648,8 +1648,9 @@ static const char *const qemu_rbd_strong_runtime_opts[] = { static BlockDriver bdrv_rbd = { .format_name = "rbd", .instance_size = sizeof(BDRVRBDState), + .bdrv_parse_filename = qemu_rbd_parse_filename, - .bdrv_file_open = qemu_rbd_open, + .bdrv_open = qemu_rbd_open, .bdrv_close = qemu_rbd_close, .bdrv_reopen_prepare = qemu_rbd_reopen_prepare, .bdrv_co_create = qemu_rbd_co_create, diff --git a/block/ssh.c b/block/ssh.c index 04726d4ecb58..07a99d6b8ed6 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -1356,7 +1356,7 @@ static BlockDriver bdrv_ssh = { .protocol_name = "ssh", .instance_size = sizeof(BDRVSSHState), .bdrv_parse_filename = ssh_parse_filename, - .bdrv_file_open = ssh_file_open, + .bdrv_open = ssh_file_open, .bdrv_co_create = ssh_co_create, .bdrv_co_create_opts = ssh_co_create_opts, .bdrv_close = ssh_close, diff --git a/block/vvfat.c b/block/vvfat.c index 723c91216e0c..88943a99db9d 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3257,7 +3257,7 @@ static BlockDriver bdrv_vvfat = { .instance_size = sizeof(BDRVVVFATState), .bdrv_parse_filename = vvfat_parse_filename, - .bdrv_file_open = vvfat_open, + .bdrv_open = vvfat_open, .bdrv_refresh_limits = vvfat_refresh_limits, .bdrv_close = vvfat_close, .bdrv_child_perm = vvfat_child_perm, diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index a6bc6b7fe9bd..c670f3c8da1c 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -245,9 +245,6 @@ struct BlockDriver { int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags, Error **errp); - /* Protocol drivers should implement this instead of bdrv_open */ - int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags, - Error **errp); void (*bdrv_close)(BlockDriverState *bs); int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts, -- 2.38.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 0/3] block: remove separate bdrv_file_open callback @ 2023-03-09 8:50 Paolo Bonzini 2023-03-09 8:50 ` [PATCH 3/3] " Paolo Bonzini 0 siblings, 1 reply; 7+ messages in thread From: Paolo Bonzini @ 2023-03-09 8:50 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-block The value of the bdrv_file_open is sometimes checked to distinguish protocol and format drivers, but apart from that there is no difference between bdrv_file_open and bdrv_open. However, they can all be distinguished by the non-NULL .protocol_name member. Change the checks to use .protocol_name instead of .bdrv_file_open, and unify the two callbacks. Paolo Paolo Bonzini (3): block: make assertion more generic block: do not check bdrv_file_open block: remove separate bdrv_file_open callback block.c | 17 +++++++---------- block/blkdebug.c | 2 +- block/blkio.c | 2 +- block/blkverify.c | 2 +- block/curl.c | 8 ++++---- block/file-posix.c | 8 ++++---- block/file-win32.c | 4 ++-- block/gluster.c | 8 ++++---- block/iscsi.c | 4 ++-- block/nbd.c | 6 +++--- block/nfs.c | 2 +- block/null.c | 4 ++-- block/nvme.c | 2 +- block/rbd.c | 3 ++- block/ssh.c | 2 +- block/vvfat.c | 2 +- include/block/block_int-common.h | 3 --- 17 files changed, 37 insertions(+), 42 deletions(-) -- 2.39.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] block: remove separate bdrv_file_open callback 2023-03-09 8:50 [PATCH 0/3] " Paolo Bonzini @ 2023-03-09 8:50 ` Paolo Bonzini 0 siblings, 0 replies; 7+ messages in thread From: Paolo Bonzini @ 2023-03-09 8:50 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-block bdrv_file_open and bdrv_open are completely equivalent, they are never checked except to see which one to invoke. So merge them into a single one. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- block.c | 2 -- block/blkdebug.c | 2 +- block/blkio.c | 2 +- block/blkverify.c | 2 +- block/curl.c | 8 ++++---- block/file-posix.c | 8 ++++---- block/file-win32.c | 4 ++-- block/gluster.c | 8 ++++---- block/iscsi.c | 4 ++-- block/nbd.c | 6 +++--- block/nfs.c | 2 +- block/null.c | 4 ++-- block/nvme.c | 2 +- block/rbd.c | 3 ++- block/ssh.c | 2 +- block/vvfat.c | 2 +- include/block/block_int-common.h | 3 --- 17 files changed, 30 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index 71f0ea24870e..e6d20f89ba82 100644 --- a/block.c +++ b/block.c @@ -1628,8 +1628,6 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, assert(!drv->bdrv_needs_filename || bs->filename[0]); if (drv->bdrv_open) { - ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); - } else if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); } else { ret = 0; diff --git a/block/blkdebug.c b/block/blkdebug.c index addad914b3f7..c9ae3cb6ae3d 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -1072,7 +1072,7 @@ static BlockDriver bdrv_blkdebug = { .is_filter = true, .bdrv_parse_filename = blkdebug_parse_filename, - .bdrv_file_open = blkdebug_open, + .bdrv_open = blkdebug_open, .bdrv_close = blkdebug_close, .bdrv_reopen_prepare = blkdebug_reopen_prepare, .bdrv_child_perm = blkdebug_child_perm, diff --git a/block/blkio.c b/block/blkio.c index 0cdc99a72960..c2efe1a8feec 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -997,7 +997,7 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) .format_name = name, \ .protocol_name = name, \ .instance_size = sizeof(BDRVBlkioState), \ - .bdrv_file_open = blkio_file_open, \ + .bdrv_open = blkio_open, \ .bdrv_close = blkio_close, \ .bdrv_co_getlength = blkio_co_getlength, \ .bdrv_co_truncate = blkio_truncate, \ diff --git a/block/blkverify.c b/block/blkverify.c index 1c16f86b2e70..9b59b34c02ba 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -312,7 +312,7 @@ static BlockDriver bdrv_blkverify = { .instance_size = sizeof(BDRVBlkverifyState), .bdrv_parse_filename = blkverify_parse_filename, - .bdrv_file_open = blkverify_open, + .bdrv_open = blkverify_open, .bdrv_close = blkverify_close, .bdrv_child_perm = bdrv_default_perms, .bdrv_co_getlength = blkverify_co_getlength, diff --git a/block/curl.c b/block/curl.c index 8bb39a134e4b..809858692652 100644 --- a/block/curl.c +++ b/block/curl.c @@ -1032,7 +1032,7 @@ static BlockDriver bdrv_http = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1051,7 +1051,7 @@ static BlockDriver bdrv_https = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1070,7 +1070,7 @@ static BlockDriver bdrv_ftp = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1089,7 +1089,7 @@ static BlockDriver bdrv_ftps = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, diff --git a/block/file-posix.c b/block/file-posix.c index 5760cf22d17d..578b512ed515 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -3323,7 +3323,7 @@ BlockDriver bdrv_file = { .bdrv_needs_filename = true, .bdrv_probe = NULL, /* no probe for protocols */ .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, @@ -3697,7 +3697,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_probe_device = hdev_probe_device, .bdrv_parse_filename = hdev_parse_filename, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -3825,7 +3825,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -3955,7 +3955,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, diff --git a/block/file-win32.c b/block/file-win32.c index c7d0b8530637..53f4955e2999 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -748,7 +748,7 @@ BlockDriver bdrv_file = { .instance_size = sizeof(BDRVRawState), .bdrv_needs_filename = true, .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_refresh_limits = raw_probe_alignment, .bdrv_close = raw_close, .bdrv_co_create_opts = raw_co_create_opts, @@ -921,7 +921,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_parse_filename = hdev_parse_filename, .bdrv_probe_device = hdev_probe_device, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_refresh_limits = hdev_refresh_limits, diff --git a/block/gluster.c b/block/gluster.c index 185a83e5e533..6b25aa2516ab 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1553,7 +1553,7 @@ static BlockDriver bdrv_gluster = { .format_name = "gluster", .protocol_name = "gluster", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1582,7 +1582,7 @@ static BlockDriver bdrv_gluster_tcp = { .format_name = "gluster", .protocol_name = "gluster+tcp", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1611,7 +1611,7 @@ static BlockDriver bdrv_gluster_unix = { .format_name = "gluster", .protocol_name = "gluster+unix", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1646,7 +1646,7 @@ static BlockDriver bdrv_gluster_rdma = { .format_name = "gluster", .protocol_name = "gluster+rdma", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, diff --git a/block/iscsi.c b/block/iscsi.c index 9fc0bed90b81..08c1d2cb0d57 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2427,7 +2427,7 @@ static BlockDriver bdrv_iscsi = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, @@ -2466,7 +2466,7 @@ static BlockDriver bdrv_iser = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, diff --git a/block/nbd.c b/block/nbd.c index bf2894ad5c9b..4ff1da684e64 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -2114,7 +2114,7 @@ static BlockDriver bdrv_nbd = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2142,7 +2142,7 @@ static BlockDriver bdrv_nbd_tcp = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2170,7 +2170,7 @@ static BlockDriver bdrv_nbd_unix = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, diff --git a/block/nfs.c b/block/nfs.c index 351dc6ec8d14..4bd43404e53f 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -889,7 +889,7 @@ static BlockDriver bdrv_nfs = { #endif .bdrv_co_truncate = nfs_file_co_truncate, - .bdrv_file_open = nfs_file_open, + .bdrv_open = nfs_file_open, .bdrv_close = nfs_file_close, .bdrv_co_create = nfs_file_co_create, .bdrv_co_create_opts = nfs_file_co_create_opts, diff --git a/block/null.c b/block/null.c index 4808704ffd3a..6fa64d20d865 100644 --- a/block/null.c +++ b/block/null.c @@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = { .protocol_name = "null-co", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_co_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, @@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = { .protocol_name = "null-aio", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_aio_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, diff --git a/block/nvme.c b/block/nvme.c index 5b744c2bdad4..015a1a3cec9e 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -1641,7 +1641,7 @@ static BlockDriver bdrv_nvme = { .create_opts = &bdrv_create_opts_simple, .bdrv_parse_filename = nvme_parse_filename, - .bdrv_file_open = nvme_file_open, + .bdrv_open = nvme_file_open, .bdrv_close = nvme_close, .bdrv_co_getlength = nvme_co_getlength, .bdrv_probe_blocksizes = nvme_probe_blocksizes, diff --git a/block/rbd.c b/block/rbd.c index 978671411ec7..9ea00d82930d 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1811,8 +1811,9 @@ static const char *const qemu_rbd_strong_runtime_opts[] = { static BlockDriver bdrv_rbd = { .format_name = "rbd", .instance_size = sizeof(BDRVRBDState), + .bdrv_parse_filename = qemu_rbd_parse_filename, - .bdrv_file_open = qemu_rbd_open, + .bdrv_open = qemu_rbd_open, .bdrv_close = qemu_rbd_close, .bdrv_reopen_prepare = qemu_rbd_reopen_prepare, .bdrv_co_create = qemu_rbd_co_create, diff --git a/block/ssh.c b/block/ssh.c index b3b3352075ff..a7820ba7fde0 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -1357,7 +1357,7 @@ static BlockDriver bdrv_ssh = { .protocol_name = "ssh", .instance_size = sizeof(BDRVSSHState), .bdrv_parse_filename = ssh_parse_filename, - .bdrv_file_open = ssh_file_open, + .bdrv_open = ssh_file_open, .bdrv_co_create = ssh_co_create, .bdrv_co_create_opts = ssh_co_create_opts, .bdrv_close = ssh_close, diff --git a/block/vvfat.c b/block/vvfat.c index 0ddc91fc096a..e0e28713c433 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3257,7 +3257,7 @@ static BlockDriver bdrv_vvfat = { .instance_size = sizeof(BDRVVVFATState), .bdrv_parse_filename = vvfat_parse_filename, - .bdrv_file_open = vvfat_open, + .bdrv_open = vvfat_open, .bdrv_refresh_limits = vvfat_refresh_limits, .bdrv_close = vvfat_close, .bdrv_child_perm = vvfat_child_perm, diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index d4190173284f..a72d7d2c93a7 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -241,9 +241,6 @@ struct BlockDriver { int (*bdrv_open)(BlockDriverState *bs, QDict *options, int flags, Error **errp); - /* Protocol drivers should implement this instead of bdrv_open */ - int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags, - Error **errp); void (*bdrv_close)(BlockDriverState *bs); int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_create)( -- 2.39.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-04-21 9:38 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-12 13:16 [PATCH 0/3] block: remove separate bdrv_file_open callback Paolo Bonzini 2022-12-12 13:16 ` [PATCH 1/3] block: apply assertion more widely Paolo Bonzini 2022-12-12 13:16 ` [PATCH 2/3] block: do not check bdrv_file_open Paolo Bonzini 2023-01-19 13:17 ` Kevin Wolf 2023-04-21 9:35 ` Paolo Bonzini 2022-12-12 13:17 ` [PATCH 3/3] block: remove separate bdrv_file_open callback Paolo Bonzini -- strict thread matches above, loose matches on Subject: below -- 2023-03-09 8:50 [PATCH 0/3] " Paolo Bonzini 2023-03-09 8:50 ` [PATCH 3/3] " Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).