From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58844) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXAvu-0000rK-W4 for qemu-devel@nongnu.org; Wed, 02 Sep 2015 12:30:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXAvq-0007Tz-97 for qemu-devel@nongnu.org; Wed, 02 Sep 2015 12:30:46 -0400 References: <1441183880-26993-1-git-send-email-wency@cn.fujitsu.com> <1441183880-26993-7-git-send-email-wency@cn.fujitsu.com> From: Eric Blake Message-ID: <55E72422.30301@redhat.com> Date: Wed, 2 Sep 2015 10:30:26 -0600 MIME-Version: 1.0 In-Reply-To: <1441183880-26993-7-git-send-email-wency@cn.fujitsu.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="IeHDGbjBTpln4BirIBgLn9FjqJCM6QpBD" Subject: Re: [Qemu-devel] [PATCH 06/16] quorum: allow ignoring child errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wen Congyang , qemu devel , Fam Zheng , Max Reitz , Paolo Bonzini , Stefan Hajnoczi Cc: Kevin Wolf , Alberto Garcia , zhanghailiang , qemu block , Jiang Yunhong , Dong Eddie , "Dr. David Alan Gilbert" , "Michael R. Hines" , Gonglei , Yang Hongyang This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --IeHDGbjBTpln4BirIBgLn9FjqJCM6QpBD Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/02/2015 02:51 AM, Wen Congyang wrote: > If the child is not ready, read/write/getlength/flush will > return -errno. It is not critical error, and can be ignored: > 1. read/write: > Just not report the error event. What happens if all the children report an error? Or is the threshold at play here? For example, if you have a threshold of 3/5, then I'm assuming that if up to two children return an errno, then it is okay to ignore; but if three or more return an errno, you haven't met threshold, so the I/O must fail. Are you ignoring ALL errors (including things like EACCES), or just EIO errors? > 2. getlength: > just ignore it. If all children's getlength return -errno, > and be ignored, return -EIO. > 3. flush: > Just ignore it. If all children's getlength return -errno, s/getlength/flush/ > and be ignored, return 0. Yuck - claiming success when all of the children fail feels dangerous. >=20 > Usage: children.x.ignore-errors=3Dtrue >=20 > Signed-off-by: Wen Congyang > Signed-off-by: zhanghailiang > Signed-off-by: Gonglei > Cc: Alberto Garcia > --- > block/quorum.c | 94 ++++++++++++++++++++++++++++++++++++++++++++= ++++---- Interface review only: > +++ b/qapi/block-core.json > @@ -1411,6 +1411,8 @@ > # @allow-write-backing-file: #optional whether the backing file is ope= ned in > # read-write mode. It is only for backing f= ile > # (Since 2.5 default: false) > +# @ignore-errors: #options whether the child's I/O error should be ign= ored. s/options/optional/ s/error/errors/ > +# it is only for quorum's child.(Since 2.5 default: fa= lse) Space after '.' in English sentences. The fact that you are documenting that this option can only be specified for quorum children makes me wonder if it belongs better as an option in BlockdevOptionsQuorum rather than BlockdevOptionsBase. Semantically, it sounds like you are trying to allow for a per-child decision of whether this particular child's errors matter to the overall quorum. So, if we have a 3/5 quorum, we can decide that for children A, B, C, and D, errors cannot be ignored, but for child E, errors are not a problem. As written, you are tying the semantics to each child BDS, and requiring special code to double-check that the property is only ever set if the BDS is used as the child of a quorum. Furthermore, if the property is set, you are then changing what the child does in response to various operations. What if you instead create a list property in the quorum parent? Maybe along the lines of: # @child-errors-okay: #optional an array of named-node children where errors will be ignored (Since 2.5, default empty) { 'struct': 'BlockdevOptionsQuorum', 'data': { '*blkverify': 'bool', 'children': [ 'BlockdevRef' ], 'vote-threshold': 'int', '*rewrite-corrupted': 'bool', '*read-pattern': 'QuorumReadPattern', '*child-errors-okay': ['str'] } } The above example of a 3/5 quorum, where only child E can ignore errors, would then be represented as: { "children": [ "A", "B", "C", "D", "E" ], 'vote-threshold':3, 'child-errors-okay': [ "E" ] } The code to ignore the errors is then done in the quorum itself (the BDS for E does not have to care about a special ignore-errors property, but just always returns the error as usual; and then the quorum is deciding how to handle the error), and you are not polluting the BDS state for something that is quorum-specific, because it is now the quorum itself that tracks the special casing. Finally, why can't hot-plug/unplug of quorum members work? If you are going to always ignore errors from a particular child, then why is that child even part of the quorum? Isn't a better design to just not add the child to the quorum until it is ready and won't be reporting errors? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --IeHDGbjBTpln4BirIBgLn9FjqJCM6QpBD Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJV5yQiAAoJEKeha0olJ0Nq5l4H+QGfz2TJyzQENuB9nS11ozu4 7pYv27rxgLGTVFY9iP3vIdXP4RwnT7qea7Lr5XhLFSjSUK0Uo7mJ9J41qnctM7su dgLz2q7eU+pzFRr8xGMBK9sooKlsih8XrHQslFTNnTCLuMabMaS1gib1zd58cJ5I 41sMt5VuMZGj1Y/7mUvVwedZfA5L9zi20ZtE0jv5rWFrElZ7f7aFJi6Nj4nm3S8v k6GRNBrHJCqguYPu0mTmQ4s8SKifJITyN/4uTH8IyqSAeC0FAh6I9dHbGowN4l4R MihmaVSncYd0RAsUqRn29fxPmf1KIMErXw5spnWtfgfbbIHt8XmYodrgfPElnpM= =OtCu -----END PGP SIGNATURE----- --IeHDGbjBTpln4BirIBgLn9FjqJCM6QpBD--