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 <jc...@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".
