From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44875) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1URqGd-00032U-UY for qemu-devel@nongnu.org; Mon, 15 Apr 2013 16:44:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1URqGc-000409-LP for qemu-devel@nongnu.org; Mon, 15 Apr 2013 16:44:47 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49113) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1URqGc-0003zz-D6 for qemu-devel@nongnu.org; Mon, 15 Apr 2013 16:44:46 -0400 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r3FKijOT010211 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 15 Apr 2013 16:44:45 -0400 Message-ID: <516C66BC.8070101@redhat.com> Date: Mon, 15 Apr 2013 14:44:44 -0600 From: Eric Blake MIME-Version: 1.0 References: <1365799688-19918-1-git-send-email-kwolf@redhat.com> <1365799688-19918-14-git-send-email-kwolf@redhat.com> In-Reply-To: <1365799688-19918-14-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="----enig2BGLTOECNFMUTDULXWRJN" Subject: Re: [Qemu-devel] [PATCH 13/15] vvfat: Use bdrv_open options instead of filename List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) ------enig2BGLTOECNFMUTDULXWRJN Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 04/12/2013 02:48 PM, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf > --- > block/vvfat.c | 210 +++++++++++++++++++++++++++++++++++++++++++-------= -------- > 1 file changed, 158 insertions(+), 52 deletions(-) >=20 > + if (!strstart(filename, "fat:", NULL)) { > + error_setg(errp, "File name string must start with 'fat:'"); > + return; > + } > + > + /* Parse options */ > + if (strstr(filename, ":32:")) { > + fat_type =3D 32; > + } else if (strstr(filename, ":16:")) { > + fat_type =3D 16; > + } else if (strstr(filename, ":12:")) { > + fat_type =3D 12; > + } > + > + if (strstr(filename, ":floppy:")) { > + floppy =3D true; > + } > + > + if (strstr(filename, ":rw:")) { > + rw =3D true; > + } > + > + /* Get the directory name without options */ > + i =3D strrchr(filename, ':') - filename; This parser is rather loose, in that it ignores unknown options (for example, if I did fat:1:file, it would happily ignore option "1"). But that's no worse than the old code. > + switch (s->fat_type) { > + case 32: > + fprintf(stderr, "Big fat greek warning: FAT32 has not been tested= =2E " > + "You are welcome to do so!\n"); Should we s/greek/Greek/ in the message, or otherwise change its contents? But you just moved the pre-existing choice of spelling, so it doesn't reflect on you. > @@ -2868,8 +2973,9 @@ static void vvfat_close(BlockDriverState *bs) > static BlockDriver bdrv_vvfat =3D { > .format_name =3D "vvfat", > .instance_size =3D sizeof(BDRVVVFATState), > - .bdrv_file_open =3D vvfat_open, > - .bdrv_rebind =3D vvfat_rebind, > + .bdrv_parse_filename =3D vvfat_parse_filename, > + .bdrv_file_open =3D vvfat_open, > + .bdrv_rebind =3D vvfat_rebind, > .bdrv_read =3D vvfat_co_read, > .bdrv_write =3D vvfat_co_write, > .bdrv_close =3D vvfat_close, Back to my comments on 9/15 on indenting the callbacks consistently. No major errors, so I'm fine if you use: Reviewed-by: Eric Blake --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org ------enig2BGLTOECNFMUTDULXWRJN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.13 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJRbGa8AAoJEKeha0olJ0Nqu0wH/3Br2XXNJZIF9eggTF/R/yWs +Sx90IkNreG/JHc4X0edmcIZRZBaPQ57BWbA0CzDsGwD6xupudeIMCyecwpOZ0yA ohEV8UQSpS92uEexfZz0kreZApfWoyt64ku829zBMrQYJIgAIVp4W/jfWpVooPol S7VDT8QlQfcBHt3pXA6fzRSX/EBr/okPas09hmW0++n8uJF3h9Mh2ddcDFHqqcrR wzNSh/2XVgWhupt8YgvDal6+yVnt/2ROFyuZAjxDx7BrERoL57qyymXa36ahLGne BrSVR8EMPtZYuxlkrEkdbqAWLIcalOO+3JbRhdiZPbB4GCsxQqeku0dVKyqGeqA= =BqCk -----END PGP SIGNATURE----- ------enig2BGLTOECNFMUTDULXWRJN--