[libvirt] PATCH: Fix build on Mingw32
pread() and pwrite() do not exist on mingw, and the nodedevxml2xmltest was adding libvirt_driver_qemu.la for some unknown reason - probably a cut & paste error. This broke the test compile when qemu was turned off as it is on mingw32. Daniel diff --git a/src/pci.c b/src/pci.c --- a/src/pci.c +++ b/src/pci.c @@ -156,7 +156,8 @@ pciRead(pciDevice *dev, unsigned pos, ui if (pciOpenConfig(dev) < 0) return -1; - if (pread(dev->fd, buf, buflen, pos) < 0) { + if (lseek(dev->fd, pos, SEEK_SET) != pos || + read(dev->fd, buf, buflen) < 0) { char ebuf[1024]; VIR_WARN(_("Failed to read from '%s' : %s"), dev->path, virStrerror(errno, ebuf, sizeof(ebuf))); @@ -195,7 +196,8 @@ pciWrite(pciDevice *dev, unsigned pos, u if (pciOpenConfig(dev) < 0) return -1; - if (pwrite(dev->fd, buf, buflen, pos) < 0) { + if (lseek(dev->fd, pos, SEEK_SET) != pos || + write(dev->fd, buf, buflen) < 0) { char ebuf[1024]; VIR_WARN(_("Failed to write to '%s' : %s"), dev->path, virStrerror(errno, ebuf, sizeof(ebuf))); diff --git a/tests/Makefile.am b/tests/Makefile.am --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -185,7 +185,7 @@ endif nodedevxml2xmltest_SOURCES = \ nodedevxml2xmltest.c \ testutils.c testutils.h -nodedevxml2xmltest_LDADD = ../src/libvirt_driver_qemu.la $(LDADDS) +nodedevxml2xmltest_LDADD = $(LDADDS) virshtest_SOURCES = \ virshtest.c \ -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Daniel P. Berrange wrote:
pread() and pwrite() do not exist on mingw, and the nodedevxml2xmltest was adding libvirt_driver_qemu.la for some unknown reason - probably a cut & paste error. This broke the test compile when qemu was turned off as it is on mingw32.
Might it be better to use gnulib's pread module? Maybe overkill for this one case, but it could save us the hassle in the future. -- Chris Lalancette
Daniel P. Berrange wrote:
pread() and pwrite() do not exist on mingw, and the nodedevxml2xmltest was adding libvirt_driver_qemu.la for some unknown reason - probably a cut & paste error. This broke the test compile when qemu was turned off as it is on mingw32.
Although the new code doesn't have exactly the same semantics as pread/pwrite (new code doesn't restore each file pointer to its original position), it looks like no caller relied on the file pointers remaining unchanged. On a related note, don't these need EAGAIN handling? Maybe EINTR, too.
diff --git a/src/pci.c b/src/pci.c --- a/src/pci.c +++ b/src/pci.c @@ -156,7 +156,8 @@ pciRead(pciDevice *dev, unsigned pos, ui if (pciOpenConfig(dev) < 0) return -1;
- if (pread(dev->fd, buf, buflen, pos) < 0) { + if (lseek(dev->fd, pos, SEEK_SET) != pos || + read(dev->fd, buf, buflen) < 0) { char ebuf[1024]; VIR_WARN(_("Failed to read from '%s' : %s"), dev->path, virStrerror(errno, ebuf, sizeof(ebuf))); @@ -195,7 +196,8 @@ pciWrite(pciDevice *dev, unsigned pos, u if (pciOpenConfig(dev) < 0) return -1;
- if (pwrite(dev->fd, buf, buflen, pos) < 0) { + if (lseek(dev->fd, pos, SEEK_SET) != pos || + write(dev->fd, buf, buflen) < 0) { char ebuf[1024]; VIR_WARN(_("Failed to write to '%s' : %s"), dev->path, virStrerror(errno, ebuf, sizeof(ebuf))); diff --git a/tests/Makefile.am b/tests/Makefile.am --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -185,7 +185,7 @@ endif nodedevxml2xmltest_SOURCES = \ nodedevxml2xmltest.c \ testutils.c testutils.h -nodedevxml2xmltest_LDADD = ../src/libvirt_driver_qemu.la $(LDADDS) +nodedevxml2xmltest_LDADD = $(LDADDS)
virshtest_SOURCES = \ virshtest.c \
On Tue, Mar 03, 2009 at 11:18:45AM +0100, Jim Meyering wrote:
Daniel P. Berrange wrote:
pread() and pwrite() do not exist on mingw, and the nodedevxml2xmltest was adding libvirt_driver_qemu.la for some unknown reason - probably a cut & paste error. This broke the test compile when qemu was turned off as it is on mingw32.
Although the new code doesn't have exactly the same semantics as pread/pwrite (new code doesn't restore each file pointer to its original position), it looks like no caller relied on the file pointers remaining unchanged.
Yes, all the I/O is random access via the pciRead() or pciWrite() functions so the file pointer position is irrelvant for this usage
On a related note, don't these need EAGAIN handling? Maybe EINTR, too.
I queried whether this should ue saferead/write when the patches were first reviewed, but Mark said it was not neccessary for use of sysfs and this code will be replaced by libpciaccess.so in the not too distant future Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On Tue, Mar 03, 2009 at 11:38:58AM +0000, Daniel P. Berrange wrote:
On Tue, Mar 03, 2009 at 11:18:45AM +0100, Jim Meyering wrote:
Daniel P. Berrange wrote:
pread() and pwrite() do not exist on mingw, and the nodedevxml2xmltest was adding libvirt_driver_qemu.la for some unknown reason - probably a cut & paste error. This broke the test compile when qemu was turned off as it is on mingw32.
Although the new code doesn't have exactly the same semantics as pread/pwrite (new code doesn't restore each file pointer to its original position), it looks like no caller relied on the file pointers remaining unchanged.
Yes, all the I/O is random access via the pciRead() or pciWrite() functions so the file pointer position is irrelvant for this usage
On a related note, don't these need EAGAIN handling? Maybe EINTR, too.
I queried whether this should ue saferead/write when the patches were first reviewed, but Mark said it was not neccessary for use of sysfs and this code will be replaced by libpciaccess.so in the not too distant future
Of course now I changed pread to read(), the make syntax-check complains, so I went all the way to saferead() and safewrite() here Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
participants (3)
-
Chris Lalancette -
Daniel P. Berrange -
Jim Meyering