From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46814) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bCnwb-0007NH-2p for qemu-devel@nongnu.org; Tue, 14 Jun 2016 08:59:50 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bCnwZ-00052d-1k for qemu-devel@nongnu.org; Tue, 14 Jun 2016 08:59:48 -0400 References: <20160610185750.30956-1-mreitz@redhat.com> <20160610185750.30956-4-mreitz@redhat.com> <20160612040850.GG27167@ad.usersys.redhat.com> From: Max Reitz Message-ID: <4809f891-a690-a312-12c9-1448e43779de@redhat.com> Date: Tue, 14 Jun 2016 14:59:35 +0200 MIME-Version: 1.0 In-Reply-To: <20160612040850.GG27167@ad.usersys.redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="i0qlnP2xkUwuRTDM1Cw4dpfh9V1pNSbcf" Subject: Re: [Qemu-devel] [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Kevin Wolf This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --i0qlnP2xkUwuRTDM1Cw4dpfh9V1pNSbcf From: Max Reitz To: Fam Zheng Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, Kevin Wolf Message-ID: <4809f891-a690-a312-12c9-1448e43779de@redhat.com> Subject: Re: [PATCH v3 3/5] block/null: Implement bdrv_refresh_filename() References: <20160610185750.30956-1-mreitz@redhat.com> <20160610185750.30956-4-mreitz@redhat.com> <20160612040850.GG27167@ad.usersys.redhat.com> In-Reply-To: <20160612040850.GG27167@ad.usersys.redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 12.06.2016 06:08, Fam Zheng wrote: > On Fri, 06/10 20:57, Max Reitz wrote: >> Signed-off-by: Max Reitz >=20 > The commit message could go a little more informative. Seems nothing sp= ecial in > the added callback, aren't things supposed to just work without this pa= tch? > What is missing? If you pass a filename, it works without this patch. If you don't, it doesn't. Compare (before this patch): $ x86_64-softmmu/qemu-system-x86_64 \ -drive if=3Dnone,file=3Dnull-co://,driver=3Draw -nodefaults -qmp stdio [...] {'execute':'query-block'} [...] "filename": "null-co://", [...] With: $ x86_64-softmmu/qemu-system-x86_64 \ -drive if=3Dnone,driver=3Draw,file.driver=3Dnull-co -nodefaults -qmp st= dio [...] {'execute':'query-block'} [...] "filename": "json:{\"driver\": \"raw\", \"file\": {\"driver\": \"null-co\"}}", [...] So the issue is that you can use the null-co/aio drivers without giving a filename. I wouldn't mind including this information in a v4, but I'm not sure it alone warrants a v4. >=20 >> --- >> block/null.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> >> diff --git a/block/null.c b/block/null.c >> index 396500b..b511010 100644 >> --- a/block/null.c >> +++ b/block/null.c >> @@ -12,6 +12,8 @@ >> =20 >> #include "qemu/osdep.h" >> #include "qapi/error.h" >> +#include "qapi/qmp/qdict.h" >> +#include "qapi/qmp/qstring.h" >> #include "block/block_int.h" >> =20 >> #define NULL_OPT_LATENCY "latency-ns" >> @@ -223,6 +225,20 @@ static int64_t coroutine_fn null_co_get_block_sta= tus(BlockDriverState *bs, >> } >> } >> =20 >> +static void null_refresh_filename(BlockDriverState *bs, QDict *opts) >> +{ >> + QINCREF(opts); >> + qdict_del(opts, "filename"); >=20 > Why is this qdict_del necessary? It's not strictly necessary. But the null drivers ignore the filename, so there's no harm in dropping it from the JSON filename (if we need to construct one). Max >> + >> + if (!qdict_size(opts)) { >> + snprintf(bs->exact_filename, sizeof(bs->exact_filename), "%s:= //", >> + bs->drv->format_name); >> + } >> + >> + qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name))= ; >> + bs->full_open_options =3D opts; >> +} >> + >> static BlockDriver bdrv_null_co =3D { >> .format_name =3D "null-co", >> .protocol_name =3D "null-co", >> @@ -238,6 +254,8 @@ static BlockDriver bdrv_null_co =3D { >> .bdrv_reopen_prepare =3D null_reopen_prepare, >> =20 >> .bdrv_co_get_block_status =3D null_co_get_block_status, >> + >> + .bdrv_refresh_filename =3D null_refresh_filename, >> }; >> =20 >> static BlockDriver bdrv_null_aio =3D { >> @@ -255,6 +273,8 @@ static BlockDriver bdrv_null_aio =3D { >> .bdrv_reopen_prepare =3D null_reopen_prepare, >> =20 >> .bdrv_co_get_block_status =3D null_co_get_block_status, >> + >> + .bdrv_refresh_filename =3D null_refresh_filename, >> }; >> =20 >> static void bdrv_null_init(void) >> --=20 >> 2.8.3 >> --i0qlnP2xkUwuRTDM1Cw4dpfh9V1pNSbcf 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 iQEcBAEBCAAGBQJXX/+3AAoJEDuxQgLoOKytg+QIAIXsFWOVGqDKI6zzOPlfeoPq NKADiDhk9mHWsTJy9F5dXElKwBZfU2rhGHxZEtaIAab0CNGxNasS74Nnzd4ufgGB h3+kPTRSzfY677lJrVrL1riwkT1kT+mSuHzPRHml1w8di0MtjvcqiGQaExCGzuSh GhSD7cOaw6iEkL+0Vv1mxJugjKb4MN2DXsijbelo8ifVnCJHJT1ERbQ90YHDbslk looJOX4yftVhlHqh8tePB2QUAhvDxV6j7NUFDQWLSAIwd3VwjrVrFS5PNIdGix7y oW9OTm+MhPTpSDOHH1VE5CxWwXucNUG7bj8NJUM1gScqqUeDlNRGy3UVfbjbHqM= =kxaR -----END PGP SIGNATURE----- --i0qlnP2xkUwuRTDM1Cw4dpfh9V1pNSbcf--