| Message ID | 20230502211119.720647-1-stefanha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block/export: call blk_set_dev_ops(blk, NULL, NULL) | expand |
On Tue, May 02, 2023 at 05:11:19PM -0400, Stefan Hajnoczi wrote: > Most export types install BlockDeviceOps pointers. It is easy to forget > to remove them because that happens automatically via the "drive" qdev > property in hw/ but not block/export/. > > Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so > the export types don't need to remember. > > This fixes the nbd and vhost-user-blk export types. > > Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server") > Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize") > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/export/export.c | 2 ++ > block/export/vduse-blk.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake <eblake@redhat.com> I'm happy to add this through my NBD queue.
On Wed, May 03, 2023 at 10:43:16AM -0500, Eric Blake wrote: > On Tue, May 02, 2023 at 05:11:19PM -0400, Stefan Hajnoczi wrote: > > Most export types install BlockDeviceOps pointers. It is easy to forget > > to remove them because that happens automatically via the "drive" qdev > > property in hw/ but not block/export/. > > > > Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so > > the export types don't need to remember. > > > > This fixes the nbd and vhost-user-blk export types. > > > > Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server") > > Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize") > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > --- > > block/export/export.c | 2 ++ > > block/export/vduse-blk.c | 1 - > > 2 files changed, 2 insertions(+), 1 deletion(-) > > Reviewed-by: Eric Blake <eblake@redhat.com> > > I'm happy to add this through my NBD queue. Sure, go ahead! Stefan
Am 02.05.2023 um 23:11 hat Stefan Hajnoczi geschrieben: > Most export types install BlockDeviceOps pointers. It is easy to forget > to remove them because that happens automatically via the "drive" qdev > property in hw/ but not block/export/. > > Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so > the export types don't need to remember. > > This fixes the nbd and vhost-user-blk export types. > > Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server") > Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize") > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/export/export.c | 2 ++ > block/export/vduse-blk.c | 1 - > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/export/export.c b/block/export/export.c > index e3fee60611..62c7c22d45 100644 > --- a/block/export/export.c > +++ b/block/export/export.c > @@ -192,6 +192,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) > return exp; > > fail: > + blk_set_dev_ops(exp->blk, NULL, NULL); > blk_unref(blk); > aio_context_release(ctx); > if (exp) { The last line of the context already shows that dereferencing exp unconditionally is wrong. I'll fix it in my next series that tries to address Fiona's concern that we need to take the graph lock even in the main thread if we're in a coroutine. Kevin
diff --git a/block/export/export.c b/block/export/export.c index e3fee60611..62c7c22d45 100644 --- a/block/export/export.c +++ b/block/export/export.c @@ -192,6 +192,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) return exp; fail: + blk_set_dev_ops(exp->blk, NULL, NULL); blk_unref(blk); aio_context_release(ctx); if (exp) { @@ -219,6 +220,7 @@ static void blk_exp_delete_bh(void *opaque) assert(exp->refcount == 0); QLIST_REMOVE(exp, next); exp->drv->delete(exp); + blk_set_dev_ops(exp->blk, NULL, NULL); blk_unref(exp->blk); qapi_event_send_block_export_deleted(exp->id); g_free(exp->id); diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c index f7ae44e3ce..b53ef39da0 100644 --- a/block/export/vduse-blk.c +++ b/block/export/vduse-blk.c @@ -346,7 +346,6 @@ static void vduse_blk_exp_delete(BlockExport *exp) blk_remove_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach, vblk_exp); - blk_set_dev_ops(exp->blk, NULL, NULL); ret = vduse_dev_destroy(vblk_exp->dev); if (ret != -EBUSY) { unlink(vblk_exp->recon_file);
Most export types install BlockDeviceOps pointers. It is easy to forget to remove them because that happens automatically via the "drive" qdev property in hw/ but not block/export/. Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so the export types don't need to remember. This fixes the nbd and vhost-user-blk export types. Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server") Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize") Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- block/export/export.c | 2 ++ block/export/vduse-blk.c | 1 - 2 files changed, 2 insertions(+), 1 deletion(-)