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

[Michael Roth]

Quoting Paolo Bonzini (2015-11-10 11:59:18)
> 
> 
> On 10/11/2015 16:40, Michael Roth wrote:
> > 
> > I hit an issue testing this though, this does fix the append
> > case, but a+, ab+, a+b all imply append+read, while
> > FILE_APPEND_DATA only grants append access.
> > 
> > FILE_APPEND_DATA|GENERIC_READ seems to work, but I'm not
> > finding much official documentation on what's valid with
> > FILE_APPEND_DATA. Do you have a reference that might
> > confirm this is valid usage? The only reference to
> > FILE_APPEND_DATA I saw was a single comment in:
> 
> I found a few references to FILE_APPEND_DATA, but not to combining it
> with GENERIC_READ.
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/gg258116%28v=vs.85%29.aspx
> (File Access Rights Constants) under FILE_APPEND_DATA says "For a file
> object, the right to append data to the file. (For local files, write
> operations will not overwrite existing data if this flag is specified
> without FILE_WRITE_DATA.)".
> 
> https://msdn.microsoft.com/en-us/library/ff548289.aspx also says:
> 
> * If only the FILE_APPEND_DATA and SYNCHRONIZE flags are set, the caller
> can write only to the end of the file, and any offset information about
> writes to the file is ignored. However, the file will automatically be
> extended as necessary for this type of write operation.
> 
> * Setting the FILE_WRITE_DATA flag for a file also allows writes beyond
> the end of the file to occur. The file is automatically extended for
> this type of write, as well.
> 
> 
> Regarding the combination of reading and appending, GENERIC_READ and
> GENERIC_WRITE are sort of "macro" access rights, which expand to
> different attributes depending on what you're opening.  For files,
> FILE_WRITE_DATA and FILE_READ_DATA are part of GENERIC_READ and
> GENERIC_WRITE:
> 
> 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

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.

> 
> Paolo
>