stream fixes for migration
PATCH 1/1 stream fixes for migration
I've attached a patch for libxutil/libxc. This fixes one of the hangs I've seen during migrations. It applies against 2.0 and 2.0-testing.
Changes:
* Encountering EOF or error when xfrd reads from stream could cause an infinite loop.
* Cleaned up the closing of streams.
* Fixed several memory leaks.
Please consider applying.
tools/libxutil/file_stream.c +12 -5
@@ -151,7 +157,12 @@* @return result of the close */ static int file_close (IOStream *s){ -return fclose (get_file (s)); +int result =0 ; +if (s->data){ + result =fclose (get_file (s)); + s->data = (void *)0 ; + } +return result; }/** Free a file stream.
@@ -159,7 +170,7 @@* @param s file stream */ static void file_free (IOStream *s){ -// Do nothing - fclose does it all? +file_close (s); }/** Create an IOStream for a stream.
@@ -189,7 +200,6 @@ io =file_stream_new (fin);if (!io){fclose (fin); -//free(fin); // fclose frees ? } }return io;
@@ -199,13 +209,16 @@* * @param fd file descriptor * @param flags giving the mode to open in (as for fdopen()) -* @return new stream for the open file, or 0 if failed +* @return new stream for the open file, or 0 if failed. Always takes +* ownership of fd. */ IOStream *file_stream_fdopen (int fd,const char *flags){IOStream *io =0 ;FILE *fin =fdopen (fd, flags);if (fin){ - io =file_stream_new (fin); + io =file_stream_new (fin); +if (!io) +fclose (fin); }return io; }
tools/libxutil/gzip_stream.c +16 -9
@@ -107,7 +107,12 @@* @return result of the close */ static int gzip_close (IOStream *s){ -return gzclose (get_gzfile (s)); +int result =0 ; +if (s->data){ + result =gzclose (get_gzfile (s)); + s->data = (void *)0 ; + } +return result; }/** Free a gzip stream.
@@ -115,7 +120,7 @@* @param s gzip stream */ static void gzip_free (IOStream *s){ -// Do nothing - fclose does it all? +gzip_close (s); }/** Create an IOStream for a gzip stream.
@@ -143,11 +148,10 @@gzFile *fgz; fgz =gzopen (file, flags);if (fgz){ - io =gzip_stream_new (fgz); -if (!io){ -gzclose (fgz); -//free(fgz); // gzclose frees ? - } + io =gzip_stream_new (fgz); +if (!io){ +gzclose (fgz); + } }return io; }
@@ -156,14 +160,17 @@* * @param fd file descriptor * @param flags giving the mode to open in (as for fdopen()) -* @return new stream for the open file, or NULL if failed +* @return new stream for the open file, or NULL if failed. Always takes +* ownership of fd. */ IOStream *gzip_stream_fdopen (int fd,const char *flags){IOStream *io =NULL ;gzFile *fgz; fgz =gzdopen (fd, flags);if (fgz){ - io =gzip_stream_new (fgz); + io =gzip_stream_new (fgz); +if (!io) +gzclose (fgz); }return io; }
tools/libxutil/iostream.h +21 -11
@@ -105,8 +105,11 @@* @return if ok, number of bytes read, otherwise negative error code */ static inline int IOStream_read (IOStream *stream,void *buf,size_t n){ -int result =0 ; -if (stream->closed) goto exit; +int result; +if (stream->closed){ + result =IOSTREAM_EOF ; + goto exit; + }if (!stream->methods || !stream->methods->read){ result = -EINVAL ; goto exit;
@@ -124,11 +127,14 @@* @param stream input * @param buf where to put input * @param n number of bytes to write -* @return if ok, number of bytes read, otherwise negative error code +* @return if ok, number of bytes written, otherwise negative error code */ static inline int IOStream_write (IOStream *stream,const void *buf,size_t n){ -int result =0 ; -if (stream->closed) goto exit; +int result; +if (stream->closed){ + result =IOSTREAM_EOF ; + goto exit; + }if (!stream->methods || !stream->methods->write){ result = -EINVAL ; goto exit;
@@ -179,6 +185,7 @@int err =1 ;if (stream->methods && stream->methods->close){ err = stream->methods->close (stream); + stream->closed =1 ; }return err; }
@@ -189,7 +196,7 @@* @return 1 if closed, 0 otherwise */ static inline int IOStream_is_closed (IOStream *stream){ -return stream->closed; +return stream->closed; }/** Free the memory used by the stream.
@@ -197,11 +204,14 @@* @param stream to free */ static inline void IOStream_free (IOStream *stream){ -if (stream->methods && stream->methods->free){ - stream->methods->free (stream); - } - *stream = (IOStream ){}; -deallocate (stream); +if (!stream->closed && stream->methods && stream->methods->close){ + stream->methods->close (stream); + } +if (stream->methods && stream->methods->free){ + stream->methods->free (stream); + } + *stream = (IOStream ){}; +deallocate (stream); }
tools/libxc/xc_io.h +2 -2
@@ -45,14 +45,14 @@int rc; rc =IOStream_read (ctxt->io, buf, n); -return (rc == n ?0 : rc); +return (rc == n ?0 :-1 ); }static inline int xcio_write (XcIOContext *ctxt,void *buf,int n){int rc; rc =IOStream_write (ctxt->io, buf, n); -return (rc == n ?0 : rc); +return (rc == n ?0 :-1 ); }static inline int xcio_flush (XcIOContext *ctxt){
tools/libxc/xc_linux_save.c +1 -3
@@ -172,7 +172,6 @@struct timeval now;struct timespec delay;long long delta; -int rc;if (START_MBIT_RATE ==0 )return xcio_write (ioctxt, buf, n);
@@ -212,8 +211,7 @@ } } } - rc =IOStream_write (ioctxt->io, buf, n); -return (rc == n ?0 : rc); +return xcio_write (ioctxt, buf, n); } static int print_stats( int xc_handle, u32 domid,
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel
Quoted 6 lines · Charles Coffing
I've attached a patch for libxutil/libxc. This fixes one of the hangs I've seen during migrations. It applies against 2.0 and 2.0-testing.
Changes:
* Encountering EOF or error when xfrd reads from stream could cause an infinite loop.
* Cleaned up the closing of streams.
* Fixed several memory leaks.
Is it possible for IOStreams to be used concurrently, or are they only used in a single-threaded manner?
Signature · 1 line
David Hopwood <[email protected]>
Quoted 10 lines · David Hopwood
Quoted 6 lines · Charles Coffing
I've attached a patch for libxutil/libxc. This fixes one of the hangs I've seen during migrations. It applies against 2.0 and 2.0-testing.
Changes:
* Encountering EOF or error when xfrd reads from stream could cause an infinite loop.
* Cleaned up the closing of streams.
* Fixed several memory leaks.Is it possible for IOStreams to be used concurrently, or are they only used in a single-threaded manner?
doing anything to a c stream will work in a multithreaded environment; ie, the internal buffers/vars will not be stompped on.
It's in the spec(don't ask me where, I read it in the info docs).
Quoted 4 lines
doing anything to a c stream will work in a multithreaded environment; ie, the internal buffers/vars will not be stompped on.
It's in the
spec(don't ask me where, I read it in the info docs).
I think the issue is with Xfrd's IOStream type (libxutil/iostream.h), rather than with standard C streams.
(and I don't know if they're safe for multiple users, although I would expect that they're single thread-oriented)
Cheers,
Mark
Quoted 8 lines · Mark Williamson
Quoted 4 lines
doing anything to a c stream will work in a multithreaded environment; ie, the internal buffers/vars will not be stompped on.
It's in the
spec(don't ask me where, I read it in the info docs).I think the issue is with Xfrd's IOStream type (
libxutil/iostream.h), rather than with standard C streams.(and I don't know if they're safe for multiple users, although I would expect that they're single thread-oriented)
Well, the patch seemed to be assuming that they don't need to be safe for multiple threads. For example, the patch tries to make multiple closes safe, but this won't work if the closes are concurrent. I was just checking that no-one was expecting this to work.
Signature · 1 line
David Hopwood <[email protected]>