From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact() Date: Thu, 19 Feb 2015 10:04:57 +0000 Message-ID: <54E5B549.6050102@citrix.com> References: <1424284314-3619-1-git-send-email-andrew.cooper3@citrix.com> <1424339351.30924.23.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1424339351.30924.23.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Wei Liu , Ian Jackson , Xen-devel List-Id: xen-devel@lists.xenproject.org On 19/02/15 09:49, Ian Campbell wrote: > On Wed, 2015-02-18 at 18:31 +0000, Andrew Cooper wrote: >> This implementation of writev_exact() will cope with an iovcnt greater than >> IOV_MAX because glibc will actually let this work anyway, and it is very >> useful not to have to work about this in the caller of writev_exact(). The > s/work/worry? Oops yes. > > Will this still work with non-glibc libcs which do not cope with iovcnt Yes - observe the min() in the call to writev(). This code never actually breaks the writev() requirements, but allows the caller of writev_exact() to be more flexible. >> IOV_MAX? > In fact looking at the code it's not clear what glibc is letting work > anyway, do you pass a count > IOV_MAX to writev? Doesn't look like it. The page data algorithm in migration v2 submits 1024+7 IOV's at once for 1024 page frames and the associated metadata. It is specifically useful not to complicate that function with IOV_MAX adjustments in the case that IOV_MAX is 1024. ~Andrew > > >> caller is still required to ensure that the sum of iov_len's doesn't overflow >> a ssize_t. >> >> Signed-off-by: Andrew Cooper >> CC: Ian Campbell >> CC: Ian Jackson >> CC: Wei Liu >> >> --- >> v4: >> * Allow this to compile in a stubdom environment. >> v3: >> * Re-add adjustment for partial writes. >> * Split min/max adjustment into separate patch. >> v2: >> * Remove adjustment for partial writes of a specific iov[] entry. >> --- >> tools/libxc/xc_private.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++ >> tools/libxc/xc_private.h | 14 ++++++++ >> 2 files changed, 93 insertions(+) >> >> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c >> index df6cd9b..35c514a 100644 >> --- a/tools/libxc/xc_private.c >> +++ b/tools/libxc/xc_private.c >> @@ -860,6 +860,85 @@ int write_exact(int fd, const void *data, size_t size) >> return 0; >> } >> >> +#if defined(__MINIOS__) >> +/* >> + * MiniOS's libc doesn't know about writev(). Implement it as multiple write()s. >> + */ >> +int writev_exact(int fd, const struct iovec *iov, int iovcnt) >> +{ >> + int rc, i; >> + >> + for ( i = 0; i < iovcnt; ++i ) >> + { >> + rc = write_exact(fd, iov[i].iov_base, iov[i].iov_len); >> + if ( rc ) >> + return rc; >> + } >> + >> + return 0; >> +} >> +#else >> +int writev_exact(int fd, const struct iovec *iov, int iovcnt) >> +{ >> + struct iovec *local_iov = NULL; >> + int rc = 0, iov_idx = 0, saved_errno = 0; >> + ssize_t len; >> + >> + while ( iov_idx < iovcnt ) >> + { >> + /* Skip over iov[] entries with 0 length. */ >> + while ( iov[iov_idx].iov_len == 0 ) >> + if ( ++iov_idx == iovcnt ) >> + goto out; >> + >> + len = writev(fd, &iov[iov_idx], min(iovcnt - iov_idx, IOV_MAX)); >> + saved_errno = errno; >> + >> + if ( (len == -1) && (errno == EINTR) ) >> + continue; >> + if ( len <= 0 ) >> + { >> + rc = -1; >> + goto out; >> + } >> + >> + /* Check iov[] to see whether we had a partial or complete write. */ >> + while ( len > 0 && (iov_idx < iovcnt) ) >> + { >> + if ( len >= iov[iov_idx].iov_len ) >> + len -= iov[iov_idx++].iov_len; >> + else >> + { >> + /* Partial write of iov[iov_idx]. Copy iov so we can adjust >> + * element iov_idx and resubmit the rest. */ >> + if ( !local_iov ) >> + { >> + local_iov = malloc(iovcnt * sizeof(*iov)); >> + if ( !local_iov ) >> + { >> + saved_errno = ENOMEM; >> + goto out; >> + } >> + >> + iov = memcpy(local_iov, iov, iovcnt * sizeof(*iov)); >> + } >> + >> + local_iov[iov_idx].iov_base += len; >> + local_iov[iov_idx].iov_len -= len; >> + break; >> + } >> + } >> + } >> + >> + saved_errno = 0; >> + >> + out: >> + free(local_iov); >> + errno = saved_errno; >> + return rc; >> +} >> +#endif >> + >> int xc_ffs8(uint8_t x) >> { >> int i; >> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h >> index 45b8644..f74f7d7 100644 >> --- a/tools/libxc/xc_private.h >> +++ b/tools/libxc/xc_private.h >> @@ -42,6 +42,19 @@ >> #define VALGRIND_MAKE_MEM_UNDEFINED(addr, len) /* addr, len */ >> #endif >> >> +#if defined(__MINIOS__) >> +/* >> + * MiniOS's libc doesn't know about sys/uio.h or writev(). >> + * Declare enough of sys/uio.h to compile. >> + */ >> +struct iovec { >> + void *iov_base; >> + size_t iov_len; >> +}; >> +#else >> +#include >> +#endif >> + >> #define DECLARE_HYPERCALL privcmd_hypercall_t hypercall >> #define DECLARE_DOMCTL struct xen_domctl domctl >> #define DECLARE_SYSCTL struct xen_sysctl sysctl >> @@ -395,6 +408,7 @@ int xc_add_mmu_update(xc_interface *xch, struct xc_mmu *mmu, >> /* Return 0 on success; -1 on error setting errno. */ >> int read_exact(int fd, void *data, size_t size); /* EOF => -1, errno=0 */ >> int write_exact(int fd, const void *data, size_t size); >> +int writev_exact(int fd, const struct iovec *iov, int iovcnt); >> >> int xc_ffs8(uint8_t x); >> int xc_ffs16(uint16_t x); >