[23/25] block: add close notifiers

Submitted by Paolo Bonzini on Oct. 10, 2012, 2:03 p.m.

Details

Message ID 1349877786-23514-24-git-send-email-pbonzini@redhat.com
State New
Headers show

Commit Message

Paolo Bonzini Oct. 10, 2012, 2:03 p.m.
The first user of close notifiers will be the embedded NBD server.
It is possible to use them to do some of the ad hoc processing
(e.g. for block jobs and I/O limits) that is currently done by
bdrv_close.

Acked-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs |  4 ++--
 block.c       | 19 ++++++++++++++-----
 block.h       |  1 +
 block_int.h   |  2 ++
 4 file modificati, 19 inserzioni(+), 7 rimozioni(-)

Comments

Markus Armbruster Oct. 19, 2012, 8:21 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> The first user of close notifiers will be the embedded NBD server.
> It is possible to use them to do some of the ad hoc processing
> (e.g. for block jobs and I/O limits) that is currently done by
> bdrv_close.
>
> Acked-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  Makefile.objs |  4 ++--
>  block.c       | 19 ++++++++++++++-----
>  block.h       |  1 +
>  block_int.h   |  2 ++
>  4 file modificati, 19 inserzioni(+), 7 rimozioni(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 3f16d67..ca67885 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -43,7 +43,7 @@ coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
>  
>  block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
>  block-obj-y += nbd.o block.o blockjob.o aio.o aes.o qemu-config.o
> -block-obj-y += qemu-progress.o qemu-sockets.o uri.o
> +block-obj-y += qemu-progress.o qemu-sockets.o uri.o notify.o
>  block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
>  block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
>  block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
> @@ -94,7 +94,7 @@ common-obj-y += bt-host.o bt-vhci.o
>  common-obj-y += dma-helpers.o
>  common-obj-y += iov.o acl.o
>  common-obj-$(CONFIG_POSIX) += compatfd.o
> -common-obj-y += notify.o event_notifier.o
> +common-obj-y += event_notifier.o
>  common-obj-y += qemu-timer.o qemu-timer-common.o
>  common-obj-y += qtest.o
>  common-obj-y += vl.o
> diff --git a/block.c b/block.c
> index e95f613..56426a9 100644
> --- a/block.c
> +++ b/block.c
> @@ -30,6 +30,7 @@
>  #include "module.h"
>  #include "qjson.h"
>  #include "sysemu.h"
> +#include "notify.h"
>  #include "qemu-coroutine.h"
>  #include "qmp-commands.h"
>  #include "qemu-timer.h"
> @@ -312,9 +313,16 @@ BlockDriverState *bdrv_new(const char *device_name)
>          QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
>      }
>      bdrv_iostatus_disable(bs);
> +    notifier_list_init(&bs->close_notifiers);
> +
>      return bs;
>  }
>  
> +void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
> +{
> +    notifier_list_add(&bs->close_notifiers, notify);
> +}
> +
>  BlockDriver *bdrv_find_format(const char *format_name)
>  {
>      BlockDriver *drv1;
> @@ -1098,12 +1106,13 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
>  void bdrv_close(BlockDriverState *bs)
>  {
>      bdrv_flush(bs);
> -    if (bs->drv) {
> -        if (bs->job) {
> -            block_job_cancel_sync(bs->job);
> -        }
> -        bdrv_drain_all();
> +    if (bs->job) {
> +        block_job_cancel_sync(bs->job);
> +    }
> +    bdrv_drain_all();

Dropping the bs->drv condition in a separate commit gives you a nice
place to explain why it's fine: the commit message.  I figure it is, but
it's not 100% obvious.

> +    notifier_list_notify(&bs->close_notifiers, bs);
>  
> +    if (bs->drv) {
>          if (bs == bs_snapshots) {
>              bs_snapshots = NULL;
>          }
[...]
Markus Armbruster Oct. 19, 2012, 8:26 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> The first user of close notifiers will be the embedded NBD server.
> It is possible to use them to do some of the ad hoc processing
> (e.g. for block jobs and I/O limits) that is currently done by
> bdrv_close.

If the second sentence is an idea for future work, you could make that
clearer by wriging "It would be possible to use them".

[...]
Paolo Bonzini Oct. 19, 2012, 9:38 a.m.
> > @@ -1098,12 +1106,13 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
> >  void bdrv_close(BlockDriverState *bs)
> >  {
> >      bdrv_flush(bs);
> > -    if (bs->drv) {
> > -        if (bs->job) {
> > -            block_job_cancel_sync(bs->job);
> > -        }
> > -        bdrv_drain_all();
> > +    if (bs->job) {
> > +        block_job_cancel_sync(bs->job);
> > +    }
> > +    bdrv_drain_all();
> 
> Dropping the bs->drv condition in a separate commit gives you a nice
> place to explain why it's fine: the commit message.  I figure it is,
> but it's not 100% obvious.

Will do, thanks.

Paolo

Patch hide | download patch | download mbox

diff --git a/Makefile.objs b/Makefile.objs
index 3f16d67..ca67885 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -43,7 +43,7 @@  coroutine-obj-$(CONFIG_WIN32) += coroutine-win32.o
 
 block-obj-y = cutils.o iov.o cache-utils.o qemu-option.o module.o async.o
 block-obj-y += nbd.o block.o blockjob.o aio.o aes.o qemu-config.o
-block-obj-y += qemu-progress.o qemu-sockets.o uri.o
+block-obj-y += qemu-progress.o qemu-sockets.o uri.o notify.o
 block-obj-y += $(coroutine-obj-y) $(qobject-obj-y) $(version-obj-y)
 block-obj-$(CONFIG_POSIX) += posix-aio-compat.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
@@ -94,7 +94,7 @@  common-obj-y += bt-host.o bt-vhci.o
 common-obj-y += dma-helpers.o
 common-obj-y += iov.o acl.o
 common-obj-$(CONFIG_POSIX) += compatfd.o
-common-obj-y += notify.o event_notifier.o
+common-obj-y += event_notifier.o
 common-obj-y += qemu-timer.o qemu-timer-common.o
 common-obj-y += qtest.o
 common-obj-y += vl.o
diff --git a/block.c b/block.c
index e95f613..56426a9 100644
--- a/block.c
+++ b/block.c
@@ -30,6 +30,7 @@ 
 #include "module.h"
 #include "qjson.h"
 #include "sysemu.h"
+#include "notify.h"
 #include "qemu-coroutine.h"
 #include "qmp-commands.h"
 #include "qemu-timer.h"
@@ -312,9 +313,16 @@  BlockDriverState *bdrv_new(const char *device_name)
         QTAILQ_INSERT_TAIL(&bdrv_states, bs, list);
     }
     bdrv_iostatus_disable(bs);
+    notifier_list_init(&bs->close_notifiers);
+
     return bs;
 }
 
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
+{
+    notifier_list_add(&bs->close_notifiers, notify);
+}
+
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
@@ -1098,12 +1106,13 @@  void bdrv_reopen_abort(BDRVReopenState *reopen_state)
 void bdrv_close(BlockDriverState *bs)
 {
     bdrv_flush(bs);
-    if (bs->drv) {
-        if (bs->job) {
-            block_job_cancel_sync(bs->job);
-        }
-        bdrv_drain_all();
+    if (bs->job) {
+        block_job_cancel_sync(bs->job);
+    }
+    bdrv_drain_all();
+    notifier_list_notify(&bs->close_notifiers, bs);
 
+    if (bs->drv) {
         if (bs == bs_snapshots) {
             bs_snapshots = NULL;
         }
diff --git a/block.h b/block.h
index e2d89d7..aa608a8 100644
--- a/block.h
+++ b/block.h
@@ -144,6 +144,7 @@  int bdrv_reopen_prepare(BDRVReopenState *reopen_state,
 void bdrv_reopen_commit(BDRVReopenState *reopen_state);
 void bdrv_reopen_abort(BDRVReopenState *reopen_state);
 void bdrv_close(BlockDriverState *bs);
+void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify);
 int bdrv_attach_dev(BlockDriverState *bs, void *dev);
 void bdrv_attach_dev_nofail(BlockDriverState *bs, void *dev);
 void bdrv_detach_dev(BlockDriverState *bs, void *dev);
diff --git a/block_int.h b/block_int.h
index f4bae04..cedabbd 100644
--- a/block_int.h
+++ b/block_int.h
@@ -233,6 +233,8 @@  struct BlockDriverState {
     BlockDriverState *backing_hd;
     BlockDriverState *file;
 
+    NotifierList close_notifiers;
+
     /* number of in-flight copy-on-read requests */
     unsigned int copy_on_read_in_flight;