* [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact()
@ 2015-02-18 18:31 Andrew Cooper
2015-02-19 9:49 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2015-02-18 18:31 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Ian Campbell, Wei Liu
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
caller is still required to ensure that the sum of iov_len's doesn't overflow
a ssize_t.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
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 <sys/uio.h>
+#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);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact()
2015-02-18 18:31 [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact() Andrew Cooper
@ 2015-02-19 9:49 ` Ian Campbell
2015-02-19 10:04 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2015-02-19 9:49 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Xen-devel
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?
Will this still work with non-glibc libcs which do not cope with iovcnt
> 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.
> caller is still required to ensure that the sum of iov_len's doesn't overflow
> a ssize_t.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
>
> ---
> 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 <sys/uio.h>
> +#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);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact()
2015-02-19 9:49 ` Ian Campbell
@ 2015-02-19 10:04 ` Andrew Cooper
2015-02-19 10:18 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2015-02-19 10:04 UTC (permalink / raw)
To: Ian Campbell; +Cc: Wei Liu, Ian Jackson, Xen-devel
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 <andrew.cooper3@citrix.com>
>> CC: Ian Campbell <Ian.Campbell@citrix.com>
>> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>>
>> ---
>> 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 <sys/uio.h>
>> +#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);
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact()
2015-02-19 10:04 ` Andrew Cooper
@ 2015-02-19 10:18 ` Ian Campbell
2015-02-19 10:23 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2015-02-19 10:18 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Xen-devel
On Thu, 2015-02-19 at 10:04 +0000, Andrew Cooper wrote:
> 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.
I see that, but where does "because glibc will actually let this work
anyway" come into it? This function will never actually pass
iovcnt>IOV_MAX to glibc, so I'm not sure what you are trying to say with
that statement in the commit log, since glibc is not being expected to
cope or "let this work" AFAICT.
If what you are trying to say is that this function is more flexible
than the underlying writev implementation is required to be by POSIX or
something then you should just say that. Conflate it with the underlying
writev implementation coping with such things is just adding confusion
to the commit message IMHO.
Ian.
>
> ~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 <andrew.cooper3@citrix.com>
> >> CC: Ian Campbell <Ian.Campbell@citrix.com>
> >> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >>
> >> ---
> >> 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 <sys/uio.h>
> >> +#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);
> >
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact()
2015-02-19 10:18 ` Ian Campbell
@ 2015-02-19 10:23 ` Andrew Cooper
0 siblings, 0 replies; 5+ messages in thread
From: Andrew Cooper @ 2015-02-19 10:23 UTC (permalink / raw)
To: Ian Campbell; +Cc: Wei Liu, Ian Jackson, Xen-devel
On 19/02/15 10:18, Ian Campbell wrote:
> On Thu, 2015-02-19 at 10:04 +0000, Andrew Cooper wrote:
>> 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.
> I see that, but where does "because glibc will actually let this work
> anyway" come into it? This function will never actually pass
> iovcnt>IOV_MAX to glibc, so I'm not sure what you are trying to say with
> that statement in the commit log, since glibc is not being expected to
> cope or "let this work" AFAICT.
>
> If what you are trying to say is that this function is more flexible
> than the underlying writev implementation is required to be by POSIX or
> something then you should just say that. Conflate it with the underlying
> writev implementation coping with such things is just adding confusion
> to the commit message IMHO.
>
> Ian.
Now that you point this out, that part is stale, and more obviously
applied to an older iteration of the patch. I will have a go at
rewriting the message from scratch and see if I can come to something
more appropriate.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-02-19 10:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-18 18:31 [PATCH v4] tools/libxc: Implement writev_exact() in the same style as write_exact() Andrew Cooper
2015-02-19 9:49 ` Ian Campbell
2015-02-19 10:04 ` Andrew Cooper
2015-02-19 10:18 ` Ian Campbell
2015-02-19 10:23 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.