qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [PATCH v2 3/3] block: remove separate bdrv_file_open callback
Date: Wed, 28 Jun 2023 09:40:45 +0200	[thread overview]
Message-ID: <ZJvj/UP9CZXQ+U2Y@redhat.com> (raw)
In-Reply-To: <20230601115139.196369-4-pbonzini@redhat.com>

Am 01.06.2023 um 13:51 hat Paolo Bonzini geschrieben:
> 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>

> diff --git a/block/blkio.c b/block/blkio.c
> index 72117fa0059b..202cf20ca4bb 100644
> --- a/block/blkio.c
> +++ b/block/blkio.c
> @@ -992,7 +992,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, \

You changed the function name here, but didn't actually rename the
function. I don't think this can build.

> 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,

If we do want to rename, then we should do it in all block drivers. As
far as I can tell, this would be null, nvme and ssh. Not nfs, because
all related function start with nfs_file_* there.

Kevin



  reply	other threads:[~2023-06-28  7:41 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-01 11:51 [PATCH v2 0/3] block: remove separate bdrv_file_open callback Paolo Bonzini
2023-06-01 11:51 ` [PATCH v2 1/3] block: make assertion more generic Paolo Bonzini
2023-06-01 11:51 ` [PATCH v2 2/3] block: do not check bdrv_file_open Paolo Bonzini
2023-06-01 11:51 ` [PATCH v2 3/3] block: remove separate bdrv_file_open callback Paolo Bonzini
2023-06-28  7:40   ` Kevin Wolf [this message]
2023-06-01 13:42 ` [PATCH v2 0/3] " Eric Blake

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZJvj/UP9CZXQ+U2Y@redhat.com \
    --to=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).