Re: [PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create()

2020-09-25 Thread Max Reitz
On 24.09.20 17:27, Kevin Wolf wrote:
> There is no real reason any more why nbd_export_new() and
> nbd_export_create() should be separate functions. The latter only
> performs a few checks before it calls the former.
> 
> What makes the current state stand out is that it's the only function in
> BlockExportDriver that is not a static function inside nbd/server.c, but
> a small wrapper in blockdev-nbd.c that then calls back into nbd/server.c
> for the real functionality.

Thanks :)



signature.asc
Description: OpenPGP digital signature


[PATCH v2 26/31] nbd: Merge nbd_export_new() and nbd_export_create()

2020-09-24 Thread Kevin Wolf
There is no real reason any more why nbd_export_new() and
nbd_export_create() should be separate functions. The latter only
performs a few checks before it calls the former.

What makes the current state stand out is that it's the only function in
BlockExportDriver that is not a static function inside nbd/server.c, but
a small wrapper in blockdev-nbd.c that then calls back into nbd/server.c
for the real functionality.

Move all the checks to nbd/server.c and make the resulting function
static to improve readability.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/nbd.h |  7 +-
 blockdev-nbd.c  | 40 +
 nbd/server.c| 54 -
 3 files changed, 45 insertions(+), 56 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 5270b7eadd..3dd9a04546 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -330,12 +330,6 @@ int nbd_errno_to_system_errno(int err);
 typedef struct NBDExport NBDExport;
 typedef struct NBDClient NBDClient;
 
-int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
-  Error **errp);
-int nbd_export_new(BlockExport *blk_exp,
-   const char *name, const char *desc,
-   const char *bitmap, bool readonly, bool shared,
-   Error **errp);
 void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
 
 AioContext *nbd_export_aio_context(NBDExport *exp);
@@ -349,6 +343,7 @@ void nbd_client_get(NBDClient *client);
 void nbd_client_put(NBDClient *client);
 
 void nbd_server_is_qemu_nbd(bool value);
+bool nbd_server_is_running(void);
 void nbd_server_start(SocketAddress *addr, const char *tls_creds,
   const char *tls_authz, uint32_t max_connections,
   Error **errp);
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 30e165c23f..8174023e5c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -37,6 +37,11 @@ void nbd_server_is_qemu_nbd(bool value)
 is_qemu_nbd = value;
 }
 
+bool nbd_server_is_running(void)
+{
+return nbd_server || is_qemu_nbd;
+}
+
 static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
 {
 nbd_client_put(client);
@@ -173,41 +178,6 @@ void qmp_nbd_server_start(SocketAddressLegacy *addr,
 qapi_free_SocketAddress(addr_flat);
 }
 
-int nbd_export_create(BlockExport *exp, BlockExportOptions *exp_args,
-  Error **errp)
-{
-BlockExportOptionsNbd *arg = _args->u.nbd;
-
-assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
-
-if (!nbd_server && !is_qemu_nbd) {
-error_setg(errp, "NBD server not running");
-return -EINVAL;
-}
-
-if (!arg->has_name) {
-arg->name = exp_args->node_name;
-}
-
-if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
-error_setg(errp, "export name '%s' too long", arg->name);
-return -EINVAL;
-}
-
-if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
-error_setg(errp, "description '%s' too long", arg->description);
-return -EINVAL;
-}
-
-if (nbd_export_find(arg->name)) {
-error_setg(errp, "NBD server already has export named '%s'", 
arg->name);
-return -EEXIST;
-}
-
-return nbd_export_new(exp, arg->name, arg->description, arg->bitmap,
-  !exp_args->writable, !exp_args->writable, errp);
-}
-
 void qmp_nbd_server_add(NbdServerAddOptions *arg, Error **errp)
 {
 BlockExport *export;
diff --git a/nbd/server.c b/nbd/server.c
index 465ec9e762..f74766add7 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1507,17 +1507,44 @@ void nbd_export_set_on_eject_blk(BlockExport *exp, 
BlockBackend *blk)
 blk_add_remove_bs_notifier(blk, _exp->eject_notifier);
 }
 
-int nbd_export_new(BlockExport *blk_exp,
-   const char *name, const char *desc,
-   const char *bitmap, bool readonly, bool shared,
-   Error **errp)
+static int nbd_export_create(BlockExport *blk_exp, BlockExportOptions 
*exp_args,
+ Error **errp)
 {
 NBDExport *exp = container_of(blk_exp, NBDExport, common);
+BlockExportOptionsNbd *arg = _args->u.nbd;
 BlockBackend *blk = blk_exp->blk;
 int64_t size;
 uint64_t perm, shared_perm;
+bool readonly = !exp_args->writable;
+bool shared = !exp_args->writable;
 int ret;
 
+assert(exp_args->type == BLOCK_EXPORT_TYPE_NBD);
+
+if (!nbd_server_is_running()) {
+error_setg(errp, "NBD server not running");
+return -EINVAL;
+}
+
+if (!arg->has_name) {
+arg->name = exp_args->node_name;
+}
+
+if (strlen(arg->name) > NBD_MAX_STRING_SIZE) {
+error_setg(errp, "export name '%s' too long", arg->name);
+return -EINVAL;
+}
+
+if (arg->description && strlen(arg->description) > NBD_MAX_STRING_SIZE) {
+error_setg(errp, "description '%s'