Re: [RFC PATCH 11/22] qemu-nbd: Use blk_exp_add() to create the export

[Kevin Wolf <kwolf@redhat.com>]

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

Am 17.08.2020 um 16:27 hat Max Reitz geschrieben:
> On 13.08.20 18:29, Kevin Wolf wrote:
> > With this change, NBD exports are only created through the BlockExport
> > interface any more. This allows us finally to move things from the NBD
> > layer to the BlockExport layer if they make sense for other export
> > types, too.
> 
> I see.
> 
> > blk_exp_add() returns only a weak reference, so the explicit
> > nbd_export_put() goes away.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  include/block/export.h |  2 ++
> >  include/block/nbd.h    |  1 +
> >  block/export/export.c  |  2 +-
> >  blockdev-nbd.c         |  8 +++++++-
> >  qemu-nbd.c             | 28 ++++++++++++++++++++++------
> >  5 files changed, 33 insertions(+), 8 deletions(-)
> 
> [...]
> 
> > diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> > index d5b084acc2..8dd127af52 100644
> > --- a/blockdev-nbd.c
> > +++ b/blockdev-nbd.c
> 
> [...]
> 
> > @@ -176,7 +182,7 @@ BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
> >  
> >      assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
> >  
> > -    if (!nbd_server) {
> > +    if (!nbd_server && !is_qemu_nbd) {
> 
> (This begs the question of how difficult it would be to let qemu-nbd use
> QMP’s nbd-server-start, but I will not ask it, for I fear the answer.)

(It would probably include something along the lines of "patches
welcome". (I initially wanted to do this, but came to the conclusion
that it wasn't for this series when I noticed the socket activation
code and discussed with danpb on IRC how to integrate it in
SocketAddress.))

> >          error_setg(errp, "NBD server not running");
> >          return NULL;
> >      }
> > diff --git a/qemu-nbd.c b/qemu-nbd.c
> > index 48aa8a9d46..d967b8fcb9 100644
> > --- a/qemu-nbd.c
> > +++ b/qemu-nbd.c
> 
> [...]
> 
> > @@ -1050,9 +1050,27 @@ int main(int argc, char **argv)
> >  
> >      bs->detect_zeroes = detect_zeroes;
> >  
> > -    export = nbd_export_new(bs, export_name,
> > -                            export_description, bitmap, readonly, shared > 1,
> > -                            writethrough, &error_fatal);
> > +    nbd_server_is_qemu_nbd(true);
> > +
> > +    export_opts = g_new(BlockExportOptions, 1);
> > +    *export_opts = (BlockExportOptions) {
> > +        .type               = BLOCK_EXPORT_TYPE_NBD,
> > +        .has_writethrough   = true,
> > +        .writethrough       = writethrough,
> > +        .u.nbd = {
> > +            .device             = g_strdup(bdrv_get_node_name(bs)),
> > +            .has_name           = true,
> > +            .name               = g_strdup(export_name),
> > +            .has_description    = !!export_description,
> > +            .description        = g_strdup(export_description),
> > +            .has_writable       = true,
> > +            .writable           = !readonly,
> > +            .has_bitmap         = !!bitmap,
> > +            .bitmap             = g_strdup(bitmap),
> > +        },
> > +    };
> > +    blk_exp_add(export_opts, &error_fatal);
> 
> Why not use the already-global qmp_block_export_add(), if we don’t need
> the return value here?  (Will we require it at some point?)

I can do that. I needed the return value originally, but after some
reshuffling of the series it magically disappeared.

Kevin

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