Re: [Qemu-devel] [PATCH] qga: fix append file open modes for win32

[Michael Roth]

Quoting Paolo Bonzini (2015-11-11 08:49:57)
> 
> 
> On 11/11/2015 15:02, Michael Roth wrote:
> >> GENERIC_READ for files
> >>         = FILE_READ_DATA
> >>         + FILE_READ_ATTRIBUTES
> >>         + FILE_READ_EA
> >>         + SYNCHRONIZE
> >>         + STANDARD_RIGHTS_READ (which is READ_CONTROL)
> >>
> >> GENERIC_WRITE for files
> >>         = FILE_APPEND_DATA
> >>         + FILE_WRITE_DATA
> >>         + FILE_WRITE_ATTRIBUTES
> >>         + FILE_WRITE_EA
> >>         + SYNCHRONIZE
> >>         + STANDARD_RIGHTS_WRITE (which is READ_CONTROL too!)
> >>
> >> Of these of course qemu-ga only needs FILE_*_DATA and possibly SYNCHRONIZE.
> >>
> >> The above description doesn't say what happens if you specify
> >> FILE_READ_DATA and FILE_APPEND_DATA together, but I guess you can expect
> >> it to just work.
> > 
> > Thanks, this is very helpful. This seems to coincide with
> > FILE_GENERIC_WRITE / FILE_GENERIC_READ if you want to access the
> > bitmasks directly (though it looks like you can still add flags
> > to GENERIC_WRITE / GENERIC_READ):
> > 
> >   
> > https://msdn.microsoft.com/en-us/library/windows/desktop/aa364399(v=vs.85).aspx
> 
> Yes, I had missed the FILE_GENERIC_* definitions.  I found them now in
> mingw as well, and they are exactly what you would expect (an | of the
> various flags).
> 
> > Looks like the crux of it is that FILE_WRITE_DATA causes us not to ignore
> > the start offset (0) for writes, so we end up writing data at the
> > beginning of the file instead of the end.
> > 
> > So I think the following
> > should work:
> > 
> >   a: FILE_GENERIC_WRITE & ~FILE_WRITE_DATA
> >   a+: FILE_GENERIC_READ | FILE_APPEND_DATA
> > 
> > FILE_APPEND_DATA by itself might work for a:, but for consistency I
> > think I'd prefer sticking with the generic set and masking out
> > FILE_WRITE_DATA.
> 
> For a+ I would use any of
> 
>         (FILE_GENERIC_READ | FILE_GENERIC_WRITE) & ~FILE_WRITE_DATA
>         GENERIC_READ | (FILE_GENERIC_WRITE & ~FILE_WRITE_DATA)
> 
> Perhaps you can define this:
> 
> #define FILE_GENERIC_APPEND     (FILE_GENERIC_WRITE & ~FILE_WRITE_DATA)
> 
> and then use
> 
>         a: FILE_GENERIC_APPEND
>         a+: GENERIC_READ|FILE_GENERIC_APPEND
> 
> or
> 
>         a: FILE_GENERIC_APPEND
>         a+: FILE_GENERIC_READ|FILE_GENERIC_APPEND

Yah, both are more proper actually (I was relying on FILE_GENERIC_READ
already containing the other flags from FILE_GENERIC_WRITE, but that's
more likely to break in the future).

I think I prefer the former since it avoids confusion on GENERIC_READ vs.
FILE_GENERIC_READ differences.

Kirk, I'm planning on squashing this into your v2 series, so no need to
resubmit.

Thanks!

> 
> ?
> 
> Paolo
>