[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/15] vvfat: Use bdrv_open options instead of f
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 13/15] vvfat: Use bdrv_open options instead of filename |
Date: |
Mon, 15 Apr 2013 14:44:44 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130402 Thunderbird/17.0.5 |
On 04/12/2013 02:48 PM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> 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 <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH 08/15] curl: Use bdrv_open options instead of filename, (continued)
- [Qemu-devel] [PATCH 09/15] gluster: Use bdrv_open options instead of filename, Kevin Wolf, 2013/04/12
- [Qemu-devel] [PATCH 10/15] iscsi: Use bdrv_open options instead of filename, Kevin Wolf, 2013/04/12
- [Qemu-devel] [PATCH 11/15] rbd: Use bdrv_open options instead of filename, Kevin Wolf, 2013/04/12
- [Qemu-devel] [PATCH 12/15] sheepdog: Use bdrv_open options instead of filename, Kevin Wolf, 2013/04/12
- [Qemu-devel] [PATCH 13/15] vvfat: Use bdrv_open options instead of filename, Kevin Wolf, 2013/04/12
- Re: [Qemu-devel] [PATCH 13/15] vvfat: Use bdrv_open options instead of filename,
Eric Blake <=
- [Qemu-devel] [PATCH 14/15] block: Remove filename parameter from .bdrv_file_open(), Kevin Wolf, 2013/04/12
- [Qemu-devel] [PATCH 15/15] block: Allow overriding backing.file.filename, Kevin Wolf, 2013/04/12
- Re: [Qemu-devel] [PATCH 00/15] block: Overriding the backing file with -drive, Stefan Hajnoczi, 2013/04/18