From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WuFIF-0000qi-4B for qemu-devel@nongnu.org; Tue, 10 Jun 2014 02:12:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WuFI7-0001Tq-HQ for qemu-devel@nongnu.org; Tue, 10 Jun 2014 02:12:23 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:46486 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WuFI7-0001TF-8u for qemu-devel@nongnu.org; Tue, 10 Jun 2014 02:12:15 -0400 Date: Tue, 10 Jun 2014 08:12:12 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140610061212.GA32204@irqsave.net> References: <1402346762-21333-1-git-send-email-benoit.canet@irqsave.net> <1402346762-21333-3-git-send-email-benoit.canet@irqsave.net> <5396224D.9070504@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <5396224D.9070504@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v7 2/3] block: Add node-name and to-replace-node-name arguments to drive-mirror List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , kwolf@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, mreitz@redhat.com The Monday 09 Jun 2014 =E0 15:08:29 (-0600), Eric Blake wrote : > On 06/09/2014 02:46 PM, Beno=EEt Canet wrote: > > node-name gives a name to the created BDS and registers it in the nod= e graph. > >=20 > > to-replace-node-name can be used when drive-mirror is called with syn= c=3Dfull. >=20 > Why can't it work with other modes? That is, if I have: >=20 > base1 <- snap1 \ > base2 <- snap2 > quorum > base3 <- snap3 / >=20 > and want to replace the 'base3 <- snap3' arm of the quorum with 'base4 > <- snap4', where base3 and base4 are identical, the fact that you are > forcing sync=3Dfull will not let me do so. There's a lot of things whe= re > if management does something stupid, then the guest will see data > instantly corrupted; but that doesn't mean that we necessarily have to > cripple the power of the command. I am affraid that the user could form loop in the graph. >=20 > >=20 > > The purpose of these fields is to be able to reconstruct and replace = a broken > > quorum file. > >=20 > > drive-mirror will bdrv_swap the new BDS named node-name with the one > > pointed by to-replace-node-name when the mirroring is finished. > >=20 > > Signed-off-by: Benoit Canet > > --- >=20 > > + > > + if (size < bdrv_getlength(to_replace_bs)) { > > + error_setg(errp, "cannot replace to-replace-node-name im= age with a " > > + "mirror image that would be smaller in = size"); >=20 > Should this be enforcing equality in size, rather than just prohibiting > shrinking? Doesn't the quorum code already require that all quorum > members have equal size? I though that quorum was still returning the smallest image size for getl= ength and that it would be a way to grow the quorum by replacing with bigger im= age. But it's not the case I will enforce equality in size. >=20 >=20 > > =20 > > + /* if we are planning to replace a graph node name the code shou= ld do a full > > + * mirror of the source image > > + */ > > + if (has_to_replace_node_name && sync !=3D MIRROR_SYNC_MODE_FULL)= { > > + error_setg(errp, > > + "to-replace-node-name can only be used with sync=3D= full"); > > + return; > > + } >=20 > Again, I'm not sure if this restriction makes sense. >=20 > > +++ b/qapi/block-core.json > > @@ -769,6 +769,14 @@ > > # @format: #optional the format of the new destination, default is t= o > > # probe if @mode is 'existing', else the format of the sour= ce > > # > > +# @new-node-name: #optional the new block driver state node name in = the graph > > +# (Since 2.1) >=20 > Is it worth splitting this patch into two? The ability to name the new > node of a drive-mirror makes sense as an independent patch, which might > be applied sooner even while worrying about the semantics of how > replacement will work. ok >=20 > > +# > > +# @to-replace-node-name: #optional with sync=3Dfull graph node name = to be > > +# replaced by the new image when a whole imag= e copy is > > +# done. This can be used to repair broken Quo= rum files. > > +# (Since 2.1) >=20 > So if I understand correctly, the point of this command is that if I > have a quorum with three backing named nodes, and want to hotswap out > one of those modes, then I create a drive-mirror that names which of th= e > three nodes is the victim, and on completion, the quorum now has the > remaining two nodes and my new mirror as its new three node setup. >=20 > Am I correct that to-replace-node-name can only be used on a node that > is already composed of other nodes, and that the replacement must be on= e > of those nodes? >=20 > What if I have a 3/5 quorum - can I replace 2 nodes at once? No you can't. The first drive-mirror will lock the quorum device. >=20 > > +# > > # @mode: #optional whether and how QEMU should create a new image, d= efault is > > # 'absolute-paths'. > > # > > @@ -801,6 +809,7 @@ > > ## > > { 'command': 'drive-mirror', > > 'data': { 'device': 'str', 'target': 'str', '*format': 'str', > > + '*new-node-name': 'str', '*to-replace-node-name': 'str', >=20 > Bikeshedding: those are some long names. Is it sufficient to go with > something shorter, '*node-name' for what to name the new mirror (again, > might be worth splitting that into its own patch), and '*replaces' for > the name of a node to be replaced? ok >=20 > --=20 > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >=20