[Qemu-devel] [PATCH] block: add .bdrv_reopen_prepare() stub for iscsi

[Qemu-devel] [PATCH] block: add .bdrv_reopen_prepare() stub for iscsi

[Jeff Cody]

To suppport reopen(), the .bdrv_reopen_prepare() stub must exist.
iSCSI does not have anything that needs to be done to support reopen,
so we can just implement the _prepare() stub.

Signed-off-by: Jeff Cody <jcody@redhat.com>
---
 block/iscsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/block/iscsi.c b/block/iscsi.c
index c0ea0c4..5976bd1 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1326,6 +1326,14 @@ static void iscsi_close(BlockDriverState *bs)
     memset(iscsilun, 0, sizeof(IscsiLun));
 }
 
+/* We have nothing to do for iSCSI reopen, stub just returns
+ * success */
+static int iscsi_reopen_prepare(BDRVReopenState *state,
+                                BlockReopenQueue *queue, Error **errp)
+{
+    return 0;
+}
+
 static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
 {
     IscsiLun *iscsilun = bs->opaque;
@@ -1434,6 +1442,7 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_close      = iscsi_close,
     .bdrv_create     = iscsi_create,
     .create_options  = iscsi_create_options,
+    .bdrv_reopen_prepare  = iscsi_reopen_prepare,
 
     .bdrv_getlength  = iscsi_getlength,
     .bdrv_get_info   = iscsi_get_info,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] block: add .bdrv_reopen_prepare() stub for iscsi

[Stefan Hajnoczi]

On Tue, Jan 14, 2014 at 01:10:24PM -0500, Jeff Cody wrote:
> To suppport reopen(), the .bdrv_reopen_prepare() stub must exist.
> iSCSI does not have anything that needs to be done to support reopen,
> so we can just implement the _prepare() stub.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/iscsi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index c0ea0c4..5976bd1 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1326,6 +1326,14 @@ static void iscsi_close(BlockDriverState *bs)
>      memset(iscsilun, 0, sizeof(IscsiLun));
>  }
>  
> +/* We have nothing to do for iSCSI reopen, stub just returns
> + * success */
> +static int iscsi_reopen_prepare(BDRVReopenState *state,
> +                                BlockReopenQueue *queue, Error **errp)
> +{
> +    return 0;
> +}

I think we should be more specific here: the assumption is that
bdrv_reopen() is only used to change bdrv_open() flags.  And since
block/iscsi.c ignores bdrv_open() flags there is no difference whether
we actually reopen the LUN.

In other words, this patch means bdrv_reopen() will *not* reconnect to
the iSCSI target, which could be useful for other reasons (e.g. graceful
restart of iSCSI target).  I think this can be justified because
block/raw-posix.c also tries to use dup instead of actually reopening
the filename.  But we should be aware of the semantics of bdrv_reopen().

Anyway, in the future the assumption that block/iscsi.c ignores
bdrv_open() flags could change.  This reopen implementation will break!

Please add a comment to iscsi_open() explaining that anyone wishing to
use flags must consider modifying reopen too.  Please also change the
comment/commit message to be more specific about why "We have nothing to
do for iSCSI reopen".

Re: [Qemu-devel] [PATCH] block: add .bdrv_reopen_prepare() stub for iscsi

[Paolo Bonzini]

Il 14/01/2014 19:10, Jeff Cody ha scritto:
> To suppport reopen(), the .bdrv_reopen_prepare() stub must exist.
> iSCSI does not have anything that needs to be done to support reopen,
> so we can just implement the _prepare() stub.
> 
> Signed-off-by: Jeff Cody <jcody@redhat.com>
> ---
>  block/iscsi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index c0ea0c4..5976bd1 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1326,6 +1326,14 @@ static void iscsi_close(BlockDriverState *bs)
>      memset(iscsilun, 0, sizeof(IscsiLun));
>  }
>  
> +/* We have nothing to do for iSCSI reopen, stub just returns
> + * success */
> +static int iscsi_reopen_prepare(BDRVReopenState *state,
> +                                BlockReopenQueue *queue, Error **errp)
> +{
> +    return 0;
> +}
> +
>  static int iscsi_truncate(BlockDriverState *bs, int64_t offset)
>  {
>      IscsiLun *iscsilun = bs->opaque;
> @@ -1434,6 +1442,7 @@ static BlockDriver bdrv_iscsi = {
>      .bdrv_close      = iscsi_close,
>      .bdrv_create     = iscsi_create,
>      .create_options  = iscsi_create_options,
> +    .bdrv_reopen_prepare  = iscsi_reopen_prepare,
>  
>      .bdrv_getlength  = iscsi_getlength,
>      .bdrv_get_info   = iscsi_get_info,
> 

Applied to scsi-next branch, thanks.

Paolo