4:57 a.m.
Hi.
I used the rawhide RPMs to take out the DLLs for a test drive on Windows 64bit
the other day.
While trying to read the "screenshot" from the test domain using the
test:///default connection, I only received 5 Bytes of the libvirtLogo.png file.
Looking at the test driver, the condition piqued my interest, but wasn't the
cause of the short read, only a bit odd. See patch #1.
The actual cause was that the file was opened in text mode and the seventh byte
was a 0x1A which triggered EOF on the FD. See patch #2.
Claudio Bley (2):
test: fix call to virFDStreamOpenFile in testDomainScreenshot
Always open files in binary mode in virFDStreamOpenFileInternal
src/fdstream.c | 2 +-
src/test/test_driver.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--
1.8.3.1
4:57 a.m.
New subject: [libvirt] [PATCH 1/2] test: fix call to virFDStreamOpenFile in testDomainScreenshot
N.B. This had no ill effects as long as O_RDONLY is defined to
to be 0, such that the expression (O_RDONLY < 0) yielded 0
again.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
---
src/test/test_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 47c9d38..e1197c9 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5862,7 +5862,7 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
if (VIR_STRDUP(ret, "image/png") < 0)
return NULL;
- if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY
< 0))
+ if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY)
< 0)
VIR_FREE(ret);
return ret;
--
1.8.3.1
6:53 a.m.
New subject: [libvirt] [PATCH 1/2] test: fix call to virFDStreamOpenFile in testDomainScreenshot
On 09/24/2013 03:57 AM, Claudio Bley wrote:
N.B. This had no ill effects as long as O_RDONLY is defined to
to be 0, such that the expression (O_RDONLY < 0) yielded 0
again.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
---
src/test/test_driver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index 47c9d38..e1197c9 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5862,7 +5862,7 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
if (VIR_STRDUP(ret, "image/png") < 0)
return NULL;
- if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY
< 0))
+ if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0, O_RDONLY)
< 0)
ACK. Embarrassing that even Coverity doesn't flag this blatant bug.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
7:32 a.m.
New subject: [libvirt] [PATCH 1/2] test: fix call to virFDStreamOpenFile in testDomainScreenshot
At Tue, 24 Sep 2013 05:53:41 -0600,
Eric Blake wrote:
On 09/24/2013 03:57 AM, Claudio Bley wrote:
> N.B. This had no ill effects as long as O_RDONLY is defined to
> to be 0, such that the expression (O_RDONLY < 0) yielded 0
> again.
>
> Signed-off-by: Claudio Bley <cbley(a)av-test.de>
> ---
> src/test/test_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/test/test_driver.c b/src/test/test_driver.c
> index 47c9d38..e1197c9 100644
> --- a/src/test/test_driver.c
> +++ b/src/test/test_driver.c
> @@ -5862,7 +5862,7 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED,
> if (VIR_STRDUP(ret, "image/png") < 0)
> return NULL;
>
> - if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0,
O_RDONLY < 0))
> + if (virFDStreamOpenFile(st, PKGDATADIR "/libvirtLogo.png", 0, 0,
O_RDONLY) < 0)
ACK. Embarrassing that even Coverity doesn't flag this blatant bug.
On 09/24/2013 03:57 AM, Claudio Bley wrote:
> On win32, using text mode for binary files might result in short
> reads since ASCII character 0x1A is interpreted as EOF. Also, it
> could lead to problems using the seek functions because of the \r
> handling.
>
> Signed-off-by: Claudio Bley <cbley(a)av-test.de>
> ---
> src/fdstream.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
Thanks, both pushed now.
Claudio
n--
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:<http://www.av-test.org>
Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
4:57 a.m.
New subject: [libvirt] [PATCH 2/2] Always open files in binary mode in virFDStreamOpenFileInternal
On win32, using text mode for binary files might result in short
reads since ASCII character 0x1A is interpreted as EOF. Also, it
could lead to problems using the seek functions because of the \r
handling.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
---
src/fdstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/fdstream.c b/src/fdstream.c
index 10c7c2a..f7dae39 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -593,7 +593,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
st, path, oflags, offset, length, mode);
- oflags |= O_NOCTTY;
+ oflags |= O_NOCTTY | O_BINARY;
if (oflags & O_CREAT)
fd = open(path, oflags, mode);
--
1.8.3.1
5:16 a.m.
New subject: [libvirt] [PATCH 2/2] Always open files in binary mode in virFDStreamOpenFileInternal
On Tue, Sep 24, 2013 at 11:57:38AM +0200, Claudio Bley wrote:
On win32, using text mode for binary files might result in short
reads since ASCII character 0x1A is interpreted as EOF. Also, it
could lead to problems using the seek functions because of the \r
handling.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
What did you do to hit this problem ? AFAIK the virFDStream code
isn't useful on Windows at all, since it requires the iohelper
process and we haven't got that working on Windows.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
5:43 a.m.
New subject: [libvirt] [PATCH 2/2] Always open files in binary mode in virFDStreamOpenFileInternal
At Tue, 24 Sep 2013 11:16:39 +0100,
Daniel P. Berrange wrote:
On Tue, Sep 24, 2013 at 11:57:38AM +0200, Claudio Bley wrote:
> On win32, using text mode for binary files might result in short
> reads since ASCII character 0x1A is interpreted as EOF. Also, it
> could lead to problems using the seek functions because of the \r
> handling.
>
> Signed-off-by: Claudio Bley <cbley(a)av-test.de>
What did you do to hit this problem ? AFAIK the virFDStream code
isn't useful on Windows at all, since it requires the iohelper
process and we haven't got that working on Windows.
AFAIS, the iohelper is only needed for non-blocking I/O which I
haven't used.
Here's my code:
,----
| virConnectPtr conn = virConnectOpen("test:///default");
|
| unsigned long libVer;
| int ret = virConnectGetLibVersion(conn, &libVer);
|
| virDomainPtr domain = virDomainLookupByName(conn, "test");
|
| virStreamPtr stream = virStreamNew(conn, 0);
|
| char* mimetype = virDomainScreenshot(domain, stream, 0, 0);
| char buf[8192];
| int got = 0;
|
| puts(mimetype);
|
| do {
| got = virStreamRecv(stream, buf, 8192);
|
| switch (got) {
| case 0:
| virStreamFinish(stream);
| break;
|
| case -1:
| puts("error");
| virStreamAbort(stream);
| goto cleanup;
|
| default:
| printf("got %d bytes\n", got);
| }
| } while(got > 0);
|
| cleanup:
| virDomainFree(domain);
| virConnectClose(conn);
`----
Here's the output:
2013-09-24 07:30:46.823+0000: 3052: debug : virDomainScreenshot:3241 :
dom=0000000000681D30, (VM: name=test, uuid=6695eb01-f6a4-8304-79aa-97f2502e193f),
stream=0000000000681E30, flags=0
2013-09-24 07:30:46.823+0000: 3052: debug : virFDStreamOpenFileInternal:599 :
st=0000000000681E30
path=/usr/x86_64-w64-mingw32/sys-root/mingw/share/libvirt/libvirtLogo.png oflags=0
offset=0 length=0 mode=0
2013-09-24 07:30:46.825+0000: 3052: debug : virFDStreamOpenInternal:490 :
st=0000000000681E30 fd=3 cmd=0000000000000000 errfd=-1 length=0
image/png
2013-09-24 07:30:46.826+0000: 3052: debug : virStreamRecv:17138 : stream=0000000000681E30,
data=000000000022DE20, nbytes=8192
got 5 bytes
2013-09-24 07:30:46.828+0000: 3052: debug : virStreamRecv:17138 : stream=0000000000681E30,
data=000000000022DE20, nbytes=8192
2013-09-24 07:30:46.830+0000: 3052: debug : virStreamFinish:17506 :
stream=0000000000681E30
2013-09-24 07:30:46.831+0000: 3052: debug : virFDStreamCloseInt:260 : st=0000000000681E30
2013-09-24 07:30:46.831+0000: 3052: debug : virFileClose:90 : Closed fd 3
/ Claudio
--
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:<http://www.av-test.org>
Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
6:53 a.m.
New subject: [libvirt] [PATCH 2/2] Always open files in binary mode in virFDStreamOpenFileInternal
On 09/24/2013 03:57 AM, Claudio Bley wrote:
On win32, using text mode for binary files might result in short
reads since ASCII character 0x1A is interpreted as EOF. Also, it
could lead to problems using the seek functions because of the \r
handling.
Signed-off-by: Claudio Bley <cbley(a)av-test.de>
---
src/fdstream.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
ACK.
diff --git a/src/fdstream.c b/src/fdstream.c
index 10c7c2a..f7dae39 100644
--- a/src/fdstream.c
+++ b/src/fdstream.c
@@ -593,7 +593,7 @@ virFDStreamOpenFileInternal(virStreamPtr st,
VIR_DEBUG("st=%p path=%s oflags=%x offset=%llu length=%llu mode=%o",
st, path, oflags, offset, length, mode);
- oflags |= O_NOCTTY;
+ oflags |= O_NOCTTY | O_BINARY;
if (oflags & O_CREAT)
fd = open(path, oflags, mode);
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
4079
days inactive
4079
days old
7 comments
3 participants
participants (3)
-
Claudio Bley -
Daniel P. Berrange -
Eric Blake