[Qemu-devel] block-vvfat RW patch

[Qemu-devel] block-vvfat RW patch

[Roger Lathrop]

[-- Attachment #1: Type: text/plain, Size: 1714 bytes --]

Attached patch fixes 3 issues I found while trying to write files from guest 
to a vfat drive using this configuration:
Host: Windows XP (NTFS)
Guest: DOS 5 (yeah, really!)
Compiler: gcc version 3.4.2 (mingw-special)
Qemu Version: 0.8.2
Command line: qemu ....-hdb fat:rw:c:\dev\qemu\work...

Problem: Qemu would fault when copying files from guest to d:\

Changes:

1) Fixed assert macro for mingw. Original causes spurious faults when 
there's a dangling else after the assert.
2) Added O_BINARY to open call in commit_one_file().  Without it, binary 
files get LF-->CR/LF translations on Windows.
3) Changed sector2cluster to return a signed int, and added type casts to 
force the division to be signed.

First 2 changes are straight forward and should be safe. 3'rd solved my 
problem, but should be looked at by someone who understands vvfat better 
than me. I've only tested on the above configuration.
The problem I saw was DOS writing to sectors less than s->faked_sectors. The 
unsigned divide returned a large positive number, which would fault in this 
bit of code in vvfat_write:
if (i >= 0)
  s->used_clusters[i] |= USED_ALLOCATED;

This fix seems reasonable, since the callers of sector2cluster seem to 
expect a signed result. I suppose an alternate fix would be to test for the 
special case and always return -1. But I hate special cases ;-)

Patch is against 0.8.2, not current CVS, sorry. Doesn't appear to have been 
fixed since 0.8.2

Good to have this working. Our app is an embedded system with really crude 
(read, no TCP/IP) network support. Until now, getting log files off the 
guest has been a pain. So, a big THANKS to whoever wrote block-vvfat, 
despite the little buglets.

Roger



[-- Attachment #2: block-vvfat.c.patch --]
[-- Type: application/octet-stream, Size: 1384 bytes --]

Index: block-vvfat.c
===================================================================
RCS file: /sources/qemu/qemu/block-vvfat.c,v
retrieving revision 1.6
diff -u -r1.6 block-vvfat.c
--- block-vvfat.c	4 Jun 2006 11:39:07 -0000	1.6
+++ block-vvfat.c	4 Sep 2006 20:31:49 -0000
@@ -61,7 +61,7 @@
     exit(-5);
 }
 #undef assert
-#define assert(a) if (!(a)) nonono(__FILE__, __LINE__, #a)
+#define assert(a) do {if (!(a)) nonono(__FILE__, __LINE__, #a);}while(0)
 #endif
 
 #else
@@ -785,9 +785,10 @@
     return 0;
 }
 
-static inline uint32_t sector2cluster(BDRVVVFATState* s,off_t sector_num)
+static inline int32_t sector2cluster(BDRVVVFATState* s,off_t sector_num)
 {
-    return (sector_num-s->faked_sectors)/s->sectors_per_cluster;
+    // Force integer divide. Must return negative result when sector < faked_sectors
+    return (int32_t)(sector_num-s->faked_sectors)/(int32_t)s->sectors_per_cluster;
 }
 
 static inline off_t cluster2sector(BDRVVVFATState* s, uint32_t cluster_num)
@@ -2178,7 +2179,7 @@
     for (i = s->cluster_size; i < offset; i += s->cluster_size)
 	c = modified_fat_get(s, c);
 
-    fd = open(mapping->path, O_RDWR | O_CREAT, 0666);
+    fd = open(mapping->path, O_RDWR | O_CREAT | O_BINARY, 0666);
     if (fd < 0) {
 	fprintf(stderr, "Could not open %s... (%s, %d)\n", mapping->path,
 		strerror(errno), errno);

Re: [Qemu-devel] block-vvfat RW patch

[Kazu]

Hi,

I confirmed it works. Thank you.

Regards,
Kazu


Sent: Tuesday, September 05, 2006 8:57 AM Roger Lathrop wrote:

> Attached patch fixes 3 issues I found while trying to write files from
guest
> to a vfat drive using this configuration:
> Host: Windows XP (NTFS)
> Guest: DOS 5 (yeah, really!)
> Compiler: gcc version 3.4.2 (mingw-special)
> Qemu Version: 0.8.2
> Command line: qemu ....-hdb fat:rw:c:\dev\qemu\work...
>
> Problem: Qemu would fault when copying files from guest to d:\
>
> Changes:
>
> 1) Fixed assert macro for mingw. Original causes spurious faults when
> there's a dangling else after the assert.
> 2) Added O_BINARY to open call in commit_one_file().  Without it, binary
> files get LF-->CR/LF translations on Windows.
> 3) Changed sector2cluster to return a signed int, and added type casts to
> force the division to be signed.
>
> First 2 changes are straight forward and should be safe. 3'rd solved my
> problem, but should be looked at by someone who understands vvfat better
> than me. I've only tested on the above configuration.
> The problem I saw was DOS writing to sectors less than s->faked_sectors.
The
> unsigned divide returned a large positive number, which would fault in
this
> bit of code in vvfat_write:
> if (i >= 0)
>  s->used_clusters[i] |= USED_ALLOCATED;
>
> This fix seems reasonable, since the callers of sector2cluster seem to
> expect a signed result. I suppose an alternate fix would be to test for
the
> special case and always return -1. But I hate special cases ;-)
>
> Patch is against 0.8.2, not current CVS, sorry. Doesn't appear to have
been
> fixed since 0.8.2
>
> Good to have this working. Our app is an embedded system with really crude
> (read, no TCP/IP) network support. Until now, getting log files off the
> guest has been a pain. So, a big THANKS to whoever wrote block-vvfat,
> despite the little buglets.
>
> Roger

Re: [Qemu-devel] block-vvfat RW patch

[Fabrice Bellard]

Roger Lathrop wrote:

> 3) Changed sector2cluster to return a signed int, and added type casts 
> to force the division to be signed.

This is not safe. vvfat_write must be modified to handle the case where 
sectors are written before s->faked_sectors (maybe ignoring the write 
could suffice).

Regards,

Fabrice.