On Fri, May 30, 2014 at 01:18:43PM +0200, Benoît Canet wrote: > When a quorum file is totally destroyed (broken NAS or SAN) the user can > start a > drive-mirror job on the quorum block backend and then replace the broken > quorum file with drive-mirror-replace given it has a node-name. > > Signed-off-by: Benoit Canet <[email protected]> > --- > block.c | 8 ++-- > block/mirror.c | 118 > ++++++++++++++++++++++++++++++++++++++++++++-- > blockdev.c | 27 +++++++++++ > include/block/block.h | 3 ++ > include/block/block_int.h | 15 ++++++ > qapi-schema.json | 33 +++++++++++++ > qmp-commands.hx | 5 ++ > trace-events | 1 + > 8 files changed, 204 insertions(+), 6 deletions(-)
The drive-mirror-replace command is just block-job-complete plus
stashing arguments away. I think the approach you took in v1 was
simpler: add an argument to drive-mirror and don't introduce new QAPI
commands.
There was discussion of a generic swap command, but drive-mirror-replace
is not generic.
Please revert to the drive-mirror argument approach since the
drive-mirror + block-job-complete API is sufficient to implement this
behavior.
> @@ -502,6 +518,11 @@ immediate_exit:
> bdrv_unref(p);
> }
> }
> + if (s->must_replace) {
> + bdrv_op_unblock_all(s->to_replace, s->replace_blocker);
> + error_free(s->replace_blocker);
> + bdrv_unref(s->to_replace);
> + }
> bdrv_unref(s->target);
> block_job_completed(&s->common, ret);
> }
Can we do this unconditionally on the to_replace target (s->common.bs or
user-defined BDS)? This code is already fairly complex and it's nice to
avoid more conditionals.
The swap operation is really the same whether to_replace == s->common.bs
or a user-defined BDS. The op blocker cleanup and unref could be
unconditional as long as we op block and ref s->common.bs when no
user-defined BDS was given.
Stefan
pgp8YYDJmPt84.pgp
Description: PGP signature
