On 04/12/2013 02:48 PM, Kevin Wolf wrote: > Signed-off-by: Kevin Wolf <kw...@redhat.com> > --- > block/vvfat.c | 210 > +++++++++++++++++++++++++++++++++++++++++++--------------- > 1 file changed, 158 insertions(+), 52 deletions(-) >
> + if (!strstart(filename, "fat:", NULL)) {
> + error_setg(errp, "File name string must start with 'fat:'");
> + return;
> + }
> +
> + /* Parse options */
> + if (strstr(filename, ":32:")) {
> + fat_type = 32;
> + } else if (strstr(filename, ":16:")) {
> + fat_type = 16;
> + } else if (strstr(filename, ":12:")) {
> + fat_type = 12;
> + }
> +
> + if (strstr(filename, ":floppy:")) {
> + floppy = true;
> + }
> +
> + if (strstr(filename, ":rw:")) {
> + rw = true;
> + }
> +
> + /* Get the directory name without options */
> + i = 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. "
> + "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 = {
> .format_name = "vvfat",
> .instance_size = sizeof(BDRVVVFATState),
> - .bdrv_file_open = vvfat_open,
> - .bdrv_rebind = vvfat_rebind,
> + .bdrv_parse_filename = vvfat_parse_filename,
> + .bdrv_file_open = vvfat_open,
> + .bdrv_rebind = vvfat_rebind,
> .bdrv_read = vvfat_co_read,
> .bdrv_write = vvfat_co_write,
> .bdrv_close = 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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
