From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41522) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkuTY-0007B9-SE for qemu-devel@nongnu.org; Thu, 15 May 2014 08:09:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WkuTN-0004DB-71 for qemu-devel@nongnu.org; Thu, 15 May 2014 08:09:28 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:37643 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WkuTM-0004Cy-UE for qemu-devel@nongnu.org; Thu, 15 May 2014 08:09:17 -0400 Date: Thu, 15 May 2014 14:09:50 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140515120950.GG2812@irqsave.net> References: <13ae3f12913b517c7f2e5e356b54b6bf872045da.1400123059.git.jcody@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <13ae3f12913b517c7f2e5e356b54b6bf872045da.1400123059.git.jcody@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block-commit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, benoit.canet@irqsave.net, pkrempa@redhat.com, famz@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com The Wednesday 14 May 2014 =E0 23:20:18 (-0400), Jeff Cody wrote : > This modifies the block operation block-commit so that it will > accept node-name arguments for either 'top' or 'base' BDS. >=20 > The filename and node-name are mutually exclusive to each other; > i.e.: > "top" and "top-node-name" are mutually exclusive (enforced) > "base" and "base-node-name" are mutually exclusive (enforced) >=20 > The preferred and recommended method now of using block-commit > is to specify the BDS to operate on via their node-name arguments. >=20 > This also adds an explicit check that base_bs is in the chain of > top_bs, because if they are referenced by their unique node-name > arguments, the discovery process does not intrinsically verify > they are in the same chain. >=20 > Signed-off-by: Jeff Cody > --- > blockdev.c | 35 +++++++++++++++++++++++++++++++++-- > qapi-schema.json | 35 ++++++++++++++++++++++++++--------- > qmp-commands.hx | 29 ++++++++++++++++++++++------- > 3 files changed, 81 insertions(+), 18 deletions(-) >=20 > diff --git a/blockdev.c b/blockdev.c > index 02c6525..d8cb396 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -1869,7 +1869,9 @@ void qmp_block_stream(const char *device, bool ha= s_base, > =20 > void qmp_block_commit(const char *device, > bool has_base, const char *base, > + bool has_base_node_name, const char *base_node_n= ame, > bool has_top, const char *top, > + bool has_top_node_name, const char *top_node_nam= e, > bool has_speed, int64_t speed, > Error **errp) > { > @@ -1888,6 +1890,15 @@ void qmp_block_commit(const char *device, > /* drain all i/o before commits */ > bdrv_drain_all(); > =20 > + if (has_base && has_base_node_name) { > + error_setg(errp, "'base' and 'base-node-name' are mutually exc= lusive"); > + return; > + } > + if (has_top && has_top_node_name) { > + error_setg(errp, "'top' and 'top-node-name' are mutually exclu= sive"); > + return; > + } > + > bs =3D bdrv_find(device); > if (!bs) { > error_set(errp, QERR_DEVICE_NOT_FOUND, device); > @@ -1897,7 +1908,14 @@ void qmp_block_commit(const char *device, > /* default top_bs is the active layer */ > top_bs =3D bs; > =20 > - if (top) { > + /* Find the 'top' image file for the commit */ > + if (has_top_node_name) { > + top_bs =3D bdrv_lookup_bs(NULL, top_node_name, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + } else if (has_top && top) { > if (strcmp(bs->filename, top) !=3D 0) { > top_bs =3D bdrv_find_backing_image(bs, top); > } > @@ -1908,7 +1926,14 @@ void qmp_block_commit(const char *device, > return; > } > =20 > - if (has_base && base) { > + /* Find the 'base' image file for the commit */ > + if (has_base_node_name) { > + base_bs =3D bdrv_lookup_bs(NULL, base_node_name, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + } else if (has_base && base) { > base_bs =3D bdrv_find_backing_image(top_bs, base); > } else { > base_bs =3D bdrv_find_base(top_bs); > @@ -1919,6 +1944,12 @@ void qmp_block_commit(const char *device, > return; > } > =20 > + /* Verify that 'base' is in the same chain as 'top' */ > + if (!bdrv_is_in_chain(top_bs, base_bs)) { > + error_setg(errp, "'base' and 'top' are not in the same chain")= ; > + return; > + } > + > if (top_bs =3D=3D bs) { > commit_active_start(bs, base_bs, speed, on_error, block_job_cb= , > bs, &local_err); > diff --git a/qapi-schema.json b/qapi-schema.json > index 06a9b5d..eddb2b8 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -2096,12 +2096,27 @@ > # > # @device: the name of the device > # > -# @base: #optional The file name of the backing image to write data = into. > -# If not specified, this is the deepest backing ima= ge > +# For 'base', either @base or @base-node-name may be set but not both.= If > +# neither is specified, this is the deepest backing image > # > -# @top: #optional The file name of the backing image within the ima= ge chain, > -# which contains the topmost data to be committed d= own. If > -# not specified, this is the active layer. > +# @base: #optional The file name of the backing image to writ= e data > +# into. > +# > +# @base-node-name #optional The name of the block driver state node of= the > +# backing image to write data into. > +# (Since 2.1) > +# > +# For 'top', either @top or @top-node-name must be set but not both. > +# > +# @top: #optional The file name of the backing image within = the image > +# chain, which contains the topmost data to = be > +# committed down. If not specified, this is = the > +# active layer. > +# > +# @top-node-name: #optional The block driver state node name of the ba= cking > +# image within the image chain, which contai= ns the > +# topmost data to be committed down. > +# (Since 2.1) > # > # If top =3D=3D base, that is an error. > # If top =3D=3D active, the job will not be complet= ed by itself, > @@ -2120,17 +2135,19 @@ > # > # Returns: Nothing on success > # If commit or stream is already active on this device, Devic= eInUse > -# If @device does not exist, DeviceNotFound > +# If @device does not exist or cannot be determined, DeviceNo= tFound > # If image commit is not supported by this device, NotSupport= ed > -# If @base or @top is invalid, a generic error is returned > +# If @base, @top, @base-node-name, @top-node-name invalid, Ge= nericError > # If @speed is invalid, InvalidParameter > +# If both @base and @base-node-name are specified, GenericErr= or > +# If both @top and @top-node-name are specified, GenericError > # > # Since: 1.3 > # > ## > { 'command': 'block-commit', > - 'data': { 'device': 'str', '*base': 'str', '*top': 'str', > - '*speed': 'int' } } > + 'data': { 'device': 'str', '*base': 'str', '*base-node-name': 'str', > + '*top': 'str', '*top-node-name': 'str', '*speed': 'int' } = } > =20 > ## > # @drive-backup > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 1aa3c65..2b9b1dc 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -985,7 +985,7 @@ EQMP > =20 > { > .name =3D "block-commit", > - .args_type =3D "device:B,base:s?,top:s?,speed:o?", > + .args_type =3D "device:B,base:s?,base-node-name:s?,top:s?,top= -node-name:s?,speed:o?", > .mhandler.cmd_new =3D qmp_marshal_input_block_commit, > }, > =20 > @@ -999,12 +999,27 @@ data between 'top' and 'base' into 'base'. > Arguments: > =20 > - "device": The device's ID, must be unique (json-string) > -- "base": The file name of the backing image to write data into. > - If not specified, this is the deepest backing image > - (json-string, optional) > -- "top": The file name of the backing image within the image chain, > - which contains the topmost data to be committed down. If > - not specified, this is the active layer. (json-string, optio= nal) > + > +For 'base', either base or base-node-name may be set but not both. If > +neither is specified, this is the deepest backing image > + > +- "base": The file name of the backing image to write data i= nto. > + (json-string, optional) > +- "base-node-name": The name of the block driver state node of the > + backing image to write data into. > + (json-string, optional) (Since 2.1) > + > +For 'top', either top or top-node-name must be set but not both. > + > +- "top": The file name of the backing image within the imag= e chain, > + which contains the topmost data to be committed do= wn. If > + not specified, this is the active layer. > + (json-string, optional) > + > +- "top-node-name": The block driver state node name of the backing > + image within the image chain, which contains the > + topmost data to be committed down. > + (json-string, optional) (Since 2.1) ^ (cosmetic) this block is not aligned with the= upper ones. > =20 > If top =3D=3D base, that is an error. > If top =3D=3D active, the job will not be completed by itsel= f, > --=20 > 1.8.3.1 >=20