tools/libxs: Rework xs_talkv() to take xsd_sockmsg within the iovec

We would like to writev() the whole outgoing message, but this is hard given
the current need to prepend the locally-constructed xsd_sockmsg.

Instead, have the caller provide xsd_sockmsg in iovec[0].  This in turn drops
the 't' and 'type' parameters from xs_talkv().

Note that xs_talkv() may alter the iovec structure.  This may happen when
writev() is really used under the covers, and it's preferable to having the
lower levels need to duplicate the iovec to edit it upon encountering a short
write.  xs_directory_part() is the only function impacted by this, and it's
easy to rearrange to be compatible.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
master commit: e2a93bed8b9e0f0c4779dcd4b10dc7ba2a959fbc
master date: 2024-07-23 15:11:27 +0100

diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 4c37de437bfb217e5ed059c363ac030818a81bcd..4d24bf7b0ff6a8d4133784e0e8331a8b2e1f8bb8 100644 (file)
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -566,25 +566,32 @@ static void *read_reply(
        return body;
 }
 
-/* Send message to xs, get malloc'ed reply.  NULL and set errno on error. */
-static void *xs_talkv(struct xs_handle *h, xs_transaction_t t,
-                     enum xsd_sockmsg_type type,
-                     const struct iovec *iovec,
+/*
+ * Send message to xenstore, get malloc'ed reply.  NULL and set errno on error.
+ *
+ * @iovec describes the entire outgoing message, starting with the xsd_sockmsg
+ * header.  xs_talkv() calculates the outgoing message length, updating
+ * xsd_sockmsg in element 0.  xs_talkv() might edit the iovec structure in
+ * place (e.g. following short writes).
+ */
+static void *xs_talkv(struct xs_handle *h,
+                     struct iovec *iovec,
                      unsigned int num_vecs,
                      unsigned int *len)
 {
-       struct xsd_sockmsg msg;
+       struct xsd_sockmsg *msg = iovec[0].iov_base;
+       enum xsd_sockmsg_type reply_type;
        void *ret = NULL;
        int saved_errno;
        unsigned int i, msg_len;
        struct sigaction ignorepipe, oldact;
 
-       msg.tx_id = t;
-       msg.req_id = 0;
-       msg.type = type;
+       /* Element 0 must be xsd_sockmsg */
+       assert(num_vecs >= 1);
+       assert(iovec[0].iov_len == sizeof(*msg));
 
        /* Calculate the payload length by summing iovec elements */
-       for (i = 0, msg_len = 0; i < num_vecs; i++) {
+       for (i = 1, msg_len = 0; i < num_vecs; i++) {
                if ((iovec[i].iov_len > XENSTORE_PAYLOAD_MAX) ||
                    ((msg_len += iovec[i].iov_len) > XENSTORE_PAYLOAD_MAX)) {
                        errno = E2BIG;
@@ -592,7 +599,7 @@ static void *xs_talkv(struct xs_handle *h, xs_transaction_t t,
                }
        }
 
-       msg.len = msg_len;
+       msg->len = msg_len;
 
        ignorepipe.sa_handler = SIG_IGN;
        sigemptyset(&ignorepipe.sa_mask);
@@ -601,28 +608,25 @@ static void *xs_talkv(struct xs_handle *h, xs_transaction_t t,
 
        mutex_lock(&h->request_mutex);
 
-       if (!xs_write_all(h->fd, &msg, sizeof(msg)))
-               goto fail;
-
        for (i = 0; i < num_vecs; i++)
                if (!xs_write_all(h->fd, iovec[i].iov_base, iovec[i].iov_len))
                        goto fail;
 
-       ret = read_reply(h, &msg.type, len);
+       ret = read_reply(h, &reply_type, len);
        if (!ret)
                goto fail;
 
        mutex_unlock(&h->request_mutex);
 
        sigaction(SIGPIPE, &oldact, NULL);
-       if (msg.type == XS_ERROR) {
+       if (reply_type == XS_ERROR) {
                saved_errno = get_error(ret);
                free(ret);
                errno = saved_errno;
                return NULL;
        }
 
-       if (msg.type != type) {
+       if (reply_type != msg->type) {
                free(ret);
                saved_errno = EBADF;
                goto close_fd;
@@ -655,11 +659,15 @@ static void *xs_single(struct xs_handle *h, xs_transaction_t t,
                       const char *string,
                       unsigned int *len)
 {
-       struct iovec iovec;
+       struct xsd_sockmsg msg = { .type = type, .tx_id = t };
+       struct iovec iov[2];
+
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = (void *)string;
+       iov[1].iov_len  = strlen(string) + 1;
 
-       iovec.iov_base = (void *)string;
-       iovec.iov_len = strlen(string) + 1;
-       return xs_talkv(h, t, type, &iovec, 1, len);
+       return xs_talkv(h, iov, ARRAY_SIZE(iov), len);
 }
 
 static bool xs_bool(char *reply)
@@ -696,21 +704,25 @@ static char **xs_directory_common(char *strings, unsigned int len,
 static char **xs_directory_part(struct xs_handle *h, xs_transaction_t t,
                                const char *path, unsigned int *num)
 {
+       struct xsd_sockmsg msg = { .type = XS_DIRECTORY_PART, .tx_id = t };
        unsigned int off, result_len;
        char gen[24], offstr[8];
-       struct iovec iovec[2];
+       struct iovec iov[3];
        char *result = NULL, *strings = NULL;
 
        memset(gen, 0, sizeof(gen));
-       iovec[0].iov_base = (void *)path;
-       iovec[0].iov_len = strlen(path) + 1;
 
        for (off = 0;;) {
                snprintf(offstr, sizeof(offstr), "%u", off);
-               iovec[1].iov_base = (void *)offstr;
-               iovec[1].iov_len = strlen(offstr) + 1;
-               result = xs_talkv(h, t, XS_DIRECTORY_PART, iovec, 2,
-                                 &result_len);
+
+               iov[0].iov_base = &msg;
+               iov[0].iov_len  = sizeof(msg);
+               iov[1].iov_base = (void *)path;
+               iov[1].iov_len  = strlen(path) + 1;
+               iov[2].iov_base = (void *)offstr;
+               iov[2].iov_len  = strlen(offstr) + 1;
+
+               result = xs_talkv(h, iov, ARRAY_SIZE(iov), &result_len);
 
                /* If XS_DIRECTORY_PART isn't supported return E2BIG. */
                if (!result) {
@@ -779,15 +791,17 @@ void *xs_read(struct xs_handle *h, xs_transaction_t t,
 bool xs_write(struct xs_handle *h, xs_transaction_t t,
              const char *path, const void *data, unsigned int len)
 {
-       struct iovec iovec[2];
+       struct xsd_sockmsg msg = { .type = XS_WRITE, .tx_id = t };
+       struct iovec iov[3];
 
-       iovec[0].iov_base = (void *)path;
-       iovec[0].iov_len = strlen(path) + 1;
-       iovec[1].iov_base = (void *)data;
-       iovec[1].iov_len = len;
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = (void *)path;
+       iov[1].iov_len  = strlen(path) + 1;
+       iov[2].iov_base = (void *)data;
+       iov[2].iov_len  = len;
 
-       return xs_bool(xs_talkv(h, t, XS_WRITE, iovec,
-                               ARRAY_SIZE(iovec), NULL));
+       return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
 }
 
 /* Create a new directory.
@@ -851,34 +865,37 @@ bool xs_set_permissions(struct xs_handle *h,
                        struct xs_permissions *perms,
                        unsigned int num_perms)
 {
+       struct xsd_sockmsg msg = { .type = XS_SET_PERMS, .tx_id = t };
        unsigned int i;
-       struct iovec iov[1+num_perms];
+       struct iovec iov[2 + num_perms];
+
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = (void *)path;
+       iov[1].iov_len  = strlen(path) + 1;
 
-       iov[0].iov_base = (void *)path;
-       iov[0].iov_len = strlen(path) + 1;
-       
        for (i = 0; i < num_perms; i++) {
                char buffer[MAX_STRLEN(unsigned int)+1];
 
                if (!xenstore_perm_to_string(&perms[i], buffer, sizeof(buffer)))
                        goto unwind;
 
-               iov[i+1].iov_base = strdup(buffer);
-               iov[i+1].iov_len = strlen(buffer) + 1;
+               iov[i + 2].iov_base = strdup(buffer);
+               iov[i + 2].iov_len  = strlen(buffer) + 1;
                if (!iov[i+1].iov_base)
                        goto unwind;
        }
 
-       if (!xs_bool(xs_talkv(h, t, XS_SET_PERMS, iov, 1+num_perms, NULL)))
+       if (!xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL)))
                goto unwind;
        for (i = 0; i < num_perms; i++)
-               free(iov[i+1].iov_base);
+               free(iov[i + 2].iov_base);
        return true;
 
 unwind:
        num_perms = i;
        for (i = 0; i < num_perms; i++)
-               free_no_errno(iov[i+1].iov_base);
+               free_no_errno(iov[i + 2].iov_base);
        return false;
 }
 
@@ -895,7 +912,8 @@ bool xs_restrict(struct xs_handle *h, unsigned domid)
  */
 bool xs_watch(struct xs_handle *h, const char *path, const char *token)
 {
-       struct iovec iov[2];
+       struct xsd_sockmsg msg = { .type = XS_WATCH };
+       struct iovec iov[3];
 
 #ifdef USE_PTHREAD
 #define DEFAULT_THREAD_STACKSIZE (16 * 1024)
@@ -953,13 +971,14 @@ bool xs_watch(struct xs_handle *h, const char *path, const char *token)
        mutex_unlock(&h->request_mutex);
 #endif
 
-       iov[0].iov_base = (void *)path;
-       iov[0].iov_len = strlen(path) + 1;
-       iov[1].iov_base = (void *)token;
-       iov[1].iov_len = strlen(token) + 1;
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = (void *)path;
+       iov[1].iov_len  = strlen(path) + 1;
+       iov[2].iov_base = (void *)token;
+       iov[2].iov_len  = strlen(token) + 1;
 
-       return xs_bool(xs_talkv(h, XBT_NULL, XS_WATCH, iov,
-                               ARRAY_SIZE(iov), NULL));
+       return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
 }
 
 
@@ -1072,20 +1091,22 @@ char **xs_read_watch(struct xs_handle *h, unsigned int *num)
  */
 bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
 {
-       struct iovec iov[2];
+       struct xsd_sockmsg sockmsg = { .type = XS_UNWATCH };
+       struct iovec iov[3];
        struct xs_stored_msg *msg, *tmsg;
        bool res;
        char *s, *p;
        unsigned int i;
        char *l_token, *l_path;
 
-       iov[0].iov_base = (char *)path;
-       iov[0].iov_len = strlen(path) + 1;
-       iov[1].iov_base = (char *)token;
-       iov[1].iov_len = strlen(token) + 1;
+       iov[0].iov_base = &sockmsg;
+       iov[0].iov_len  = sizeof(sockmsg);
+       iov[1].iov_base = (char *)path;
+       iov[1].iov_len  = strlen(path) + 1;
+       iov[2].iov_base = (char *)token;
+       iov[2].iov_len  = strlen(token) + 1;
 
-       res = xs_bool(xs_talkv(h, XBT_NULL, XS_UNWATCH, iov,
-                              ARRAY_SIZE(iov), NULL));
+       res = xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
 
        if (!h->unwatch_filter) /* Don't filter the watch list */
                return res;
@@ -1178,43 +1199,47 @@ bool xs_introduce_domain(struct xs_handle *h,
                         unsigned int domid, unsigned long mfn,
                         unsigned int eventchn)
 {
+       struct xsd_sockmsg msg = { .type = XS_INTRODUCE };
        char domid_str[MAX_STRLEN(domid)];
        char mfn_str[MAX_STRLEN(mfn)];
        char eventchn_str[MAX_STRLEN(eventchn)];
-       struct iovec iov[3];
+       struct iovec iov[4];
 
        snprintf(domid_str, sizeof(domid_str), "%u", domid);
        snprintf(mfn_str, sizeof(mfn_str), "%lu", mfn);
        snprintf(eventchn_str, sizeof(eventchn_str), "%u", eventchn);
 
-       iov[0].iov_base = domid_str;
-       iov[0].iov_len = strlen(domid_str) + 1;
-       iov[1].iov_base = mfn_str;
-       iov[1].iov_len = strlen(mfn_str) + 1;
-       iov[2].iov_base = eventchn_str;
-       iov[2].iov_len = strlen(eventchn_str) + 1;
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = domid_str;
+       iov[1].iov_len  = strlen(domid_str) + 1;
+       iov[2].iov_base = mfn_str;
+       iov[2].iov_len  = strlen(mfn_str) + 1;
+       iov[3].iov_base = eventchn_str;
+       iov[3].iov_len  = strlen(eventchn_str) + 1;
 
-       return xs_bool(xs_talkv(h, XBT_NULL, XS_INTRODUCE, iov,
-                               ARRAY_SIZE(iov), NULL));
+       return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
 }
 
 bool xs_set_target(struct xs_handle *h,
                         unsigned int domid, unsigned int target)
 {
+       struct xsd_sockmsg msg = { .type = XS_SET_TARGET };
        char domid_str[MAX_STRLEN(domid)];
        char target_str[MAX_STRLEN(target)];
-       struct iovec iov[2];
+       struct iovec iov[3];
 
        snprintf(domid_str, sizeof(domid_str), "%u", domid);
        snprintf(target_str, sizeof(target_str), "%u", target);
 
-       iov[0].iov_base = domid_str;
-       iov[0].iov_len = strlen(domid_str) + 1;
-       iov[1].iov_base = target_str;
-       iov[1].iov_len = strlen(target_str) + 1;
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = domid_str;
+       iov[1].iov_len  = strlen(domid_str) + 1;
+       iov[2].iov_base = target_str;
+       iov[2].iov_len  = strlen(target_str) + 1;
 
-       return xs_bool(xs_talkv(h, XBT_NULL, XS_SET_TARGET, iov,
-                               ARRAY_SIZE(iov), NULL));
+       return xs_bool(xs_talkv(h, iov, ARRAY_SIZE(iov), NULL));
 }
 
 static void * single_with_domid(struct xs_handle *h,
@@ -1310,15 +1335,17 @@ out:
 char *xs_control_command(struct xs_handle *h, const char *cmd,
                         void *data, unsigned int len)
 {
-       struct iovec iov[2];
+       struct xsd_sockmsg msg = { .type = XS_CONTROL };
+       struct iovec iov[3];
 
-       iov[0].iov_base = (void *)cmd;
-       iov[0].iov_len = strlen(cmd) + 1;
-       iov[1].iov_base = data;
-       iov[1].iov_len = len;
+       iov[0].iov_base = &msg;
+       iov[0].iov_len  = sizeof(msg);
+       iov[1].iov_base = (void *)cmd;
+       iov[1].iov_len  = strlen(cmd) + 1;
+       iov[2].iov_base = data;
+       iov[2].iov_len  = len;
 
-       return xs_talkv(h, XBT_NULL, XS_CONTROL, iov,
-                       ARRAY_SIZE(iov), NULL);
+       return xs_talkv(h, iov, ARRAY_SIZE(iov), NULL);
 }
 
 char *xs_debug_command(struct xs_handle *h, const char *cmd,