Re: [PATCH v2 12/15] nbd/server: Support inactive nodes

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

On Fri, Jan 31, 2025 at 10:50:48AM +0100, Kevin Wolf wrote:
> In order to support running an NBD export on inactive nodes, we must
> make sure to return errors for any operations that aren't allowed on
> inactive nodes. Reads are the only operation we know we need for
> inactive images, so to err on the side of caution, return errors for
> everything else, even if some operations could possibly be okay.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  nbd/server.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index f64e47270c..2076fb2666 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -2026,6 +2026,7 @@ static void nbd_export_delete(BlockExport *blk_exp)
>  const BlockExportDriver blk_exp_nbd = {
>      .type               = BLOCK_EXPORT_TYPE_NBD,
>      .instance_size      = sizeof(NBDExport),
> +    .supports_inactive  = true,
>      .create             = nbd_export_create,
>      .delete             = nbd_export_delete,
>      .request_shutdown   = nbd_export_request_shutdown,
> @@ -2920,6 +2921,22 @@ static coroutine_fn int nbd_handle_request(NBDClient *client,
>      NBDExport *exp = client->exp;
>      char *msg;
>      size_t i;
> +    bool inactive;
> +
> +    WITH_GRAPH_RDLOCK_GUARD() {
> +        inactive = bdrv_is_inactive(blk_bs(exp->common.blk));
> +        if (inactive) {
> +            switch (request->type) {
> +            case NBD_CMD_READ:
> +                /* These commands are allowed on inactive nodes */
> +                break;
> +            default:
> +                /* Return an error for the rest */
> +                return nbd_send_generic_reply(client, request, -EPERM,
> +                                              "export is inactive", errp);
> +            }
> +        }
> +    }

Hmm...end of lock guard. What prevents the race where inactive changes
before the request is performed?

>  
>      switch (request->type) {
>      case NBD_CMD_CACHE:
> -- 
> 2.48.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]