From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51298) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFOJe-0007fB-Rq for qemu-devel@nongnu.org; Mon, 17 Feb 2014 08:33:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WFOJX-000501-J4 for qemu-devel@nongnu.org; Mon, 17 Feb 2014 08:32:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54424) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WFOJX-0004zw-23 for qemu-devel@nongnu.org; Mon, 17 Feb 2014 08:32:51 -0500 Date: Mon, 17 Feb 2014 21:31:56 +0800 From: Fam Zheng Message-ID: <20140217133156.GF1428@T430.redhat.com> References: <1390972061-26560-1-git-send-email-famz@redhat.com> <1390972061-26560-6-git-send-email-famz@redhat.com> <20140213124921.GD4547@irqsave.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140213124921.GD4547@irqsave.net> 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: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: kwolf@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, rjones@redhat.com, imain@redhat.com, stefanha@redhat.com, pbonzini@redhat.com On Thu, 02/13 13:49, Beno=EEt Canet wrote: > 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 *bac= king_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->= filename); >=20 >=20 > > + pstrcpy(bs->backing_format, sizeof(bs->backing_file), >=20 > I don't understand the sizeof(bs->backing_file). Is it sizeof(bs->backi= ng_format) ? >=20 Yes. > > + 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, opt= ions, > > 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 c= onfict > with Max latests bdrv_open patches. > You probably need to rebase and get rid of this bdrv_unref() : check Ma= x series > to be sure. I'll check it. >=20 > > 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 = char *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 *bac= king_hd); > > int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Err= or **errp); > > int bdrv_open(BlockDriverState *bs, const char *filename, QDict *opt= ions, > > int flags, BlockDriver *drv, Error **errp); > > --=20 > > 1.8.5.3 > >=20 > >=20 >=20 > bdrv_set_backing_hd seems to be handy since some people want to dynamic= ally > change backing_file using QMP to move them from slow storage to fast st= orage and > the reverse. >=20 Thanks, Fam