From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46017) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axXwx-00086W-FI for qemu-devel@nongnu.org; Tue, 03 May 2016 06:53:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axXwL-00016k-HE for qemu-devel@nongnu.org; Tue, 03 May 2016 06:53:02 -0400 References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-4-git-send-email-mreitz@redhat.com> <20160502153603.GI4882@noname.redhat.com> From: Max Reitz Message-ID: <5728828F.5070809@redhat.com> Date: Tue, 3 May 2016 12:50:55 +0200 MIME-Version: 1.0 In-Reply-To: <20160502153603.GI4882@noname.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Isc9LoFBApdDEQRX5wukeBwxOCwWiJvKt" Subject: Re: [Qemu-devel] [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --Isc9LoFBApdDEQRX5wukeBwxOCwWiJvKt Content-Type: multipart/mixed; boundary="EOhhRJ5ouR6tT9Ndik1U1OvODtVoGWFW8" From: Max Reitz To: Kevin Wolf Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Alberto Garcia , Eric Blake Message-ID: <5728828F.5070809@redhat.com> Subject: Re: [PATCH 03/19] block: Respect backing bs in bdrv_refresh_filename References: <1461706338-20219-1-git-send-email-mreitz@redhat.com> <1461706338-20219-4-git-send-email-mreitz@redhat.com> <20160502153603.GI4882@noname.redhat.com> In-Reply-To: <20160502153603.GI4882@noname.redhat.com> --EOhhRJ5ouR6tT9Ndik1U1OvODtVoGWFW8 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 02.05.2016 17:36, Kevin Wolf wrote: > Am 26.04.2016 um 23:32 hat Max Reitz geschrieben: >> Basically, bdrv_refresh_filename() should respect all children of a >> BlockDriverState. However, generally those children are driver-specifi= c, >> so this function cannot handle the general case. On the other hand, >> there are only few drivers which use other children than @file and >> @backing (that being vmdk, quorum, and blkverify). >> >> Most block drivers only use @file and/or @backing (if they use any >> children at all). Both can be implemented directly in >> bdrv_refresh_filename. >> >> The user overriding the file's filename is already handled, however, t= he >> user overriding the backing file is not. If this is done, opening the >> BDS with the plain filename of its file will not be correct, so we may= >> not set bs->exact_filename in that case. >> >> iotest 051 contains test cases for overwriting the backing file, and s= o >> its output changes with this patch applied (which I consider a good >> thing). >> >> Signed-off-by: Max Reitz >> --- >> block.c | 14 +++++++++++++- >> tests/qemu-iotests/051.out | 8 ++++---- >> tests/qemu-iotests/051.pc.out | 8 ++++---- >> 3 files changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/block.c b/block.c >> index e178488..aff9ea3 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3925,6 +3925,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)= >> =20 >> opts =3D qdict_new(); >> has_open_options =3D append_open_options(opts, bs); >> + has_open_options |=3D bs->backing_overridden; >> =20 >> /* If no specific options have been given for this BDS, the f= ilename of >> * the underlying file should suffice for this one as well */= >> @@ -3936,13 +3937,24 @@ void bdrv_refresh_filename(BlockDriverState *b= s) >> * file BDS. The full options QDict of that file BDS should s= omehow >> * contain a representation of the filename, therefore the fo= llowing >> * suffices without querying the (exact_)filename of this BDS= =2E */ >> - if (bs->file->bs->full_open_options) { >> + if (bs->file->bs->full_open_options && >> + (!bs->backing || bs->backing->bs->full_open_options)) >> + { >> qdict_put_obj(opts, "driver", >> QOBJECT(qstring_from_str(drv->format_name))= ); >> + >> QINCREF(bs->file->bs->full_open_options); >> qdict_put_obj(opts, "file", >> QOBJECT(bs->file->bs->full_open_options)); >> =20 >> + if (bs->backing) { >> + QINCREF(bs->backing->bs->full_open_options); >> + qdict_put_obj(opts, "backing", >> + QOBJECT(bs->backing->bs->full_open_opti= ons)); >> + } else if (bs->backing_overridden && !bs->backing) { >> + qdict_put_obj(opts, "backing", QOBJECT(qstring_new())= ); >> + } >=20 > I see that the surrounding code does the same, but what's wrong with > qdict_put()? That it's a macro. clang_complete does not tell me about macros. Will fix and try to keep in mind in the future, thanks. Max --EOhhRJ5ouR6tT9Ndik1U1OvODtVoGWFW8-- --Isc9LoFBApdDEQRX5wukeBwxOCwWiJvKt 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 iQEcBAEBCAAGBQJXKIKPAAoJEDuxQgLoOKytROIH/jQOMXRAfeK8TMIF/J5i4xR9 C1UkY77W1BXesWxuHgQmVVHi8JL5wBYxPpwFQ9NP2+6M7rOiCtpHTOlQsLikzq+I n8xF53ljvFdRQhlNwEIM+By3FBnHwYhoPFCUqwHht5uY+q1a2S+t9GSfGdRhD6Sb QWJ7DmBbQCVMXXfRWArWZ1YFEZx5ctECdbJJOeNHrn6IKGMscwiLj30QvYxaHeVm 46O6uaX/df3t1gyLxXk74VlNJAqvjsxqPILkk3N/7QWXhYcjeGsgGldeayiltQAy +AyDxhdnOCjuAxV0vNNlBlT02GzLkwJ9d8y8f2PhFYN2QRLwwsN6T/MeSGPLV64= =dzMC -----END PGP SIGNATURE----- --Isc9LoFBApdDEQRX5wukeBwxOCwWiJvKt--