[Libvir] [PATCH] 1/2: Move safewrite and saferead to a separate file.

Move safewrite and saferead to a separate file. We currently use safewrite from inside libvirt and don't want to publish any such function name. However, we do want to use it in applications like virsh, libvirtd and libvirt_proxy that link with libvirt. To that end, this change moves that function definition (along with the nearly identical saferead) into a new file, util-lib.c. To avoid maintaining separate copies of even such small functions, we simply include that new file from util.c. Then, the separate applications that need to use safewrite simply compile and link with util-lib.c. Of course, this does mean that each of those applications will containing two copies of these functions. However, the functions are so small that it's not worth worrying about that. * src/util.c (saferead, safewrite): Move function definitions to util-lib.c and include that .c file. * src/util-lib.c (saferead, safewrite): New file. Functions from src/util.c with slight change (s/int r =/ssize_t r =/) to reflect read/write return type. * src/util-lib.h: Declare the two moved functions. * src/util.h: Remove declarations. Include src/util-lib.h. * proxy/Makefile.am (libvirt_proxy_SOURCES): Add src/util-lib.c. * qemud/Makefile.am (libvirtd_SOURCES): Likewise. * src/Makefile.am (virsh_SOURCES): Add util-lib.c. Remove some SP-before-TAB. Signed-off-by: Jim Meyering <meyering@redhat.com> --- proxy/Makefile.am | 6 +++- proxy/libvirt_proxy.c | 16 ++++---------- qemud/Makefile.am | 3 +- src/Makefile.am | 16 +++++++------- src/util-lib.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++ src/util-lib.h | 16 +++++++++++++++ src/util.c | 43 ++------------------------------------- src/util.h | 10 ++++---- 8 files changed, 95 insertions(+), 67 deletions(-) create mode 100644 src/util-lib.c create mode 100644 src/util-lib.h diff --git a/proxy/Makefile.am b/proxy/Makefile.am index ff9a3eb..68e3317 100644 --- a/proxy/Makefile.am +++ b/proxy/Makefile.am @@ -12,11 +12,13 @@ libexec_PROGRAMS = libvirt_proxy libvirt_proxy_SOURCES = libvirt_proxy.c @top_srcdir@/src/xend_internal.c \ @top_srcdir@/src/xen_internal.c @top_srcdir@/src/virterror.c \ @top_srcdir@/src/sexpr.c @top_srcdir@/src/xml.c \ - @top_srcdir@/src/xs_internal.c @top_srcdir@/src/buf.c @top_srcdir@/src/uuid.c + @top_srcdir@/src/xs_internal.c @top_srcdir@/src/buf.c \ + @top_srcdir@/src/util-lib.c \ + @top_srcdir@/src/uuid.c libvirt_proxy_LDFLAGS = $(WARN_CFLAGS) libvirt_proxy_DEPENDENCIES = libvirt_proxy_LDADD = install-exec-hook: chmod u+s $(DESTDIR)$(libexecdir)/libvirt_proxy -endif \ No newline at end of file +endif diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index d96d3db..a22ba6c 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -2,7 +2,7 @@ * proxy_svr.c: root suid proxy server for Xen access to APIs with no * side effects from unauthenticated clients. * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -26,6 +26,7 @@ #include "internal.h" #include "proxy_internal.h" +#include "util.h" #include "xen_internal.h" #include "xend_internal.h" #include "xs_internal.h" @@ -317,19 +318,12 @@ proxyWriteClientSocket(int nr, virProxyPacketPtr req) { return(-1); } -retry: - ret = write(pollInfos[nr].fd, (char *) req, req->len); + ret = safewrite(pollInfos[nr].fd, (char *) req, req->len); if (ret < 0) { - if (errno == EINTR) { - if (debug > 0) - fprintf(stderr, "write socket %d to client %d interrupted\n", - pollInfos[nr].fd, nr); - goto retry; - } fprintf(stderr, "write %d bytes to socket %d from client %d failed\n", req->len, pollInfos[nr].fd, nr); - proxyCloseClientSocket(nr); - return(-1); + proxyCloseClientSocket(nr); + return(-1); } if (ret == 0) { if (debug) diff --git a/qemud/Makefile.am b/qemud/Makefile.am index db6adb1..1e1f861 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -42,6 +42,7 @@ libvirtd_SOURCES = \ qemud.c internal.h \ remote_protocol.h remote_protocol.c \ remote.c \ + $(srcdir)/../src/util-lib.c \ event.c event.h #-D_XOPEN_SOURCE=600 -D_XOPEN_SOURCE_EXTENDED=1 -D_POSIX_C_SOURCE=199506L @@ -151,4 +152,4 @@ uninstall-init: endif # DBUS_INIT_SCRIPTS_RED_HAT -endif # WITH_LIBVIRTD \ No newline at end of file +endif # WITH_LIBVIRTD diff --git a/src/Makefile.am b/src/Makefile.am index 7bbf816..1da0d73 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,21 +51,21 @@ CLIENT_SOURCES = \ conf.c conf.h \ xm_internal.c xm_internal.h \ remote_internal.c remote_internal.h \ - bridge.c bridge.h \ - iptables.c iptables.h \ - uuid.c uuid.h \ - qemu_driver.c qemu_driver.h \ + bridge.c bridge.h \ + iptables.c iptables.h \ + uuid.c uuid.h \ + qemu_driver.c qemu_driver.h \ qemu_conf.c qemu_conf.h \ openvz_conf.c openvz_conf.h \ - openvz_driver.c openvz_driver.h \ + openvz_driver.c openvz_driver.h \ nodeinfo.h nodeinfo.c \ storage_conf.h storage_conf.c \ storage_driver.h storage_driver.c \ storage_backend.h storage_backend.c \ - storage_backend_fs.h storage_backend_fs.c \ + storage_backend_fs.h storage_backend_fs.c \ util.c util.h -SERVER_SOURCES = \ +SERVER_SOURCES = \ ../qemud/remote_protocol.c ../qemud/remote_protocol.h if WITH_STORAGE_LVM @@ -100,7 +100,7 @@ libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) bin_PROGRAMS = virsh -virsh_SOURCES = virsh.c console.c console.h +virsh_SOURCES = virsh.c console.c console.h util-lib.c virsh_LDFLAGS = $(WARN_CFLAGS) $(COVERAGE_LDFLAGS) virsh_DEPENDENCIES = $(DEPS) virsh_LDADD = $(LDADDS) $(VIRSH_LIBS) diff --git a/src/util-lib.c b/src/util-lib.c new file mode 100644 index 0000000..92496cd --- /dev/null +++ b/src/util-lib.c @@ -0,0 +1,52 @@ +/* + * common, generic utility functions + * + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. + * See COPYING.LIB for the License of this software + */ + +#include <config.h> + +#include <unistd.h> +#include <errno.h> + +#include "util-lib.h" + +/* Like read(), but restarts after EINTR */ +int saferead(int fd, void *buf, size_t count) +{ + size_t nread = 0; + while (count > 0) { + ssize_t r = read(fd, buf, count); + if (r < 0 && errno == EINTR) + continue; + if (r < 0) + return r; + if (r == 0) + return nread; + buf = (char *)buf + r; + count -= r; + nread += r; + } + return nread; +} + +/* Like write(), but restarts after EINTR */ +ssize_t safewrite(int fd, const void *buf, size_t count) +{ + size_t nwritten = 0; + while (count > 0) { + ssize_t r = write(fd, buf, count); + + if (r < 0 && errno == EINTR) + continue; + if (r < 0) + return r; + if (r == 0) + return nwritten; + buf = (const char *)buf + r; + count -= r; + nwritten += r; + } + return nwritten; +} diff --git a/src/util-lib.h b/src/util-lib.h new file mode 100644 index 0000000..4a14810 --- /dev/null +++ b/src/util-lib.h @@ -0,0 +1,16 @@ +/* + * private utility functions + * + * Copyright (C) 2008 Red Hat, Inc. + * See COPYING.LIB for the License of this software + */ + +#ifndef __UTIL_LIB_H__ +#define __UTIL_LIB_H__ + +#include <sys/types.h> + +int saferead(int fd, void *buf, size_t count); +ssize_t safewrite(int fd, const void *buf, size_t count); + +#endif diff --git a/src/util.c b/src/util.c index 19131a7..67cb0b8 100644 --- a/src/util.c +++ b/src/util.c @@ -1,7 +1,7 @@ /* * utils.c: common, generic utility functions * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain @@ -46,6 +46,8 @@ #include "buf.h" #include "util.h" +#include "util-lib.c" + #define MAX_ERROR_LEN 1024 #define virLog(msg...) fprintf(stderr, msg) @@ -276,45 +278,6 @@ virExecNonBlock(virConnectPtr conn, #endif /* __MINGW32__ */ -/* Like read(), but restarts after EINTR */ -int saferead(int fd, void *buf, size_t count) -{ - size_t nread = 0; - while (count > 0) { - int r = read(fd, buf, count); - if (r < 0 && errno == EINTR) - continue; - if (r < 0) - return r; - if (r == 0) - return nread; - buf = (unsigned char *)buf + r; - count -= r; - nread += r; - } - return nread; -} - -/* Like write(), but restarts after EINTR */ -ssize_t safewrite(int fd, const void *buf, size_t count) -{ - size_t nwritten = 0; - while (count > 0) { - int r = write(fd, buf, count); - - if (r < 0 && errno == EINTR) - continue; - if (r < 0) - return r; - if (r == 0) - return nwritten; - buf = (unsigned char *)buf + r; - count -= r; - nwritten += r; - } - return nwritten; -} - int __virFileReadAll(const char *path, int maxlen, diff --git a/src/util.h b/src/util.h index 7080ed0..23a016d 100644 --- a/src/util.h +++ b/src/util.h @@ -25,14 +25,14 @@ #define __VIR_UTIL_H__ #include "internal.h" +#include "util-lib.h" -int virExec(virConnectPtr conn, char **argv, int *retpid, int infd, int *outfd, int *errfd); -int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int infd, int *outfd, int *errfd); +int virExec(virConnectPtr conn, char **argv, int *retpid, + int infd, int *outfd, int *errfd); +int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, + int infd, int *outfd, int *errfd); int virRun(virConnectPtr conn, char **argv, int *status); -int saferead(int fd, void *buf, size_t count); -ssize_t safewrite(int fd, const void *buf, size_t count); - int __virFileReadAll(const char *path, int maxlen, char **buf); -- 1.5.4.2.185.gf5f8

Should we consider moving the buf.[ch] functions into this mini-util library too? +1 Rich. -- Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/ Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 03798903

On Fri, Feb 22, 2008 at 02:58:36PM +0100, Jim Meyering wrote:
Move safewrite and saferead to a separate file.
We currently use safewrite from inside libvirt and don't want to publish any such function name. However, we do want to use it in applications like virsh, libvirtd and libvirt_proxy that link with libvirt. To that end, this change moves that function definition (along with the nearly identical saferead) into a new file, util-lib.c. To avoid maintaining separate copies of even such small functions, we simply include that new file from util.c. Then, the separate applications that need to use safewrite simply compile and link with util-lib.c.
I realized this morning that as is this didn't worked well if virsh was compiled against a static version of the library, since the function would be redefined and an error shows up at link time. The enclosed patch is the simplest we could find with Jim to solve the issue, it just renames the safe function when compiled inside the library, Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

Daniel Veillard <veillard@redhat.com> wrote:
Move safewrite and saferead to a separate file. ... I realized this morning that as is this didn't worked well if virsh was compiled against a static version of the library, since the function would be redefined and an error shows up at link time. The enclosed patch is the simplest we could find with Jim to solve the issue, it just renames
On Fri, Feb 22, 2008 at 02:58:36PM +0100, Jim Meyering wrote: the safe function when compiled inside the library,
Index: src/Makefile.am ... - libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) + libvirt_la_CFLAGS = $(COVERAGE_CFLAGS) -DIN_LIBVIRT ... Index: src/util-lib.h ... + /* + * To avoid a double definition of the function when compiling + * programs using both util-lib and libvirt, like virsh + */ + #ifdef IN_LIBVIRT + #define saferead libvirt_saferead + #define safewrite libvirt_safewrite + #endif
Looks fine. Thanks for cleaning up after me.
participants (3)
-
Daniel Veillard
-
Jim Meyering
-
Richard W.M. Jones