[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 11/37] block/export: Remove magic from block-export-add
From: |
Kevin Wolf |
Subject: |
[PULL 11/37] block/export: Remove magic from block-export-add |
Date: |
Fri, 2 Oct 2020 16:43:19 +0200 |
nbd-server-add tries to be convenient and adds two questionable
features that we don't want to share in block-export-add, even for NBD
exports:
1. When requesting a writable export of a read-only device, the export
is silently downgraded to read-only. This should be an error in the
context of block-export-add.
2. When using a BlockBackend name, unplugging the device from the guest
will automatically stop the NBD server, too. This may sometimes be
what you want, but it could also be very surprising. Let's keep
things explicit with block-export-add. If the user wants to stop the
export, they should tell us so.
Move these things into the nbd-server-add QMP command handler so that
they apply only there.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20200924152717.287415-8-kwolf@redhat.com>
Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/export.h | 2 ++
include/block/nbd.h | 3 ++-
block/export/export.c | 13 +++++++++---
blockdev-nbd.c | 47 +++++++++++++++++++++++++++++++++++-------
nbd/server.c | 20 +++++++++++-------
qemu-nbd.c | 3 +--
6 files changed, 67 insertions(+), 21 deletions(-)
diff --git a/include/block/export.h b/include/block/export.h
index 42e3c055fc..e7af2c7687 100644
--- a/include/block/export.h
+++ b/include/block/export.h
@@ -30,4 +30,6 @@ struct BlockExport {
const BlockExportDriver *drv;
};
+BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp);
+
#endif
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 451f399b0a..f55f5b710b 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -335,7 +335,8 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
- BlockBackend *on_eject_blk, Error **errp);
+ Error **errp);
+void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk);
void nbd_export_close(NBDExport *exp);
void nbd_export_remove(NBDExport *exp, NbdServerRemoveMode mode, Error **errp);
void nbd_export_get(NBDExport *exp);
diff --git a/block/export/export.c b/block/export/export.c
index fd65541963..05bc5e3744 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -13,6 +13,8 @@
#include "qemu/osdep.h"
+#include "block/block.h"
+#include "sysemu/block-backend.h"
#include "block/export.h"
#include "block/nbd.h"
#include "qapi/error.h"
@@ -34,15 +36,20 @@ static const BlockExportDriver
*blk_exp_find_driver(BlockExportType type)
return NULL;
}
-void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+BlockExport *blk_exp_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;
+ return NULL;
}
- drv->create(export, errp);
+ return drv->create(export, errp);
+}
+
+void qmp_block_export_add(BlockExportOptions *export, Error **errp)
+{
+ blk_exp_add(export, errp);
}
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 96cb0100e9..7bcca105f9 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -152,7 +152,6 @@ BlockExport *nbd_export_create(BlockExportOptions
*exp_args, Error **errp)
{
BlockExportOptionsNbd *arg = &exp_args->u.nbd;
BlockDriverState *bs = NULL;
- BlockBackend *on_eject_blk;
NBDExport *exp = NULL;
AioContext *aio_context;
@@ -182,8 +181,6 @@ BlockExport *nbd_export_create(BlockExportOptions
*exp_args, Error **errp)
return NULL;
}
- on_eject_blk = blk_by_name(arg->device);
-
bs = bdrv_lookup_bs(arg->device, arg->device, errp);
if (!bs) {
return NULL;
@@ -195,13 +192,14 @@ BlockExport *nbd_export_create(BlockExportOptions
*exp_args, Error **errp)
if (!arg->has_writable) {
arg->writable = false;
}
- if (bdrv_is_read_only(bs)) {
- arg->writable = false;
+ if (bdrv_is_read_only(bs) && arg->writable) {
+ error_setg(errp, "Cannot export read-only node as writable");
+ goto out;
}
exp = nbd_export_new(bs, arg->name, arg->description, arg->bitmap,
!arg->writable, !arg->writable,
- NULL, false, on_eject_blk, errp);
+ NULL, false, errp);
if (!exp) {
goto out;
}
@@ -219,11 +217,44 @@ BlockExport *nbd_export_create(BlockExportOptions
*exp_args, Error **errp)
void qmp_nbd_server_add(BlockExportOptionsNbd *arg, Error **errp)
{
- BlockExportOptions export = {
+ BlockExport *export;
+ BlockDriverState *bs;
+ BlockBackend *on_eject_blk;
+ BlockExportOptions export_opts;
+
+ bs = bdrv_lookup_bs(arg->device, arg->device, errp);
+ if (!bs) {
+ return;
+ }
+
+ export_opts = (BlockExportOptions) {
.type = BLOCK_EXPORT_TYPE_NBD,
.u.nbd = *arg,
};
- qmp_block_export_add(&export, errp);
+
+ /*
+ * nbd-server-add doesn't complain when a read-only device should be
+ * exported as writable, but simply downgrades it. This is an error with
+ * block-export-add.
+ */
+ if (bdrv_is_read_only(bs)) {
+ export_opts.u.nbd.has_writable = true;
+ export_opts.u.nbd.writable = false;
+ }
+
+ export = blk_exp_add(&export_opts, errp);
+ if (!export) {
+ return;
+ }
+
+ /*
+ * nbd-server-add removes the export when the named BlockBackend used for
+ * @device goes away.
+ */
+ on_eject_blk = blk_by_name(arg->device);
+ if (on_eject_blk) {
+ nbd_export_set_on_eject_blk(export, on_eject_blk);
+ }
}
void qmp_nbd_server_remove(const char *name,
diff --git a/nbd/server.c b/nbd/server.c
index 33aaca918c..23d9a53094 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1506,11 +1506,23 @@ static void nbd_eject_notifier(Notifier *n, void *data)
aio_context_release(aio_context);
}
+void nbd_export_set_on_eject_blk(BlockExport *exp, BlockBackend *blk)
+{
+ NBDExport *nbd_exp = container_of(exp, NBDExport, common);
+ assert(exp->drv == &blk_exp_nbd);
+ assert(nbd_exp->eject_notifier_blk == NULL);
+
+ blk_ref(blk);
+ nbd_exp->eject_notifier_blk = blk;
+ nbd_exp->eject_notifier.notify = nbd_eject_notifier;
+ blk_add_remove_bs_notifier(blk, &nbd_exp->eject_notifier);
+}
+
NBDExport *nbd_export_new(BlockDriverState *bs,
const char *name, const char *desc,
const char *bitmap, bool readonly, bool shared,
void (*close)(NBDExport *), bool writethrough,
- BlockBackend *on_eject_blk, Error **errp)
+ Error **errp)
{
AioContext *ctx;
BlockBackend *blk;
@@ -1617,12 +1629,6 @@ NBDExport *nbd_export_new(BlockDriverState *bs,
exp->ctx = ctx;
blk_add_aio_context_notifier(blk, blk_aio_attached, blk_aio_detach, exp);
- if (on_eject_blk) {
- blk_ref(on_eject_blk);
- exp->eject_notifier_blk = on_eject_blk;
- exp->eject_notifier.notify = nbd_eject_notifier;
- blk_add_remove_bs_notifier(on_eject_blk, &exp->eject_notifier);
- }
QTAILQ_INSERT_TAIL(&exports, exp, next);
nbd_export_get(exp);
return exp;
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 16473d809c..b2a0ea6c5e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1067,8 +1067,7 @@ int main(int argc, char **argv)
export = nbd_export_new(bs, export_name,
export_description, bitmap, readonly, shared > 1,
- nbd_export_closed, writethrough, NULL,
- &error_fatal);
+ nbd_export_closed, writethrough, &error_fatal);
if (device) {
#if HAVE_NBD_DEVICE
--
2.25.4
- [PULL 00/37] Block layer patches, Kevin Wolf, 2020/10/02
- [PULL 01/37] block/sheepdog: Replace magic val by NANOSECONDS_PER_SECOND definition, Kevin Wolf, 2020/10/02
- [PULL 02/37] tests/check-block: Do not run the iotests with old versions of bash, Kevin Wolf, 2020/10/02
- [PULL 04/37] qemu-io-cmds: Simplify help_oneline, Kevin Wolf, 2020/10/02
- [PULL 03/37] docs: Document the throttle block filter, Kevin Wolf, 2020/10/02
- [PULL 05/37] nbd: Remove unused nbd_export_get_blockdev(), Kevin Wolf, 2020/10/02
- [PULL 08/37] block/export: Add BlockExport infrastructure and block-export-add, Kevin Wolf, 2020/10/02
- [PULL 06/37] qapi: Create block-export module, Kevin Wolf, 2020/10/02
- [PULL 09/37] qemu-storage-daemon: Use qmp_block_export_add(), Kevin Wolf, 2020/10/02
- [PULL 07/37] qapi: Rename BlockExport to BlockExportOptions, Kevin Wolf, 2020/10/02
- [PULL 11/37] block/export: Remove magic from block-export-add,
Kevin Wolf <=
- [PULL 10/37] qemu-nbd: Use raw block driver for --offset, Kevin Wolf, 2020/10/02
- [PULL 12/37] nbd: Add max-connections to nbd-server-start, Kevin Wolf, 2020/10/02
- [PULL 13/37] nbd: Add writethrough to block-export-add, Kevin Wolf, 2020/10/02
- [PULL 16/37] nbd/server: Simplify export shutdown, Kevin Wolf, 2020/10/02
- [PULL 14/37] nbd: Remove NBDExport.close callback, Kevin Wolf, 2020/10/02
- [PULL 17/37] block/export: Move refcount from NBDExport to BlockExport, Kevin Wolf, 2020/10/02
- [PULL 15/37] qemu-nbd: Use blk_exp_add() to create the export, Kevin Wolf, 2020/10/02
- [PULL 18/37] block/export: Move AioContext from NBDExport to BlockExport, Kevin Wolf, 2020/10/02
- [PULL 19/37] block/export: Add node-name to BlockExportOptions, Kevin Wolf, 2020/10/02
- [PULL 21/37] block/export: Add blk_exp_close_all(_type), Kevin Wolf, 2020/10/02