| Message ID | e0ba08677155b2cf3820b580fb5057458ed017d0.1539786605.git.berto@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Misc Quorum fixes | expand |
Am 17.10.2018 um 16:33 hat Alberto Garcia geschrieben: > The blkverify mode of Quorum can only be enabled if the number of > children is exactly two and the value of vote-threshold is also two. > > If the user tries to enable it but the other settings are incorrect > then QEMU simply prints an error message to stderr and carries on > disabling the blkverify setting. > > This patch makes quorum_open() fail and return an error in this case. > > Signed-off-by: Alberto Garcia <berto@igalia.com> > Reported-by: Markus Armbruster <armbru@redhat.com> While reviewing this, I think I found another problem: Shouldn't .bdrv_add_child refuse to add a child if s->is_blkverify == true? (And probably assert in .bdrv_del_child that s->is_blkverify == false after checking s->num_children <= s->threshold) Kevin
On Wed 17 Oct 2018 05:31:32 PM CEST, Kevin Wolf wrote: > Am 17.10.2018 um 16:33 hat Alberto Garcia geschrieben: >> The blkverify mode of Quorum can only be enabled if the number of >> children is exactly two and the value of vote-threshold is also two. >> >> If the user tries to enable it but the other settings are incorrect >> then QEMU simply prints an error message to stderr and carries on >> disabling the blkverify setting. >> >> This patch makes quorum_open() fail and return an error in this case. >> >> Signed-off-by: Alberto Garcia <berto@igalia.com> >> Reported-by: Markus Armbruster <armbru@redhat.com> > > While reviewing this, I think I found another problem: Shouldn't > .bdrv_add_child refuse to add a child if s->is_blkverify == true? (And > probably assert in .bdrv_del_child that s->is_blkverify == false after > checking s->num_children <= s->threshold) I think you're right. Do you want me to send this in a later patch or do you prefer that I make it part of this series? Berto
Am 17.10.2018 um 17:34 hat Alberto Garcia geschrieben: > On Wed 17 Oct 2018 05:31:32 PM CEST, Kevin Wolf wrote: > > Am 17.10.2018 um 16:33 hat Alberto Garcia geschrieben: > >> The blkverify mode of Quorum can only be enabled if the number of > >> children is exactly two and the value of vote-threshold is also two. > >> > >> If the user tries to enable it but the other settings are incorrect > >> then QEMU simply prints an error message to stderr and carries on > >> disabling the blkverify setting. > >> > >> This patch makes quorum_open() fail and return an error in this case. > >> > >> Signed-off-by: Alberto Garcia <berto@igalia.com> > >> Reported-by: Markus Armbruster <armbru@redhat.com> > > > > While reviewing this, I think I found another problem: Shouldn't > > .bdrv_add_child refuse to add a child if s->is_blkverify == true? (And > > probably assert in .bdrv_del_child that s->is_blkverify == false after > > checking s->num_children <= s->threshold) > > I think you're right. Do you want me to send this in a later patch or do > you prefer that I make it part of this series? This series looked good, so no need to respin. A separate patch is fine. Kevin
diff --git a/block/quorum.c b/block/quorum.c index b1b777baef..6188ff6666 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -912,13 +912,12 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags, s->read_pattern = ret; if (s->read_pattern == QUORUM_READ_PATTERN_QUORUM) { - /* is the driver in blkverify mode */ - if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false) && - s->num_children == 2 && s->threshold == 2) { - s->is_blkverify = true; - } else if (qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false)) { - fprintf(stderr, "blkverify mode is set by setting blkverify=on " - "and using two files with vote_threshold=2\n"); + s->is_blkverify = qemu_opt_get_bool(opts, QUORUM_OPT_BLKVERIFY, false); + if (s->is_blkverify && (s->num_children != 2 || s->threshold != 2)) { + error_setg(&local_err, "blkverify=on can only be set if there are " + "exactly two files and vote-threshold is 2"); + ret = -EINVAL; + goto exit; } s->rewrite_corrupted = qemu_opt_get_bool(opts, QUORUM_OPT_REWRITE,
The blkverify mode of Quorum can only be enabled if the number of children is exactly two and the value of vote-threshold is also two. If the user tries to enable it but the other settings are incorrect then QEMU simply prints an error message to stderr and carries on disabling the blkverify setting. This patch makes quorum_open() fail and return an error in this case. Signed-off-by: Alberto Garcia <berto@igalia.com> Reported-by: Markus Armbruster <armbru@redhat.com> --- block/quorum.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)