[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 05/13] vvfat: introduce offset_to_b
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 05/13] vvfat: introduce offset_to_bootsector, offset_to_fat and offset_to_root_dir |
Date: |
Tue, 16 May 2017 16:16:02 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 15.05.2017 um 22:31 hat Hervé Poussineau geschrieben:
> - offset_to_bootsector is the number of sectors up to FAT bootsector
> - offset_to_fat is the number of sectors up to first File Allocation Table
> - offset_to_root_dir is the number of sectors up to root directory sector
Hm... These names make me think of byte offsets. Not completely opposed
to them, but if anyone can think of something better...?
> Replace first_sectors_number - 1 by offset_to_bootsector.
> Replace first_sectors_number by offset_to_fat.
> Replace faked_sectors by offset_to_rootdir.
>
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
> block/vvfat.c | 67
> +++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 40 insertions(+), 27 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 4f4a63c03f..f60d2a3889 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -320,22 +320,24 @@ static void print_mapping(const struct mapping_t*
> mapping);
> typedef struct BDRVVVFATState {
> CoMutex lock;
> BlockDriverState* bs; /* pointer to parent */
> - unsigned int first_sectors_number; /* 1 for a single partition, 0x40 for
> a disk with partition table */
> unsigned char first_sectors[0x40*0x200];
>
> int fat_type; /* 16 or 32 */
> array_t fat,directory,mapping;
> char volume_label[11];
>
> + uint32_t offset_to_bootsector; /* 0 for floppy, 0x3f for disk */
> +
> unsigned int cluster_size;
> unsigned int sectors_per_cluster;
> unsigned int sectors_per_fat;
> unsigned int sectors_of_root_directory;
> uint32_t last_cluster_of_root_directory;
> - unsigned int faked_sectors; /* how many sectors are faked before file
> data */
> uint32_t sector_count; /* total number of sectors of the partition */
> uint32_t cluster_count; /* total number of clusters of this partition */
> uint32_t max_fat_value;
> + uint32_t offset_to_fat;
> + uint32_t offset_to_root_dir;
>
> int current_fd;
> mapping_t* current_mapping;
> @@ -394,15 +396,15 @@ static void init_mbr(BDRVVVFATState *s, int cyls, int
> heads, int secs)
> partition->attributes=0x80; /* bootable */
>
> /* LBA is used when partition is outside the CHS geometry */
> - lba = sector2CHS(&partition->start_CHS, s->first_sectors_number - 1,
> + lba = sector2CHS(&partition->start_CHS, s->offset_to_bootsector,
> cyls, heads, secs);
> lba |= sector2CHS(&partition->end_CHS, s->bs->total_sectors - 1,
> cyls, heads, secs);
>
> /*LBA partitions are identified only by start/length_sector_long not by
> CHS*/
> - partition->start_sector_long = cpu_to_le32(s->first_sectors_number - 1);
> + partition->start_sector_long = cpu_to_le32(s->offset_to_bootsector);
> partition->length_sector_long = cpu_to_le32(s->bs->total_sectors
> - - s->first_sectors_number +
> 1);
> + - s->offset_to_bootsector);
>
> /* FAT12/FAT16/FAT32 */
> /* DOS uses different types when partition is LBA,
> @@ -823,12 +825,12 @@ static int read_directory(BDRVVVFATState* s, int
> mapping_index)
>
> static inline uint32_t sector2cluster(BDRVVVFATState* s,off_t sector_num)
> {
> - return (sector_num-s->faked_sectors)/s->sectors_per_cluster;
> + return (sector_num - s->offset_to_root_dir) / s->sectors_per_cluster;
> }
>
> static inline off_t cluster2sector(BDRVVVFATState* s, uint32_t cluster_num)
> {
> - return s->faked_sectors + s->sectors_per_cluster * cluster_num;
> + return s->offset_to_root_dir + s->sectors_per_cluster * cluster_num;
> }
>
> static int init_directories(BDRVVVFATState* s,
> @@ -855,6 +857,9 @@ static int init_directories(BDRVVVFATState* s,
> i = 1+s->sectors_per_cluster*0x200*8/s->fat_type;
> s->sectors_per_fat=(s->sector_count+i)/i; /* round up */
>
> + s->offset_to_fat = s->offset_to_bootsector + 1;
> + s->offset_to_root_dir = s->offset_to_fat + s->sectors_per_fat * 2;
> +
> array_init(&(s->mapping),sizeof(mapping_t));
> array_init(&(s->directory),sizeof(direntry_t));
>
> @@ -868,7 +873,6 @@ static int init_directories(BDRVVVFATState* s,
> /* Now build FAT, and write back information into directory */
> init_fat(s);
>
> - s->faked_sectors=s->first_sectors_number+s->sectors_per_fat*2;
> s->cluster_count=sector2cluster(s, s->sector_count);
>
> mapping = array_get_next(&(s->mapping));
> @@ -946,7 +950,8 @@ static int init_directories(BDRVVVFATState* s,
>
> s->current_mapping = NULL;
>
> -
> bootsector=(bootsector_t*)(s->first_sectors+(s->first_sectors_number-1)*0x200);
> + bootsector = (bootsector_t *)(s->first_sectors
> + + s->offset_to_bootsector * 0x200);
> bootsector->jump[0]=0xeb;
> bootsector->jump[1]=0x3e;
> bootsector->jump[2]=0x90;
> @@ -957,16 +962,16 @@ static int init_directories(BDRVVVFATState* s,
> bootsector->number_of_fats=0x2; /* number of FATs */
> bootsector->root_entries=cpu_to_le16(s->sectors_of_root_directory*0x10);
>
> bootsector->total_sectors16=s->sector_count>0xffff?0:cpu_to_le16(s->sector_count);
> - bootsector->media_type=(s->first_sectors_number>1?0xf8:0xf0); /* media
> descriptor (f8=hd, f0=3.5 fd)*/
> + bootsector->media_type = (s->offset_to_bootsector > 0 ? 0xf8 : 0xf0);
Please keep the comment. I don't mind if you want to move it to its own
line, but it's better to have magic numbers like 0xf8 and 0xf0 explained
somewhere. (Or you could introduce descriptive #defines for them.)
> s->fat.pointer[0] = bootsector->media_type;
> bootsector->sectors_per_fat=cpu_to_le16(s->sectors_per_fat);
> bootsector->sectors_per_track = cpu_to_le16(secs);
> bootsector->number_of_heads = cpu_to_le16(heads);
> -
> bootsector->hidden_sectors=cpu_to_le32(s->first_sectors_number==1?0:0x3f);
> + bootsector->hidden_sectors = cpu_to_le32(s->offset_to_bootsector);
>
> bootsector->total_sectors=cpu_to_le32(s->sector_count>0xffff?s->sector_count:0);
Nice simplification. :-)
> /* LATER TODO: if FAT32, this is wrong */
> - bootsector->u.fat16.drive_number=s->first_sectors_number==1?0:0x80; /*
> fda=0, hda=0x80 */
> + bootsector->u.fat16.drive_number = s->offset_to_bootsector == 0 ? 0 :
> 0x80;
Here I would like the comment to be preserved again.
> bootsector->u.fat16.current_head=0;
> bootsector->u.fat16.signature=0x29;
> bootsector->u.fat16.id=cpu_to_le32(0xfabe1afd);
> @@ -1123,7 +1128,6 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> secs = s->fat_type == 12 ? 18 : 36;
> s->sectors_per_cluster = 1;
> }
> - s->first_sectors_number = 1;
> cyls = 80;
> heads = 2;
> } else {
> @@ -1131,7 +1135,7 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> if (!s->fat_type) {
> s->fat_type = 16;
> }
> - s->first_sectors_number = 0x40;
> + s->offset_to_bootsector = 0x3f;
> cyls = s->fat_type == 12 ? 64 : 1024;
> heads = 16;
> secs = 63;
> @@ -1169,7 +1173,7 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> fprintf(stderr, "vvfat %s chs %d,%d,%d\n",
> dirname, cyls, heads, secs);
>
> - s->sector_count = cyls * heads * secs - (s->first_sectors_number - 1);
> + s->sector_count = cyls * heads * secs - s->offset_to_bootsector;
>
> if (qemu_opt_get_bool(opts, "rw", false)) {
> ret = enable_write_target(bs, errp);
> @@ -1186,7 +1190,8 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> goto fail;
> }
>
> - s->sector_count = s->faked_sectors +
> s->sectors_per_cluster*s->cluster_count;
> + s->sector_count = s->offset_to_root_dir
> + + s->sectors_per_cluster * s->cluster_count;
>
> /* Disable migration when vvfat is used rw */
> if (s->qcow) {
> @@ -1202,7 +1207,7 @@ static int vvfat_open(BlockDriverState *bs, QDict
> *options, int flags,
> }
> }
>
> - if (s->first_sectors_number == 0x40) {
> + if (s->offset_to_bootsector > 0) {
> init_mbr(s, cyls, heads, secs);
> }
>
> @@ -1415,15 +1420,23 @@ static int vvfat_read(BlockDriverState *bs, int64_t
> sector_num,
> }
> DLOG(fprintf(stderr, "sector %d not allocated\n", (int)sector_num));
> }
> - if(sector_num<s->faked_sectors) {
> - if(sector_num<s->first_sectors_number)
> -
> memcpy(buf+i*0x200,&(s->first_sectors[sector_num*0x200]),0x200);
> - else if(sector_num-s->first_sectors_number<s->sectors_per_fat)
> -
> memcpy(buf+i*0x200,&(s->fat.pointer[(sector_num-s->first_sectors_number)*0x200]),0x200);
> - else
> if(sector_num-s->first_sectors_number-s->sectors_per_fat<s->sectors_per_fat)
> -
> memcpy(buf+i*0x200,&(s->fat.pointer[(sector_num-s->first_sectors_number-s->sectors_per_fat)*0x200]),0x200);
> + if (sector_num < s->offset_to_root_dir) {
> + if (sector_num < s->offset_to_fat)
> + memcpy(buf + i * 0x200,
> + &(s->first_sectors[sector_num * 0x200]),
> + 0x200);
> + else if (sector_num < s->offset_to_fat + s->sectors_per_fat)
> + memcpy(buf + i * 0x200,
> + &(s->fat.pointer[(sector_num
> + - s->offset_to_fat) * 0x200]),
> + 0x200);
> + else if (sector_num < s->offset_to_root_dir)
> + memcpy(buf + i * 0x200,
> + &(s->fat.pointer[(sector_num - s->offset_to_fat
> + - s->sectors_per_fat) * 0x200]),
> + 0x200);
The QEMU coding style requires braces for all branches of this if
statement.
Kevin
- [Qemu-block] [PATCH 04/13] vvfat: rename useless enumeration values, (continued)
- [Qemu-block] [PATCH 04/13] vvfat: rename useless enumeration values, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 07/13] vvfat: always create . and .. entries at first and in that order, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 09/13] vvfat: correctly create base short names for non-ASCII filenames, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 12/13] vvfat: handle KANJI lead byte 0xe5, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 08/13] vvfat: correctly create long names for non-ASCII filenames, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 13/13] vvfat: change OEM name to 'MSWIN4.1', Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 11/13] vvfat: limit number of entries in root directory in FAT12/FAT16, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 10/13] vvfat: correctly generate numeric-tail of short file names, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 05/13] vvfat: introduce offset_to_bootsector, offset_to_fat and offset_to_root_dir, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 01/13] vvfat: fix qemu-img map and qemu-img convert, Hervé Poussineau, 2017/05/15
- [Qemu-block] [PATCH 02/13] vvfat: replace tabs by 8 spaces, Hervé Poussineau, 2017/05/15