From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47858) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDvjN-0001xA-4y for qemu-devel@nongnu.org; Thu, 13 Feb 2014 07:49:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WDvjH-0006x2-5D for qemu-devel@nongnu.org; Thu, 13 Feb 2014 07:49:29 -0500 Received: from paradis.irqsave.net ([62.212.105.220]:49923) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WDvjG-0006wv-Js for qemu-devel@nongnu.org; Thu, 13 Feb 2014 07:49:23 -0500 Date: Thu, 13 Feb 2014 13:49:21 +0100 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140213124921.GD4547@irqsave.net> References: <1390972061-26560-1-git-send-email-famz@redhat.com> <1390972061-26560-6-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1390972061-26560-6-git-send-email-famz@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v13 05/14] block: Add bdrv_set_backing_hd() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, rjones@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, imain@redhat.com, stefanha@redhat.com, pbonzini@redhat.com The Wednesday 29 Jan 2014 =E0 13:07:32 (+0800), Fam Zheng wrote : > This is the common but non-trivial steps to assign or change the > backing_hd of BDS. >=20 > Signed-off-by: Fam Zheng > --- > block.c | 34 +++++++++++++++++++++++++--------- > include/block/block.h | 1 + > 2 files changed, 26 insertions(+), 9 deletions(-) >=20 > diff --git a/block.c b/block.c > index 8000ac0..c4eaa37 100644 > --- a/block.c > +++ b/block.c > @@ -1072,6 +1072,26 @@ fail: > return ret; > } > =20 > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backi= ng_hd) > +{ > + if (bs->backing_hd) { > + bdrv_unref(bs->backing_hd); > + } > + > + bs->backing_hd =3D backing_hd; > + if (!backing_hd) { > + bs->backing_file[0] =3D '\0'; > + bs->backing_format[0] =3D '\0'; > + goto out; > + } > + pstrcpy(bs->backing_file, sizeof(bs->backing_file), backing_hd->fi= lename); > + pstrcpy(bs->backing_format, sizeof(bs->backing_file), I don't understand the sizeof(bs->backing_file). Is it sizeof(bs->backing= _format) ? > + backing_hd->drv ? backing_hd->drv->format_name : ""); > + bdrv_ref(bs->backing_hd); > +out: > + bdrv_refresh_limits(bs); > +} > + > /* > * Opens the backing file for a BlockDriverState if not yet open > * > @@ -1085,6 +1105,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, = QDict *options, Error **errp) > char backing_filename[PATH_MAX]; > int back_flags, ret; > BlockDriver *back_drv =3D NULL; > + BlockDriverState *backing_hd; > Error *local_err =3D NULL; > =20 > if (bs->backing_hd !=3D NULL) { > @@ -1108,7 +1129,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, = QDict *options, Error **errp) > sizeof(backing_filename)); > } > =20 > - bs->backing_hd =3D bdrv_new(""); > + backing_hd =3D bdrv_new(""); > =20 > if (bs->backing_format[0] !=3D '\0') { > back_drv =3D bdrv_find_format(bs->backing_format); > @@ -1118,23 +1139,18 @@ int bdrv_open_backing_file(BlockDriverState *bs= , QDict *options, Error **errp) > back_flags =3D bs->open_flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | > BDRV_O_COPY_ON_READ); > =20 > - ret =3D bdrv_open(bs->backing_hd, > + ret =3D bdrv_open(backing_hd, > *backing_filename ? backing_filename : NULL, optio= ns, > back_flags, back_drv, &local_err); > if (ret < 0) { > - bdrv_unref(bs->backing_hd); > - bs->backing_hd =3D NULL; > + bdrv_unref(backing_hd); Here I wonder if this way of calling bdrv_open and doing a bdrv_unref con= fict with Max latests bdrv_open patches. You probably need to rebase and get rid of this bdrv_unref() : check Max = series to be sure. > bs->open_flags |=3D BDRV_O_NO_BACKING; > error_setg(errp, "Could not open backing file: %s", > error_get_pretty(local_err)); > error_free(local_err); > return ret; > } > - > - if (bs->backing_hd->file) { > - pstrcpy(bs->backing_file, sizeof(bs->backing_file), > - bs->backing_hd->file->filename); > - } > + bdrv_set_backing_hd(bs, backing_hd); > =20 > /* Recalculate the BlockLimits with the backing file */ > bdrv_refresh_limits(bs); > diff --git a/include/block/block.h b/include/block/block.h > index 9125bbe..f449753 100644 > --- a/include/block/block.h > +++ b/include/block/block.h > @@ -208,6 +208,7 @@ int bdrv_file_open(BlockDriverState **pbs, const ch= ar *filename, > int bdrv_open_image(BlockDriverState **pbs, const char *filename, > QDict *options, const char *bdref_key, int flags, > bool force_raw, bool allow_none, Error **errp); > +void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backi= ng_hd); > int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error= **errp); > int bdrv_open(BlockDriverState *bs, const char *filename, QDict *optio= ns, > int flags, BlockDriver *drv, Error **errp); > --=20 > 1.8.5.3 >=20 >=20 bdrv_set_backing_hd seems to be handy since some people want to dynamical= ly change backing_file using QMP to move them from slow storage to fast stor= age and the reverse. Best regards Beno=EEt