| Message ID | 1367945808-19979-1-git-send-email-lersek@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/07/2013 10:56 AM, Laszlo Ersek wrote: > In Windows guests this may make a difference. > > Suggested-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > qga/commands-posix.c | 22 ++++++++++++++++------ > 1 files changed, 16 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake <eblake@redhat.com>
On 05/07/2013 10:56 AM, Laszlo Ersek wrote: > In Windows guests this may make a difference. > > Suggested-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > --- > qga/commands-posix.c | 22 ++++++++++++++++------ > 1 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > index 04c6951..2eec712 100644 > --- a/qga/commands-posix.c > +++ b/qga/commands-posix.c Oh, and only NOW do I notice that this is in a file named commands-posix.c that doesn't get compiled into the Windows build of qga (there, we only build commands-win32.c, and THAT file always fails this command, because no one has ported guest-file-open there yet). But I guess there is still the argument that some weirdnix system exists that isn't quite POSIX compliant and does have a distinct binary mode (maybe someone plans on compiling qga for Cygwin instead of native windows, since at least that would be able to open files when the commands-win32.c variant doesn't?), so I still think the patch is worth keeping.
On 7 May 2013 18:27, Eric Blake <eblake@redhat.com> wrote: > On 05/07/2013 10:56 AM, Laszlo Ersek wrote: >> In Windows guests this may make a difference. >> >> Suggested-by: Eric Blake <eblake@redhat.com> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > Oh, and only NOW do I notice that this is in a file named > commands-posix.c that doesn't get compiled into the Windows build of qga > (there, we only build commands-win32.c, and THAT file always fails this > command, because no one has ported guest-file-open there yet). But I > guess there is still the argument that some weirdnix system exists that > isn't quite POSIX compliant and does have a distinct binary mode (maybe > someone plans on compiling qga for Cygwin instead of native windows, > since at least that would be able to open files when the > commands-win32.c variant doesn't?), so I still think the patch is worth > keeping. If we accept this we should at a minimum update the commit message to say that we're doing this for theoretical completeness rather than any practical reason. (Not that "this may make a difference" is a particularly convincing commit message in the first place :-)) thanks -- PMM
On Tue, May 07, 2013 at 11:27:03AM -0600, Eric Blake wrote: > On 05/07/2013 10:56 AM, Laszlo Ersek wrote: > > In Windows guests this may make a difference. > > > > Suggested-by: Eric Blake <eblake@redhat.com> > > Signed-off-by: Laszlo Ersek <lersek@redhat.com> > > --- > > qga/commands-posix.c | 22 ++++++++++++++++------ > > 1 files changed, 16 insertions(+), 6 deletions(-) > > > > diff --git a/qga/commands-posix.c b/qga/commands-posix.c > > index 04c6951..2eec712 100644 > > --- a/qga/commands-posix.c > > +++ b/qga/commands-posix.c > > Oh, and only NOW do I notice that this is in a file named > commands-posix.c that doesn't get compiled into the Windows build of qga > (there, we only build commands-win32.c, and THAT file always fails this > command, because no one has ported guest-file-open there yet). But I > guess there is still the argument that some weirdnix system exists that > isn't quite POSIX compliant and does have a distinct binary mode (maybe > someone plans on compiling qga for Cygwin instead of native windows, > since at least that would be able to open files when the > commands-win32.c variant doesn't?), so I still think the patch is worth > keeping. FWIW, I have some rough patches for w32 implementations of guest-file-* commands queued up that I'll be cleaning up for 1.6, so it's not so much theoretical as just a tad early :) > > -- > Eric Blake eblake redhat com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
diff --git a/qga/commands-posix.c b/qga/commands-posix.c index 04c6951..2eec712 100644 --- a/qga/commands-posix.c +++ b/qga/commands-posix.c @@ -242,17 +242,27 @@ static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err) typedef const char * const ccpc; +#ifndef O_BINARY +#define O_BINARY 0 +#endif + /* http://pubs.opengroup.org/onlinepubs/9699919799/functions/fopen.html */ static const struct { ccpc *forms; int oflag_base; } guest_file_open_modes[] = { - { (ccpc[]){ "r", "rb", NULL }, O_RDONLY }, - { (ccpc[]){ "w", "wb", NULL }, O_WRONLY | O_CREAT | O_TRUNC }, - { (ccpc[]){ "a", "ab", NULL }, O_WRONLY | O_CREAT | O_APPEND }, - { (ccpc[]){ "r+", "rb+", "r+b", NULL }, O_RDWR }, - { (ccpc[]){ "w+", "wb+", "w+b", NULL }, O_RDWR | O_CREAT | O_TRUNC }, - { (ccpc[]){ "a+", "ab+", "a+b", NULL }, O_RDWR | O_CREAT | O_APPEND } + { (ccpc[]){ "r", NULL }, O_RDONLY }, + { (ccpc[]){ "rb", NULL }, O_RDONLY | O_BINARY }, + { (ccpc[]){ "w", NULL }, O_WRONLY | O_CREAT | O_TRUNC }, + { (ccpc[]){ "wb", NULL }, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY }, + { (ccpc[]){ "a", NULL }, O_WRONLY | O_CREAT | O_APPEND }, + { (ccpc[]){ "ab", NULL }, O_WRONLY | O_CREAT | O_APPEND | O_BINARY }, + { (ccpc[]){ "r+", NULL }, O_RDWR }, + { (ccpc[]){ "rb+", "r+b", NULL }, O_RDWR | O_BINARY }, + { (ccpc[]){ "w+", NULL }, O_RDWR | O_CREAT | O_TRUNC }, + { (ccpc[]){ "wb+", "w+b", NULL }, O_RDWR | O_CREAT | O_TRUNC | O_BINARY }, + { (ccpc[]){ "a+", NULL }, O_RDWR | O_CREAT | O_APPEND }, + { (ccpc[]){ "ab+", "a+b", NULL }, O_RDWR | O_CREAT | O_APPEND | O_BINARY } }; static int
In Windows guests this may make a difference. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Laszlo Ersek <lersek@redhat.com> --- qga/commands-posix.c | 22 ++++++++++++++++------ 1 files changed, 16 insertions(+), 6 deletions(-)