Re: [RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-19 Thread Eric Blake

On 8/13/20 11:29 AM, Kevin Wolf wrote:

We want to have a common set of commands for all types of block exports.
Currently, this is only NBD, but we're going to add more types.

This patch adds the basic BlockExport and BlockExportDriver structs and
a QMP command block-export-add that creates a new export based on the
given BlockExportOptions.

qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().

Signed-off-by: Kevin Wolf 
---


Seeing if I can spot anything beyond Max's fine points:


+++ b/qapi/block-export.json
@@ -170,3 +170,12 @@
'nbd': 'BlockExportOptionsNbd'
 } }
  
+##

+# @block-export-add:
+#
+# Creates a new block export.
+#
+# Since: 5.2
+##
+{ 'command': 'block-export-add',
+  'data': 'BlockExportOptions', 'boxed': true }


So if I read patch 3 correctly, the difference between nbd-server-add 
and block-export-add is that the latter includes a "type":"nbd" member. 
(If we ever play with allowing a default discriminator value in QAPI, we 
could declare the default for "type" to be NBD, and then the two would 
be identical - except that since you are adding a new command designed 
to extend to more than just nbd, I don't think keeping nbd as default 
makes sense)



+++ b/block/export/export.c



+void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+{
+BlockExportOptions export = {
+.type = BLOCK_EXPORT_TYPE_NBD,
+.u.nbd = *arg,
+};
+qmp_block_export_add(, errp);
+}


And indeed, this matches that analysis.



@@ -217,6 +220,8 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error 
**errp)
  
   out:

  aio_context_release(aio_context);
+/* TODO Remove the cast: Move to server.c which can access fields of exp */
+return (BlockExport*) exp;


Should this use container_of()?  Ah, the TODO says you want to, but 
can't because exp is an opaque type in this file...



  }
  
  void qmp_nbd_server_remove(const char *name,

diff --git a/nbd/server.c b/nbd/server.c
index bee2ef8bd1..774325dbe5 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -18,6 +18,8 @@
   */
  
  #include "qemu/osdep.h"

+
+#include "block/export.h"
  #include "qapi/error.h"
  #include "qemu/queue.h"
  #include "trace.h"
@@ -80,6 +82,7 @@ struct NBDRequestData {
  };
  
  struct NBDExport {

+BlockExport common;


...but at least the cast is accurate.

Overall, the idea looks sane.  I'm happy if you want to move 
blockdev-nbd.c out of the top level.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




Re: [RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-17 Thread Kevin Wolf
Am 17.08.2020 um 15:19 hat Max Reitz geschrieben:
> On 17.08.20 14:45, Kevin Wolf wrote:
> > Am 17.08.2020 um 12:03 hat Max Reitz geschrieben:
> >> On 13.08.20 18:29, Kevin Wolf wrote:
> >>> We want to have a common set of commands for all types of block exports.
> >>> Currently, this is only NBD, but we're going to add more types.
> >>>
> >>> This patch adds the basic BlockExport and BlockExportDriver structs and
> >>> a QMP command block-export-add that creates a new export based on the
> >>> given BlockExportOptions.
> >>>
> >>> qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
> >>>
> >>> Signed-off-by: Kevin Wolf 
> >>> ---
> >>>  qapi/block-export.json |  9 ++
> >>>  include/block/export.h | 32 +
> >>>  include/block/nbd.h|  3 +-
> >>>  block/export/export.c  | 57 ++
> >>>  blockdev-nbd.c | 19 -
> >>>  nbd/server.c   | 15 +-
> >>>  Makefile.objs  |  6 ++--
> >>>  block/Makefile.objs|  2 ++
> >>>  block/export/Makefile.objs |  1 +
> >>>  9 files changed, 132 insertions(+), 12 deletions(-)
> >>>  create mode 100644 include/block/export.h
> >>>  create mode 100644 block/export/export.c
> >>>  create mode 100644 block/export/Makefile.objs
> >>
> >> Nothing of too great importance below.  But it’s an RFC, so comments I
> >> will give.
> >>
> >>> diff --git a/block/export/export.c b/block/export/export.c
> >>> new file mode 100644
> >>> index 00..3d0dacb3f2
> >>> --- /dev/null
> >>> +++ b/block/export/export.c
> >>> @@ -0,0 +1,57 @@
> >>> +/*
> >>> + * Common block export infrastructure
> >>> + *
> >>> + * Copyright (c) 2012, 2020 Red Hat, Inc.
> >>> + *
> >>> + * Authors:
> >>> + * Paolo Bonzini 
> >>> + * Kevin Wolf 
> >>> + *
> >>> + * This work is licensed under the terms of the GNU GPL, version 2 or
> >>> + * later.  See the COPYING file in the top-level directory.
> >>> + */
> >>> +
> >>> +#include "qemu/osdep.h"
> >>> +
> >>> +#include "block/export.h"
> >>> +#include "block/nbd.h"
> >>> +#include "qapi/error.h"
> >>> +#include "qapi/qapi-commands-block-export.h"
> >>> +
> >>> +static const BlockExportDriver* blk_exp_drivers[] = {
> >>  ^^
> >> Sternenplatzierung *hust*
> >>
> >>> +_exp_nbd,
> >>> +};
> >>
> >> Not sure whether I like this better than the block driver way of
> >> registering block drivers with a constructor.  It requires writing less
> >> code, at the expense of making the variable global.  So I think there’s
> >> no good reason to prefer the block driver approach.
> > 
> > I guess I can see one reason why we may want to switch to the
> > registration style eventually: If we we want to make export drivers
> > optional modules which may or may not be present.
> 
> Good point.
> 
> >> Maybe my hesitance comes from the variable being declared (as extern) in
> >> a header file (block/export.h).  I think I would prefer it if we put
> >> that external reference only here in this file.  Would that work, or do
> >> you have other plans that require blk_exp_nbd to be visible outside of
> >> nbd/server.c and this file here?
> > 
> > Hm, do we have precedence for "public, but not really" variables?
> > Normally I expect public symbols to be declared in a header file.
> 
> Hm, yes.
> 
> tl;dr: I was wrong about a local external reference being nicer.  But I
> believe there is a difference between externally-facing header files
> (e.g. block.h) and internal header files (e.g. block_int.h).  I don’t
> know which of those block/export.h is supposed to be.
> 
> (And of course it doesn’t even matter at all, really.)
> 
> 
> non-tl;dr:
> 
> We have a similar case for bdrv_{file,raw,qcow2}, but those are at least
> in a *_int.h.  I can’t say I like that style.
> 
> OK, let me try to figure out what my problem with this is.
> 
> I think if a module (in this case the NBD export code) exports
> something, it should be available in the respective header (i.e., some
> NBD header), not in some other header.  A module’s header should present
> what it exports to the rest of the code.  The export code probably
> doesn’t want to export the NBD driver object, it wants to import it,
> actually.  So if it should be in a header file, it should be in an NBD
> header.
> 
> Now none of our block drivers has a header file for exporting symbols to
> the rest of the block code, which is why their symbols have been put
> into block_int.h.  I think that’s cutting corners, but can be defended
> by saying that block_int.h is not for exporting anything, but just
> collects stuff internal to the block layer, so it kind of fits there.
> 
> (Still, technically, I believe bdrv_{file,raw,qcow2} should be exported
> by each respective block driver in a driver-specific header file.  If
> that isn’t the case, it doesn’t really matter to me whether it’s put
> into a dedicated header file to collect internal stuff (block_int.h) 

Re: [RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-17 Thread Max Reitz
On 17.08.20 14:45, Kevin Wolf wrote:
> Am 17.08.2020 um 12:03 hat Max Reitz geschrieben:
>> On 13.08.20 18:29, Kevin Wolf wrote:
>>> We want to have a common set of commands for all types of block exports.
>>> Currently, this is only NBD, but we're going to add more types.
>>>
>>> This patch adds the basic BlockExport and BlockExportDriver structs and
>>> a QMP command block-export-add that creates a new export based on the
>>> given BlockExportOptions.
>>>
>>> qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
>>>
>>> Signed-off-by: Kevin Wolf 
>>> ---
>>>  qapi/block-export.json |  9 ++
>>>  include/block/export.h | 32 +
>>>  include/block/nbd.h|  3 +-
>>>  block/export/export.c  | 57 ++
>>>  blockdev-nbd.c | 19 -
>>>  nbd/server.c   | 15 +-
>>>  Makefile.objs  |  6 ++--
>>>  block/Makefile.objs|  2 ++
>>>  block/export/Makefile.objs |  1 +
>>>  9 files changed, 132 insertions(+), 12 deletions(-)
>>>  create mode 100644 include/block/export.h
>>>  create mode 100644 block/export/export.c
>>>  create mode 100644 block/export/Makefile.objs
>>
>> Nothing of too great importance below.  But it’s an RFC, so comments I
>> will give.
>>
>>> diff --git a/block/export/export.c b/block/export/export.c
>>> new file mode 100644
>>> index 00..3d0dacb3f2
>>> --- /dev/null
>>> +++ b/block/export/export.c
>>> @@ -0,0 +1,57 @@
>>> +/*
>>> + * Common block export infrastructure
>>> + *
>>> + * Copyright (c) 2012, 2020 Red Hat, Inc.
>>> + *
>>> + * Authors:
>>> + * Paolo Bonzini 
>>> + * Kevin Wolf 
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or
>>> + * later.  See the COPYING file in the top-level directory.
>>> + */
>>> +
>>> +#include "qemu/osdep.h"
>>> +
>>> +#include "block/export.h"
>>> +#include "block/nbd.h"
>>> +#include "qapi/error.h"
>>> +#include "qapi/qapi-commands-block-export.h"
>>> +
>>> +static const BlockExportDriver* blk_exp_drivers[] = {
>>  ^^
>> Sternenplatzierung *hust*
>>
>>> +_exp_nbd,
>>> +};
>>
>> Not sure whether I like this better than the block driver way of
>> registering block drivers with a constructor.  It requires writing less
>> code, at the expense of making the variable global.  So I think there’s
>> no good reason to prefer the block driver approach.
> 
> I guess I can see one reason why we may want to switch to the
> registration style eventually: If we we want to make export drivers
> optional modules which may or may not be present.

Good point.

>> Maybe my hesitance comes from the variable being declared (as extern) in
>> a header file (block/export.h).  I think I would prefer it if we put
>> that external reference only here in this file.  Would that work, or do
>> you have other plans that require blk_exp_nbd to be visible outside of
>> nbd/server.c and this file here?
> 
> Hm, do we have precedence for "public, but not really" variables?
> Normally I expect public symbols to be declared in a header file.

Hm, yes.

tl;dr: I was wrong about a local external reference being nicer.  But I
believe there is a difference between externally-facing header files
(e.g. block.h) and internal header files (e.g. block_int.h).  I don’t
know which of those block/export.h is supposed to be.

(And of course it doesn’t even matter at all, really.)


non-tl;dr:

We have a similar case for bdrv_{file,raw,qcow2}, but those are at least
in a *_int.h.  I can’t say I like that style.

OK, let me try to figure out what my problem with this is.

I think if a module (in this case the NBD export code) exports
something, it should be available in the respective header (i.e., some
NBD header), not in some other header.  A module’s header should present
what it exports to the rest of the code.  The export code probably
doesn’t want to export the NBD driver object, it wants to import it,
actually.  So if it should be in a header file, it should be in an NBD
header.

Now none of our block drivers has a header file for exporting symbols to
the rest of the block code, which is why their symbols have been put
into block_int.h.  I think that’s cutting corners, but can be defended
by saying that block_int.h is not for exporting anything, but just
collects stuff internal to the block layer, so it kind of fits there.

(Still, technically, I believe bdrv_{file,raw,qcow2} should be exported
by each respective block driver in a driver-specific header file.  If
that isn’t the case, it doesn’t really matter to me whether it’s put
into a dedicated header file to collect internal stuff (block_int.h) or
just imported locally (with an external declaration) where it’s used.
Probably the dedicated header file is cleaner after all, right.)

Maybe block/export.h is the same in that it’s just supposed to collect
symbols used internally by the export code, then it isn’t wrong to put
it 

Re: [RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-17 Thread Kevin Wolf
Am 17.08.2020 um 12:03 hat Max Reitz geschrieben:
> On 13.08.20 18:29, Kevin Wolf wrote:
> > We want to have a common set of commands for all types of block exports.
> > Currently, this is only NBD, but we're going to add more types.
> > 
> > This patch adds the basic BlockExport and BlockExportDriver structs and
> > a QMP command block-export-add that creates a new export based on the
> > given BlockExportOptions.
> > 
> > qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
> > 
> > Signed-off-by: Kevin Wolf 
> > ---
> >  qapi/block-export.json |  9 ++
> >  include/block/export.h | 32 +
> >  include/block/nbd.h|  3 +-
> >  block/export/export.c  | 57 ++
> >  blockdev-nbd.c | 19 -
> >  nbd/server.c   | 15 +-
> >  Makefile.objs  |  6 ++--
> >  block/Makefile.objs|  2 ++
> >  block/export/Makefile.objs |  1 +
> >  9 files changed, 132 insertions(+), 12 deletions(-)
> >  create mode 100644 include/block/export.h
> >  create mode 100644 block/export/export.c
> >  create mode 100644 block/export/Makefile.objs
> 
> Nothing of too great importance below.  But it’s an RFC, so comments I
> will give.
> 
> > diff --git a/block/export/export.c b/block/export/export.c
> > new file mode 100644
> > index 00..3d0dacb3f2
> > --- /dev/null
> > +++ b/block/export/export.c
> > @@ -0,0 +1,57 @@
> > +/*
> > + * Common block export infrastructure
> > + *
> > + * Copyright (c) 2012, 2020 Red Hat, Inc.
> > + *
> > + * Authors:
> > + * Paolo Bonzini 
> > + * Kevin Wolf 
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > + * later.  See the COPYING file in the top-level directory.
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "block/export.h"
> > +#include "block/nbd.h"
> > +#include "qapi/error.h"
> > +#include "qapi/qapi-commands-block-export.h"
> > +
> > +static const BlockExportDriver* blk_exp_drivers[] = {
>  ^^
> Sternenplatzierung *hust*
> 
> > +_exp_nbd,
> > +};
> 
> Not sure whether I like this better than the block driver way of
> registering block drivers with a constructor.  It requires writing less
> code, at the expense of making the variable global.  So I think there’s
> no good reason to prefer the block driver approach.

I guess I can see one reason why we may want to switch to the
registration style eventually: If we we want to make export drivers
optional modules which may or may not be present.

> Maybe my hesitance comes from the variable being declared (as extern) in
> a header file (block/export.h).  I think I would prefer it if we put
> that external reference only here in this file.  Would that work, or do
> you have other plans that require blk_exp_nbd to be visible outside of
> nbd/server.c and this file here?

Hm, do we have precedence for "public, but not really" variables?
Normally I expect public symbols to be declared in a header file.

> > +static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
> > +{
> > +int i;
> > +
> > +for (i = 0; i < ARRAY_SIZE(blk_exp_drivers); i++) {
> > +if (blk_exp_drivers[i]->type == type) {
> > +return blk_exp_drivers[i];
> > +}
> > +}
> 
> How bad would it be to define blk_exp_drivers as
> blk_exp_drivers[BLOCK_EXPORT_TYPE__MAX] and use the BlockExportType as
> the driver index so we don’t have to loop here?
> 
> Not that it matters performance-wise.  Just something I wondered.

Might be nicer indeed. It would be incompatible with a registration
model, though, so if we're not sure yet what we want to have in the long
term, maybe the more neutral way is to leave it as it is.

> > +return NULL;
> 
> Why not e.g. g_assert_not_reached()?
> 
> (If the BlockExportType were used as the index, I’d assert that
> type < ARRAY_SIZE(blk_exp_drivers) && blk_exp_drivers[type] != NULL.  I
> don’t think there’s a reason for graceful handling.)

Same thing actually. This works as long as all drivers are always
present.

Now I understand that the current state is somewhat inconsistent in that
it uses a simple array of things that are always present, but has
functions that work as if it were dynamic. I don't mind this
inconsistency very much, but if you do, I guess I could implement a
registration type thing right away.

Kevin


signature.asc
Description: PGP signature


Re: [RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-17 Thread Max Reitz
On 13.08.20 18:29, Kevin Wolf wrote:
> We want to have a common set of commands for all types of block exports.
> Currently, this is only NBD, but we're going to add more types.
> 
> This patch adds the basic BlockExport and BlockExportDriver structs and
> a QMP command block-export-add that creates a new export based on the
> given BlockExportOptions.
> 
> qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().
> 
> Signed-off-by: Kevin Wolf 
> ---
>  qapi/block-export.json |  9 ++
>  include/block/export.h | 32 +
>  include/block/nbd.h|  3 +-
>  block/export/export.c  | 57 ++
>  blockdev-nbd.c | 19 -
>  nbd/server.c   | 15 +-
>  Makefile.objs  |  6 ++--
>  block/Makefile.objs|  2 ++
>  block/export/Makefile.objs |  1 +
>  9 files changed, 132 insertions(+), 12 deletions(-)
>  create mode 100644 include/block/export.h
>  create mode 100644 block/export/export.c
>  create mode 100644 block/export/Makefile.objs

Nothing of too great importance below.  But it’s an RFC, so comments I
will give.

> diff --git a/block/export/export.c b/block/export/export.c
> new file mode 100644
> index 00..3d0dacb3f2
> --- /dev/null
> +++ b/block/export/export.c
> @@ -0,0 +1,57 @@
> +/*
> + * Common block export infrastructure
> + *
> + * Copyright (c) 2012, 2020 Red Hat, Inc.
> + *
> + * Authors:
> + * Paolo Bonzini 
> + * Kevin Wolf 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or
> + * later.  See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "block/export.h"
> +#include "block/nbd.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-block-export.h"
> +
> +static const BlockExportDriver* blk_exp_drivers[] = {
 ^^
Sternenplatzierung *hust*

> +_exp_nbd,
> +};

Not sure whether I like this better than the block driver way of
registering block drivers with a constructor.  It requires writing less
code, at the expense of making the variable global.  So I think there’s
no good reason to prefer the block driver approach.

Maybe my hesitance comes from the variable being declared (as extern) in
a header file (block/export.h).  I think I would prefer it if we put
that external reference only here in this file.  Would that work, or do
you have other plans that require blk_exp_nbd to be visible outside of
nbd/server.c and this file here?

> +static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
> +{
> +int i;
> +
> +for (i = 0; i < ARRAY_SIZE(blk_exp_drivers); i++) {
> +if (blk_exp_drivers[i]->type == type) {
> +return blk_exp_drivers[i];
> +}
> +}

How bad would it be to define blk_exp_drivers as
blk_exp_drivers[BLOCK_EXPORT_TYPE__MAX] and use the BlockExportType as
the driver index so we don’t have to loop here?

Not that it matters performance-wise.  Just something I wondered.

> +return NULL;

Why not e.g. g_assert_not_reached()?

(If the BlockExportType were used as the index, I’d assert that
type < ARRAY_SIZE(blk_exp_drivers) && blk_exp_drivers[type] != NULL.  I
don’t think there’s a reason for graceful handling.)

> +}

[...]

> +void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
> +{
> +BlockExportOptions export = {
> +.type = BLOCK_EXPORT_TYPE_NBD,
> +.u.nbd = *arg,
> +};
> +qmp_block_export_add(, errp);
> +}

Hm.  I think I’d’ve kept this in blockdev-nbd.c, actually, but, well.
It’ll stay a one-off.

> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 98ee1b6170..a1dc11bdd7 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c

[...]

> @@ -217,6 +220,8 @@ void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error 
> **errp)
>  
>   out:
>  aio_context_release(aio_context);
> +/* TODO Remove the cast: Move to server.c which can access fields of exp 
> */
> +return (BlockExport*) exp;

*hust*

(But if it’s moved soon anyway so we can use >common, then whatever.)

Max



signature.asc
Description: OpenPGP digital signature


[RFC PATCH 04/22] block/export: Add BlockExport infrastructure and block-export-add

2020-08-13 Thread Kevin Wolf
We want to have a common set of commands for all types of block exports.
Currently, this is only NBD, but we're going to add more types.

This patch adds the basic BlockExport and BlockExportDriver structs and
a QMP command block-export-add that creates a new export based on the
given BlockExportOptions.

qmp_nbd_server_add() becomes a wrapper around qmp_block_export_add().

Signed-off-by: Kevin Wolf 
---
 qapi/block-export.json |  9 ++
 include/block/export.h | 32 +
 include/block/nbd.h|  3 +-
 block/export/export.c  | 57 ++
 blockdev-nbd.c | 19 -
 nbd/server.c   | 15 +-
 Makefile.objs  |  6 ++--
 block/Makefile.objs|  2 ++
 block/export/Makefile.objs |  1 +
 9 files changed, 132 insertions(+), 12 deletions(-)
 create mode 100644 include/block/export.h
 create mode 100644 block/export/export.c
 create mode 100644 block/export/Makefile.objs

diff --git a/qapi/block-export.json b/qapi/block-export.json
index 9332076a05..40369814b4 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -170,3 +170,12 @@
   'nbd': 'BlockExportOptionsNbd'
} }
 
+##
+# @block-export-add:
+#
+# Creates a new block export.
+#
+# Since: 5.2
+##
+{ 'command': 'block-export-add',
+  'data': 'BlockExportOptions', 'boxed': true }
diff --git a/include/block/export.h b/include/block/export.h
new file mode 100644
index 00..b1d7325403
--- /dev/null
+++ b/include/block/export.h
@@ -0,0 +1,32 @@
+/*
+ * Declarations for block exports
+ *
+ * Copyright (c) 2012, 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Paolo Bonzini 
+ * Kevin Wolf 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#ifndef BLOCK_EXPORT_H
+#define BLOCK_EXPORT_H
+
+#include "qapi/qapi-types-block-export.h"
+
+typedef struct BlockExport BlockExport;
+
+typedef struct BlockExportDriver {
+BlockExportType type;
+BlockExport *(*create)(BlockExportOptions *, Error **);
+} BlockExportDriver;
+
+struct BlockExport {
+const BlockExportDriver *drv;
+};
+
+extern const BlockExportDriver blk_exp_nbd;
+
+#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 262f6da2ce..c8c5cb6b61 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -20,7 +20,7 @@
 #ifndef NBD_H
 #define NBD_H
 
-#include "qapi/qapi-types-block-export.h"
+#include "block/export.h"
 #include "io/channel-socket.h"
 #include "crypto/tlscreds.h"
 #include "qapi/error.h"
@@ -328,6 +328,7 @@ int nbd_errno_to_system_errno(int err);
 typedef struct NBDExport NBDExport;
 typedef struct NBDClient NBDClient;
 
+BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp);
 NBDExport *nbd_export_new(BlockDriverState *bs, uint64_t dev_offset,
   uint64_t size, const char *name, const char *desc,
   const char *bitmap, bool readonly, bool shared,
diff --git a/block/export/export.c b/block/export/export.c
new file mode 100644
index 00..3d0dacb3f2
--- /dev/null
+++ b/block/export/export.c
@@ -0,0 +1,57 @@
+/*
+ * Common block export infrastructure
+ *
+ * Copyright (c) 2012, 2020 Red Hat, Inc.
+ *
+ * Authors:
+ * Paolo Bonzini 
+ * Kevin Wolf 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "block/export.h"
+#include "block/nbd.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-block-export.h"
+
+static const BlockExportDriver* blk_exp_drivers[] = {
+_exp_nbd,
+};
+
+static const BlockExportDriver *blk_exp_find_driver(BlockExportType type)
+{
+int i;
+
+for (i = 0; i < ARRAY_SIZE(blk_exp_drivers); i++) {
+if (blk_exp_drivers[i]->type == type) {
+return blk_exp_drivers[i];
+}
+}
+return NULL;
+}
+
+void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+{
+const BlockExportDriver *drv;
+
+drv = blk_exp_find_driver(export->type);
+if (!drv) {
+error_setg(errp, "No driver found for the requested export type");
+return;
+}
+
+drv->create(export, errp);
+}
+
+void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+{
+BlockExportOptions export = {
+.type = BLOCK_EXPORT_TYPE_NBD,
+.u.nbd = *arg,
+};
+qmp_block_export_add(, errp);
+}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 98ee1b6170..a1dc11bdd7 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -148,17 +148,20 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
 qapi_free_SocketAddress(addr_flat);
 }
 
-void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
+BlockExport *nbd_export_create(BlockExportOptions *exp_args, Error **errp)
 {
+BlockExportOptionsNbd *arg = _args->u.nbd;
 BlockDriverState *bs = NULL;