Message ID | 1433360659-1915-8-git-send-email-mreitz@redhat.com |
---|---|
State | New |
Headers | show |
On Wed 03 Jun 2015 09:43:48 PM CEST, Max Reitz wrote: > bdrv_is_inserted() should be invoked recursively on the children of > quorum. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > +static bool quorum_is_inserted(BlockDriverState *bs) > +{ > + BDRVQuorumState *s = bs->opaque; > + int i; > + > + for (i = 0; i < s->num_children; i++) { > + if (!bdrv_is_inserted(s->bs[i])) { > + return false; > + } > + } > + > + return true; > +} > + I wonder if it can actually happen that only some of the BDS are inserted :-? Berto
On 06/04/2015 06:37 AM, Alberto Garcia wrote: > On Wed 03 Jun 2015 09:43:48 PM CEST, Max Reitz wrote: >> bdrv_is_inserted() should be invoked recursively on the children of >> quorum. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- > >> +static bool quorum_is_inserted(BlockDriverState *bs) >> +{ >> + BDRVQuorumState *s = bs->opaque; >> + int i; >> + >> + for (i = 0; i < s->num_children; i++) { >> + if (!bdrv_is_inserted(s->bs[i])) { >> + return false; >> + } >> + } >> + >> + return true; >> +} >> + > > I wonder if it can actually happen that only some of the BDS are > inserted :-? Probably not possible while having a working quorum. But if I understand the series correctly, there may be windows of time when building up or hot-swapping a child within a quorum where things are not yet consistent, but where the code can query the current state of the quorum, so it's better to be prepared for those windows. And while unlikely, it is possible to build up a quorum that includes host cdrom passthrough or other scenario where one child can independently fail to be inserted. Who knows - we may even take advantage of this in COLO checkpointing where an NBD child is not yet inserted.
On 04.06.2015 14:37, Alberto Garcia wrote: > On Wed 03 Jun 2015 09:43:48 PM CEST, Max Reitz wrote: >> bdrv_is_inserted() should be invoked recursively on the children of >> quorum. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> +static bool quorum_is_inserted(BlockDriverState *bs) >> +{ >> + BDRVQuorumState *s = bs->opaque; >> + int i; >> + >> + for (i = 0; i < s->num_children; i++) { >> + if (!bdrv_is_inserted(s->bs[i])) { >> + return false; >> + } >> + } >> + >> + return true; >> +} >> + > I wonder if it can actually happen that only some of the BDS are > inserted :-? I'm actually able to start Quorum on an empty DVD drive: $ x86_64-softmmu/qemu-system-x86_64 -drive if=none,id=drv,driver=quorum,children.0.file.filename=/dev/sr0,children.0.driver=raw,children.1.file.filename=test.qcow2,vote-threshold=1 -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 3, "major": 2}, "package": ""}, "capabilities": []}} {'execute':'qmp_capabilities'} {"return": {}} {'execute':'human-monitor-command','arguments':{'command-line':'qemu-io drv "write 0 512"'}} write failed: No medium found {"return": ""} (note that on master, the write is successful (???), but you do get: {"timestamp": {"seconds": 1433518087, "microseconds": 527326}, "event": "QUORUM_REPORT_BAD", "data": {"node-name": "", "sectors-count": 1, "sector-num": 0, "error": "No medium found"}}) Thank you for reviewing! :-) Max
On Wed 03 Jun 2015 09:43:48 PM CEST, Max Reitz <mreitz@redhat.com> wrote: > bdrv_is_inserted() should be invoked recursively on the children of > quorum. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Berto
diff --git a/block/quorum.c b/block/quorum.c index f91ef75..61fdcd0 100644 --- a/block/quorum.c +++ b/block/quorum.c @@ -1061,6 +1061,20 @@ static void quorum_refresh_filename(BlockDriverState *bs) bs->full_open_options = opts; } +static bool quorum_is_inserted(BlockDriverState *bs) +{ + BDRVQuorumState *s = bs->opaque; + int i; + + for (i = 0; i < s->num_children; i++) { + if (!bdrv_is_inserted(s->bs[i])) { + return false; + } + } + + return true; +} + static BlockDriver bdrv_quorum = { .format_name = "quorum", .protocol_name = "quorum", @@ -1084,6 +1098,8 @@ static BlockDriver bdrv_quorum = { .is_filter = true, .bdrv_recurse_is_first_non_filter = quorum_recurse_is_first_non_filter, + + .bdrv_is_inserted = quorum_is_inserted, }; static void bdrv_quorum_init(void)