* [PATCH v3 01/16] tools/xenstore: verify command line parameters better
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-05-30 8:54 ` [PATCH v3 02/16] tools/xenstore: do some cleanup of hashtable.c Juergen Gross
` (15 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD, Julien Grall
Add some more verification of command line parameters.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
tools/xenstore/xenstored_core.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 6f03f6687a..22df395aac 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2820,7 +2820,7 @@ int main(int argc, char *argv[])
no_domain_init = true;
break;
case 'E':
- hard_quotas[ACC_NODES].val = strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_NODES].val = get_optval_uint(optarg);
break;
case 'F':
pidfile = optarg;
@@ -2838,10 +2838,10 @@ int main(int argc, char *argv[])
recovery = false;
break;
case 'S':
- hard_quotas[ACC_NODESZ].val = strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_NODESZ].val = get_optval_uint(optarg);
break;
case 't':
- hard_quotas[ACC_TRANS].val = strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_TRANS].val = get_optval_uint(optarg);
break;
case 'T':
tracefile = optarg;
@@ -2861,14 +2861,13 @@ int main(int argc, char *argv[])
verbose = true;
break;
case 'W':
- hard_quotas[ACC_WATCH].val = strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_WATCH].val = get_optval_uint(optarg);
break;
case 'A':
- hard_quotas[ACC_NPERM].val = strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_NPERM].val = get_optval_uint(optarg);
break;
case 'M':
- hard_quotas[ACC_PATHLEN].val =
- strtoul(optarg, NULL, 10);
+ hard_quotas[ACC_PATHLEN].val = get_optval_uint(optarg);
hard_quotas[ACC_PATHLEN].val =
min((unsigned int)XENSTORE_REL_PATH_MAX,
hard_quotas[ACC_PATHLEN].val);
@@ -2883,13 +2882,13 @@ int main(int argc, char *argv[])
set_timeout(optarg);
break;
case 'e':
- dom0_event = strtol(optarg, NULL, 10);
+ dom0_event = get_optval_uint(optarg);
break;
case 'm':
- dom0_domid = strtol(optarg, NULL, 10);
+ dom0_domid = get_optval_uint(optarg);
break;
case 'p':
- priv_domid = strtol(optarg, NULL, 10);
+ priv_domid = get_optval_uint(optarg);
break;
#ifndef NO_LIVE_UPDATE
case 'U':
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 02/16] tools/xenstore: do some cleanup of hashtable.c
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
2023-05-30 8:54 ` [PATCH v3 01/16] tools/xenstore: verify command line parameters better Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-05-30 8:54 ` [PATCH v3 03/16] tools/xenstore: modify interface of create_hashtable() Juergen Gross
` (14 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD, Julien Grall
Do the following cleanups:
- hashtable_count() isn't used at all, so remove it
- replace prime_table_length and max_load_factor with macros
- make hash() static
- add a loadlimit() helper function
- remove the /***/ lines between functions
- do some style corrections
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
---
V3:
- more style corrections (Julien Grall)
---
tools/xenstore/hashtable.c | 76 +++++++++++++++-----------------------
tools/xenstore/hashtable.h | 10 -----
2 files changed, 30 insertions(+), 56 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index 3d4466b597..dc209158fa 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -40,22 +40,25 @@ static const unsigned int primes[] = {
50331653, 100663319, 201326611, 402653189,
805306457, 1610612741
};
-const unsigned int prime_table_length = sizeof(primes)/sizeof(primes[0]);
-const unsigned int max_load_factor = 65; /* percentage */
-/*****************************************************************************/
-/* indexFor */
-static inline unsigned int
-indexFor(unsigned int tablelength, unsigned int hashvalue) {
+#define PRIME_TABLE_LEN ARRAY_SIZE(primes)
+#define MAX_LOAD_PERCENT 65
+
+static inline unsigned int indexFor(unsigned int tablelength,
+ unsigned int hashvalue)
+{
return (hashvalue % tablelength);
}
-/*****************************************************************************/
-struct hashtable *
-create_hashtable(const void *ctx, unsigned int minsize,
- unsigned int (*hashf) (const void *),
- int (*eqf) (const void *, const void *),
- unsigned int flags)
+static unsigned int loadlimit(unsigned int pindex)
+{
+ return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
+}
+
+struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
+ unsigned int (*hashf) (const void *),
+ int (*eqf) (const void *, const void *),
+ unsigned int flags)
{
struct hashtable *h;
unsigned int pindex, size = primes[0];
@@ -64,8 +67,11 @@ create_hashtable(const void *ctx, unsigned int minsize,
if (minsize > (1u << 30)) return NULL;
/* Enforce size as prime */
- for (pindex=0; pindex < prime_table_length; pindex++) {
- if (primes[pindex] > minsize) { size = primes[pindex]; break; }
+ for (pindex = 0; pindex < PRIME_TABLE_LEN; pindex++) {
+ if (primes[pindex] > minsize) {
+ size = primes[pindex];
+ break;
+ }
}
h = talloc_zero(ctx, struct hashtable);
@@ -81,7 +87,7 @@ create_hashtable(const void *ctx, unsigned int minsize,
h->entrycount = 0;
h->hashfn = hashf;
h->eqfn = eqf;
- h->loadlimit = (unsigned int)(((uint64_t)size * max_load_factor) / 100);
+ h->loadlimit = loadlimit(pindex);
return h;
err1:
@@ -90,9 +96,7 @@ err0:
return NULL;
}
-/*****************************************************************************/
-unsigned int
-hash(const struct hashtable *h, const void *k)
+static unsigned int hash(const struct hashtable *h, const void *k)
{
/* Aim to protect against poor hash functions by adding logic here
* - logic taken from java 1.4 hashtable source */
@@ -104,9 +108,7 @@ hash(const struct hashtable *h, const void *k)
return i;
}
-/*****************************************************************************/
-static int
-hashtable_expand(struct hashtable *h)
+static int hashtable_expand(struct hashtable *h)
{
/* Double the size of the table to accomodate more entries */
struct entry **newtable;
@@ -114,7 +116,7 @@ hashtable_expand(struct hashtable *h)
struct entry **pE;
unsigned int newsize, i, index;
/* Check we're not hitting max capacity */
- if (h->primeindex == (prime_table_length - 1)) return 0;
+ if (h->primeindex == (PRIME_TABLE_LEN - 1)) return 0;
newsize = primes[++(h->primeindex)];
newtable = talloc_realloc(h, h->table, struct entry *, newsize);
@@ -144,21 +146,11 @@ hashtable_expand(struct hashtable *h)
}
h->tablelength = newsize;
- h->loadlimit = (unsigned int)
- (((uint64_t)newsize * max_load_factor) / 100);
+ h->loadlimit = loadlimit(h->primeindex);
return -1;
}
-/*****************************************************************************/
-unsigned int
-hashtable_count(const struct hashtable *h)
-{
- return h->entrycount;
-}
-
-/*****************************************************************************/
-int
-hashtable_insert(struct hashtable *h, void *k, void *v)
+int hashtable_insert(struct hashtable *h, void *k, void *v)
{
/* This method allows duplicate keys - but they shouldn't be used */
unsigned int index;
@@ -186,9 +178,7 @@ hashtable_insert(struct hashtable *h, void *k, void *v)
return -1;
}
-/*****************************************************************************/
-void * /* returns value associated with key */
-hashtable_search(const struct hashtable *h, const void *k)
+void *hashtable_search(const struct hashtable *h, const void *k)
{
struct entry *e;
unsigned int hashvalue, index;
@@ -204,7 +194,6 @@ hashtable_search(const struct hashtable *h, const void *k)
return NULL;
}
-/*****************************************************************************/
void
hashtable_remove(struct hashtable *h, const void *k)
{
@@ -234,10 +223,8 @@ hashtable_remove(struct hashtable *h, const void *k)
}
}
-/*****************************************************************************/
-int
-hashtable_iterate(struct hashtable *h,
- int (*func)(const void *k, void *v, void *arg), void *arg)
+int hashtable_iterate(struct hashtable *h,
+ int (*func)(const void *k, void *v, void *arg), void *arg)
{
int ret;
unsigned int i;
@@ -260,10 +247,7 @@ hashtable_iterate(struct hashtable *h,
return 0;
}
-/*****************************************************************************/
-/* destroy */
-void
-hashtable_destroy(struct hashtable *h)
+void hashtable_destroy(struct hashtable *h)
{
talloc_free(h);
}
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index cc0090f133..04310783b6 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -74,16 +74,6 @@ hashtable_search(const struct hashtable *h, const void *k);
void
hashtable_remove(struct hashtable *h, const void *k);
-/*****************************************************************************
- * hashtable_count
-
- * @name hashtable_count
- * @param h the hashtable
- * @return the number of items stored in the hashtable
- */
-unsigned int
-hashtable_count(const struct hashtable *h);
-
/*****************************************************************************
* hashtable_iterate
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 03/16] tools/xenstore: modify interface of create_hashtable()
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
2023-05-30 8:54 ` [PATCH v3 01/16] tools/xenstore: verify command line parameters better Juergen Gross
2023-05-30 8:54 ` [PATCH v3 02/16] tools/xenstore: do some cleanup of hashtable.c Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 17:52 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success Juergen Gross
` (13 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
The minsize parameter of create_hashtable() doesn't have any real use
case for Xenstore, so drop it.
For better talloc_report_full() diagnostic output add a name parameter
to create_hashtable().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- make code more readable (Julien Grall)
---
tools/xenstore/hashtable.c | 23 ++++++-----------------
tools/xenstore/hashtable.h | 4 ++--
tools/xenstore/xenstored_core.c | 2 +-
tools/xenstore/xenstored_domain.c | 4 ++--
4 files changed, 11 insertions(+), 22 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index dc209158fa..3d2d3a0e22 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -55,39 +55,28 @@ static unsigned int loadlimit(unsigned int pindex)
return ((uint64_t)primes[pindex] * MAX_LOAD_PERCENT) / 100;
}
-struct hashtable *create_hashtable(const void *ctx, unsigned int minsize,
+struct hashtable *create_hashtable(const void *ctx, const char *name,
unsigned int (*hashf) (const void *),
int (*eqf) (const void *, const void *),
unsigned int flags)
{
struct hashtable *h;
- unsigned int pindex, size = primes[0];
-
- /* Check requested hashtable isn't too large */
- if (minsize > (1u << 30)) return NULL;
-
- /* Enforce size as prime */
- for (pindex = 0; pindex < PRIME_TABLE_LEN; pindex++) {
- if (primes[pindex] > minsize) {
- size = primes[pindex];
- break;
- }
- }
h = talloc_zero(ctx, struct hashtable);
if (NULL == h)
goto err0;
- h->table = talloc_zero_array(h, struct entry *, size);
+ talloc_set_name_const(h, name);
+ h->table = talloc_zero_array(h, struct entry *, primes[0]);
if (NULL == h->table)
goto err1;
- h->tablelength = size;
+ h->primeindex = 0;
+ h->tablelength = primes[h->primeindex];
h->flags = flags;
- h->primeindex = pindex;
h->entrycount = 0;
h->hashfn = hashf;
h->eqfn = eqf;
- h->loadlimit = loadlimit(pindex);
+ h->loadlimit = loadlimit(h->primeindex);
return h;
err1:
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index 04310783b6..0e1a6f61c2 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -10,7 +10,7 @@ struct hashtable;
* @name create_hashtable
* @param ctx talloc context to use for allocations
- * @param minsize minimum initial size of hashtable
+ * @param name talloc name of the hashtable
* @param hashfunction function for hashing keys
* @param key_eq_fn function for determining key equality
* @param flags flags HASHTABLE_*
@@ -23,7 +23,7 @@ struct hashtable;
#define HASHTABLE_FREE_KEY (1U << 1)
struct hashtable *
-create_hashtable(const void *ctx, unsigned int minsize,
+create_hashtable(const void *ctx, const char *name,
unsigned int (*hashfunction) (const void *),
int (*key_eq_fn) (const void *, const void *),
unsigned int flags
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 22df395aac..418790d8d7 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2521,7 +2521,7 @@ void check_store(void)
struct check_store_data data;
/* Don't free values (they are all void *1) */
- data.reachable = create_hashtable(NULL, 16, hash_from_key_fn,
+ data.reachable = create_hashtable(NULL, "checkstore", hash_from_key_fn,
keys_equal_fn, HASHTABLE_FREE_KEY);
if (!data.reachable) {
log("check_store: ENOMEM");
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index a641f633a5..a1c91ef3f3 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1015,7 +1015,7 @@ void domain_init(int evtfd)
int rc;
/* Start with a random rather low domain count for the hashtable. */
- domhash = create_hashtable(NULL, 8, domhash_fn, domeq_fn, 0);
+ domhash = create_hashtable(NULL, "domains", domhash_fn, domeq_fn, 0);
if (!domhash)
barf_perror("Failed to allocate domain hashtable");
@@ -1807,7 +1807,7 @@ struct hashtable *domain_check_acc_init(void)
{
struct hashtable *domains;
- domains = create_hashtable(NULL, 8, domhash_fn, domeq_fn,
+ domains = create_hashtable(NULL, "domain_check", domhash_fn, domeq_fn,
HASHTABLE_FREE_VALUE);
if (!domains)
return NULL;
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (2 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 03/16] tools/xenstore: modify interface of create_hashtable() Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 17:56 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 05/16] tools/xenstore: make some write limit functions static Juergen Gross
` (12 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Today hashtable_insert() returns 0 in case of an error. Change that to
let it return an errno value in the error case and 0 in case of success.
In order to avoid any missed return value checks or related future
backport errors, rename hashtable_insert() to hashtable_add().
Even if not used today, do the same switch for the return value of
hashtable_expand().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- rename to hashtable_add() (triggered by Julien Grall)
---
tools/xenstore/hashtable.c | 17 +++++++++++------
tools/xenstore/hashtable.h | 8 ++++----
tools/xenstore/xenstored_core.c | 6 +++---
tools/xenstore/xenstored_domain.c | 4 ++--
tools/xenstore/xenstored_transaction.c | 4 ++--
5 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/tools/xenstore/hashtable.c b/tools/xenstore/hashtable.c
index 3d2d3a0e22..11f6bf8f15 100644
--- a/tools/xenstore/hashtable.c
+++ b/tools/xenstore/hashtable.c
@@ -105,14 +105,15 @@ static int hashtable_expand(struct hashtable *h)
struct entry **pE;
unsigned int newsize, i, index;
/* Check we're not hitting max capacity */
- if (h->primeindex == (PRIME_TABLE_LEN - 1)) return 0;
+ if (h->primeindex == (PRIME_TABLE_LEN - 1))
+ return ENOSPC;
newsize = primes[++(h->primeindex)];
newtable = talloc_realloc(h, h->table, struct entry *, newsize);
if (!newtable)
{
h->primeindex--;
- return 0;
+ return ENOMEM;
}
h->table = newtable;
@@ -136,10 +137,10 @@ static int hashtable_expand(struct hashtable *h)
h->tablelength = newsize;
h->loadlimit = loadlimit(h->primeindex);
- return -1;
+ return 0;
}
-int hashtable_insert(struct hashtable *h, void *k, void *v)
+int hashtable_add(struct hashtable *h, void *k, void *v)
{
/* This method allows duplicate keys - but they shouldn't be used */
unsigned int index;
@@ -153,7 +154,11 @@ int hashtable_insert(struct hashtable *h, void *k, void *v)
hashtable_expand(h);
}
e = talloc_zero(h, struct entry);
- if (NULL == e) { --(h->entrycount); return 0; } /*oom*/
+ if (NULL == e)
+ {
+ --h->entrycount;
+ return ENOMEM;
+ }
e->h = hash(h,k);
index = indexFor(h->tablelength,e->h);
e->k = k;
@@ -164,7 +169,7 @@ int hashtable_insert(struct hashtable *h, void *k, void *v)
talloc_steal(e, v);
e->next = h->table[index];
h->table[index] = e;
- return -1;
+ return 0;
}
void *hashtable_search(const struct hashtable *h, const void *k)
diff --git a/tools/xenstore/hashtable.h b/tools/xenstore/hashtable.h
index 0e1a6f61c2..5a2cc4a4be 100644
--- a/tools/xenstore/hashtable.h
+++ b/tools/xenstore/hashtable.h
@@ -30,13 +30,13 @@ create_hashtable(const void *ctx, const char *name,
);
/*****************************************************************************
- * hashtable_insert
+ * hashtable_add
- * @name hashtable_insert
+ * @name hashtable_add
* @param h the hashtable to insert into
* @param k the key - hashtable claims ownership and will free on removal
* @param v the value - does not claim ownership
- * @return non-zero for successful insertion
+ * @return zero for successful insertion
*
* This function will cause the table to expand if the insertion would take
* the ratio of entries to table size over the maximum load factor.
@@ -49,7 +49,7 @@ create_hashtable(const void *ctx, const char *name,
*/
int
-hashtable_insert(struct hashtable *h, void *k, void *v);
+hashtable_add(struct hashtable *h, void *k, void *v);
/*****************************************************************************
* hashtable_search
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 418790d8d7..c467a704a1 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2404,8 +2404,8 @@ int remember_string(struct hashtable *hash, const char *str)
char *k = talloc_strdup(NULL, str);
if (!k)
- return 0;
- return hashtable_insert(hash, k, (void *)1);
+ return ENOMEM;
+ return hashtable_add(hash, k, (void *)1);
}
/**
@@ -2438,7 +2438,7 @@ static int check_store_step(const void *ctx, struct connection *conn,
: WALK_TREE_SKIP_CHILDREN;
}
- if (!remember_string(data->reachable, node->name))
+ if (remember_string(data->reachable, node->name))
return WALK_TREE_ERROR_STOP;
domain_check_acc_add(node, data->domains);
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index a1c91ef3f3..815f15cd1d 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -538,7 +538,7 @@ static struct domain *alloc_domain(const void *context, unsigned int domid)
domain->generation = generation;
domain->introduced = false;
- if (!hashtable_insert(domhash, &domain->domid, domain)) {
+ if (hashtable_add(domhash, &domain->domid, domain)) {
talloc_free(domain);
errno = ENOMEM;
return NULL;
@@ -1795,7 +1795,7 @@ static int domain_check_acc_init_sub(const void *k, void *v, void *arg)
*/
dom->nodes = -d->acc[ACC_NODES].val;
- if (!hashtable_insert(domains, &dom->domid, dom)) {
+ if (hashtable_add(domains, &dom->domid, dom)) {
talloc_free(dom);
return -1;
}
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index db06d0e7f1..334f1609f1 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -601,13 +601,13 @@ int check_transactions(struct hashtable *hash)
list_for_each_entry(trans, &conn->transaction_list, list) {
tname = talloc_asprintf(trans, "%"PRIu64,
trans->generation);
- if (!tname || !remember_string(hash, tname))
+ if (!tname || remember_string(hash, tname))
goto nomem;
list_for_each_entry(i, &trans->accessed, list) {
if (!i->ta_node)
continue;
- if (!remember_string(hash, i->trans_name))
+ if (remember_string(hash, i->trans_name))
goto nomem;
}
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success
2023-05-30 8:54 ` [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success Juergen Gross
@ 2023-06-09 17:56 ` Julien Grall
0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-09 17:56 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> Today hashtable_insert() returns 0 in case of an error. Change that to
> let it return an errno value in the error case and 0 in case of success.
> In order to avoid any missed return value checks or related future
> backport errors, rename hashtable_insert() to hashtable_add().
>
> Even if not used today, do the same switch for the return value of
> hashtable_expand().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 05/16] tools/xenstore: make some write limit functions static
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (3 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 04/16] tools/xenstore: rename hashtable_insert() and let it return 0 on success Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 18:00 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 06/16] tools/xenstore: switch write limiting to use millisecond time base Juergen Gross
` (11 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Some wrl_*() functions are only used in xenstored_domain.c, so make
them static. In order to avoid the need of forward declarations, move
the whole function block to the start of the file.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/xenstored_domain.c | 456 +++++++++++++++---------------
tools/xenstore/xenstored_domain.h | 3 -
2 files changed, 228 insertions(+), 231 deletions(-)
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 815f15cd1d..b21bdf6194 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -157,6 +157,234 @@ struct changed_domain
static struct hashtable *domhash;
+static wrl_creditt wrl_config_writecost = WRL_FACTOR;
+static wrl_creditt wrl_config_rate = WRL_RATE * WRL_FACTOR;
+static wrl_creditt wrl_config_dburst = WRL_DBURST * WRL_FACTOR;
+static wrl_creditt wrl_config_gburst = WRL_GBURST * WRL_FACTOR;
+static wrl_creditt wrl_config_newdoms_dburst =
+ WRL_DBURST * WRL_NEWDOMS * WRL_FACTOR;
+
+long wrl_ntransactions;
+
+static long wrl_ndomains;
+static wrl_creditt wrl_reserve; /* [-wrl_config_newdoms_dburst, +_gburst ] */
+static time_t wrl_log_last_warning; /* 0: no previous warning */
+
+#define trace_wrl(...) \
+do { \
+ if (trace_flags & TRACE_WRL) \
+ trace("wrl: " __VA_ARGS__); \
+} while (0)
+
+void wrl_gettime_now(struct wrl_timestampt *now_wt)
+{
+ struct timespec now_ts;
+ int r;
+
+ r = clock_gettime(CLOCK_MONOTONIC, &now_ts);
+ if (r)
+ barf_perror("Could not find time (clock_gettime failed)");
+
+ now_wt->sec = now_ts.tv_sec;
+ now_wt->msec = now_ts.tv_nsec / 1000000;
+}
+
+static void wrl_xfer_credit(wrl_creditt *debit, wrl_creditt debit_floor,
+ wrl_creditt *credit, wrl_creditt credit_ceil)
+ /*
+ * Transfers zero or more credit from "debit" to "credit".
+ * Transfers as much as possible while maintaining
+ * debit >= debit_floor and credit <= credit_ceil.
+ * (If that's violated already, does nothing.)
+ *
+ * Sufficient conditions to avoid overflow, either of:
+ * |every argument| <= 0x3fffffff
+ * |every argument| <= 1E9
+ * |every argument| <= WRL_CREDIT_MAX
+ * (And this condition is preserved.)
+ */
+{
+ wrl_creditt xfer = MIN( *debit - debit_floor,
+ credit_ceil - *credit );
+ if (xfer > 0) {
+ *debit -= xfer;
+ *credit += xfer;
+ }
+}
+
+static void wrl_domain_new(struct domain *domain)
+{
+ domain->wrl_credit = 0;
+ wrl_gettime_now(&domain->wrl_timestamp);
+ wrl_ndomains++;
+ /* Steal up to DBURST from the reserve */
+ wrl_xfer_credit(&wrl_reserve, -wrl_config_newdoms_dburst,
+ &domain->wrl_credit, wrl_config_dburst);
+}
+
+static void wrl_domain_destroy(struct domain *domain)
+{
+ wrl_ndomains--;
+ /*
+ * Don't bother recalculating domain's credit - this just
+ * means we don't give the reserve the ending domain's credit
+ * for time elapsed since last update.
+ */
+ wrl_xfer_credit(&domain->wrl_credit, 0,
+ &wrl_reserve, wrl_config_dburst);
+}
+
+static void wrl_credit_update(struct domain *domain, struct wrl_timestampt now)
+{
+ /*
+ * We want to calculate
+ * credit += (now - timestamp) * RATE / ndoms;
+ * But we want it to saturate, and to avoid floating point.
+ * To avoid rounding errors from constantly adding small
+ * amounts of credit, we only add credit for whole milliseconds.
+ */
+ long seconds = now.sec - domain->wrl_timestamp.sec;
+ long milliseconds = now.msec - domain->wrl_timestamp.msec;
+ long msec;
+ int64_t denom, num;
+ wrl_creditt surplus;
+
+ seconds = MIN(seconds, 1000*1000); /* arbitrary, prevents overflow */
+ msec = seconds * 1000 + milliseconds;
+
+ if (msec < 0)
+ /* shouldn't happen with CLOCK_MONOTONIC */
+ msec = 0;
+
+ /* 32x32 -> 64 cannot overflow */
+ denom = (int64_t)msec * wrl_config_rate;
+ num = (int64_t)wrl_ndomains * 1000;
+ /* denom / num <= 1E6 * wrl_config_rate, so with
+ reasonable wrl_config_rate, denom / num << 2^64 */
+
+ /* at last! */
+ domain->wrl_credit = MIN( (int64_t)domain->wrl_credit + denom / num,
+ WRL_CREDIT_MAX );
+ /* (maybe briefly violating the DBURST cap on wrl_credit) */
+
+ /* maybe take from the reserve to make us nonnegative */
+ wrl_xfer_credit(&wrl_reserve, 0,
+ &domain->wrl_credit, 0);
+
+ /* return any surplus (over DBURST) to the reserve */
+ surplus = 0;
+ wrl_xfer_credit(&domain->wrl_credit, wrl_config_dburst,
+ &surplus, WRL_CREDIT_MAX);
+ wrl_xfer_credit(&surplus, 0,
+ &wrl_reserve, wrl_config_gburst);
+ /* surplus is now implicitly discarded */
+
+ domain->wrl_timestamp = now;
+
+ trace_wrl("dom %4d %6ld msec %9ld credit %9ld reserve %9ld discard\n",
+ domain->domid, msec, (long)domain->wrl_credit,
+ (long)wrl_reserve, (long)surplus);
+}
+
+void wrl_check_timeout(struct domain *domain,
+ struct wrl_timestampt now,
+ int *ptimeout)
+{
+ uint64_t num, denom;
+ int wakeup;
+
+ wrl_credit_update(domain, now);
+
+ if (domain->wrl_credit >= 0)
+ /* not blocked */
+ return;
+
+ if (!*ptimeout)
+ /* already decided on immediate wakeup,
+ so no need to calculate our timeout */
+ return;
+
+ /* calculate wakeup = now + -credit / (RATE / ndoms); */
+
+ /* credit cannot go more -ve than one transaction,
+ * so the first multiplication cannot overflow even 32-bit */
+ num = (uint64_t)(-domain->wrl_credit * 1000) * wrl_ndomains;
+ denom = wrl_config_rate;
+
+ wakeup = MIN( num / denom /* uint64_t */, INT_MAX );
+ if (*ptimeout==-1 || wakeup < *ptimeout)
+ *ptimeout = wakeup;
+
+ trace_wrl("domain %u credit=%ld (reserve=%ld) SLEEPING for %d\n",
+ domain->domid, (long)domain->wrl_credit, (long)wrl_reserve,
+ wakeup);
+}
+
+#define WRL_LOG(now, ...) \
+ (syslog(LOG_WARNING, "write rate limit: " __VA_ARGS__))
+
+void wrl_apply_debit_actual(struct domain *domain)
+{
+ struct wrl_timestampt now;
+
+ if (!domain || !domain_is_unprivileged(domain->conn))
+ /* sockets and privileged domain escape the write rate limit */
+ return;
+
+ wrl_gettime_now(&now);
+ wrl_credit_update(domain, now);
+
+ domain->wrl_credit -= wrl_config_writecost;
+ trace_wrl("domain %u credit=%ld (reserve=%ld)\n", domain->domid,
+ (long)domain->wrl_credit, (long)wrl_reserve);
+
+ if (domain->wrl_credit < 0) {
+ if (!domain->wrl_delay_logged) {
+ domain->wrl_delay_logged = true;
+ WRL_LOG(now, "domain %ld is affected\n",
+ (long)domain->domid);
+ } else if (!wrl_log_last_warning) {
+ WRL_LOG(now, "rate limiting restarts\n");
+ }
+ wrl_log_last_warning = now.sec;
+ }
+}
+
+void wrl_log_periodic(struct wrl_timestampt now)
+{
+ if (wrl_log_last_warning &&
+ (now.sec - wrl_log_last_warning) > WRL_LOGEVERY) {
+ WRL_LOG(now, "not in force recently\n");
+ wrl_log_last_warning = 0;
+ }
+}
+
+void wrl_apply_debit_direct(struct connection *conn)
+{
+ if (!conn)
+ /* some writes are generated internally */
+ return;
+
+ if (conn->transaction)
+ /* these are accounted for when the transaction ends */
+ return;
+
+ if (!wrl_ntransactions)
+ /* we don't conflict with anyone */
+ return;
+
+ wrl_apply_debit_actual(conn->domain);
+}
+
+void wrl_apply_debit_trans_commit(struct connection *conn)
+{
+ if (wrl_ntransactions <= 1)
+ /* our own transaction appears in the counter */
+ return;
+
+ wrl_apply_debit_actual(conn->domain);
+}
+
static bool check_indexes(XENSTORE_RING_IDX cons, XENSTORE_RING_IDX prod)
{
return ((prod - cons) <= XENSTORE_RING_SIZE);
@@ -1446,234 +1674,6 @@ unsigned int domain_transaction_get(struct connection *conn)
: 0;
}
-static wrl_creditt wrl_config_writecost = WRL_FACTOR;
-static wrl_creditt wrl_config_rate = WRL_RATE * WRL_FACTOR;
-static wrl_creditt wrl_config_dburst = WRL_DBURST * WRL_FACTOR;
-static wrl_creditt wrl_config_gburst = WRL_GBURST * WRL_FACTOR;
-static wrl_creditt wrl_config_newdoms_dburst =
- WRL_DBURST * WRL_NEWDOMS * WRL_FACTOR;
-
-long wrl_ntransactions;
-
-static long wrl_ndomains;
-static wrl_creditt wrl_reserve; /* [-wrl_config_newdoms_dburst, +_gburst ] */
-static time_t wrl_log_last_warning; /* 0: no previous warning */
-
-#define trace_wrl(...) \
-do { \
- if (trace_flags & TRACE_WRL) \
- trace("wrl: " __VA_ARGS__); \
-} while (0)
-
-void wrl_gettime_now(struct wrl_timestampt *now_wt)
-{
- struct timespec now_ts;
- int r;
-
- r = clock_gettime(CLOCK_MONOTONIC, &now_ts);
- if (r)
- barf_perror("Could not find time (clock_gettime failed)");
-
- now_wt->sec = now_ts.tv_sec;
- now_wt->msec = now_ts.tv_nsec / 1000000;
-}
-
-static void wrl_xfer_credit(wrl_creditt *debit, wrl_creditt debit_floor,
- wrl_creditt *credit, wrl_creditt credit_ceil)
- /*
- * Transfers zero or more credit from "debit" to "credit".
- * Transfers as much as possible while maintaining
- * debit >= debit_floor and credit <= credit_ceil.
- * (If that's violated already, does nothing.)
- *
- * Sufficient conditions to avoid overflow, either of:
- * |every argument| <= 0x3fffffff
- * |every argument| <= 1E9
- * |every argument| <= WRL_CREDIT_MAX
- * (And this condition is preserved.)
- */
-{
- wrl_creditt xfer = MIN( *debit - debit_floor,
- credit_ceil - *credit );
- if (xfer > 0) {
- *debit -= xfer;
- *credit += xfer;
- }
-}
-
-void wrl_domain_new(struct domain *domain)
-{
- domain->wrl_credit = 0;
- wrl_gettime_now(&domain->wrl_timestamp);
- wrl_ndomains++;
- /* Steal up to DBURST from the reserve */
- wrl_xfer_credit(&wrl_reserve, -wrl_config_newdoms_dburst,
- &domain->wrl_credit, wrl_config_dburst);
-}
-
-void wrl_domain_destroy(struct domain *domain)
-{
- wrl_ndomains--;
- /*
- * Don't bother recalculating domain's credit - this just
- * means we don't give the reserve the ending domain's credit
- * for time elapsed since last update.
- */
- wrl_xfer_credit(&domain->wrl_credit, 0,
- &wrl_reserve, wrl_config_dburst);
-}
-
-void wrl_credit_update(struct domain *domain, struct wrl_timestampt now)
-{
- /*
- * We want to calculate
- * credit += (now - timestamp) * RATE / ndoms;
- * But we want it to saturate, and to avoid floating point.
- * To avoid rounding errors from constantly adding small
- * amounts of credit, we only add credit for whole milliseconds.
- */
- long seconds = now.sec - domain->wrl_timestamp.sec;
- long milliseconds = now.msec - domain->wrl_timestamp.msec;
- long msec;
- int64_t denom, num;
- wrl_creditt surplus;
-
- seconds = MIN(seconds, 1000*1000); /* arbitrary, prevents overflow */
- msec = seconds * 1000 + milliseconds;
-
- if (msec < 0)
- /* shouldn't happen with CLOCK_MONOTONIC */
- msec = 0;
-
- /* 32x32 -> 64 cannot overflow */
- denom = (int64_t)msec * wrl_config_rate;
- num = (int64_t)wrl_ndomains * 1000;
- /* denom / num <= 1E6 * wrl_config_rate, so with
- reasonable wrl_config_rate, denom / num << 2^64 */
-
- /* at last! */
- domain->wrl_credit = MIN( (int64_t)domain->wrl_credit + denom / num,
- WRL_CREDIT_MAX );
- /* (maybe briefly violating the DBURST cap on wrl_credit) */
-
- /* maybe take from the reserve to make us nonnegative */
- wrl_xfer_credit(&wrl_reserve, 0,
- &domain->wrl_credit, 0);
-
- /* return any surplus (over DBURST) to the reserve */
- surplus = 0;
- wrl_xfer_credit(&domain->wrl_credit, wrl_config_dburst,
- &surplus, WRL_CREDIT_MAX);
- wrl_xfer_credit(&surplus, 0,
- &wrl_reserve, wrl_config_gburst);
- /* surplus is now implicitly discarded */
-
- domain->wrl_timestamp = now;
-
- trace_wrl("dom %4d %6ld msec %9ld credit %9ld reserve %9ld discard\n",
- domain->domid, msec, (long)domain->wrl_credit,
- (long)wrl_reserve, (long)surplus);
-}
-
-void wrl_check_timeout(struct domain *domain,
- struct wrl_timestampt now,
- int *ptimeout)
-{
- uint64_t num, denom;
- int wakeup;
-
- wrl_credit_update(domain, now);
-
- if (domain->wrl_credit >= 0)
- /* not blocked */
- return;
-
- if (!*ptimeout)
- /* already decided on immediate wakeup,
- so no need to calculate our timeout */
- return;
-
- /* calculate wakeup = now + -credit / (RATE / ndoms); */
-
- /* credit cannot go more -ve than one transaction,
- * so the first multiplication cannot overflow even 32-bit */
- num = (uint64_t)(-domain->wrl_credit * 1000) * wrl_ndomains;
- denom = wrl_config_rate;
-
- wakeup = MIN( num / denom /* uint64_t */, INT_MAX );
- if (*ptimeout==-1 || wakeup < *ptimeout)
- *ptimeout = wakeup;
-
- trace_wrl("domain %u credit=%ld (reserve=%ld) SLEEPING for %d\n",
- domain->domid, (long)domain->wrl_credit, (long)wrl_reserve,
- wakeup);
-}
-
-#define WRL_LOG(now, ...) \
- (syslog(LOG_WARNING, "write rate limit: " __VA_ARGS__))
-
-void wrl_apply_debit_actual(struct domain *domain)
-{
- struct wrl_timestampt now;
-
- if (!domain || !domid_is_unprivileged(domain->domid))
- /* sockets and privileged domain escape the write rate limit */
- return;
-
- wrl_gettime_now(&now);
- wrl_credit_update(domain, now);
-
- domain->wrl_credit -= wrl_config_writecost;
- trace_wrl("domain %u credit=%ld (reserve=%ld)\n", domain->domid,
- (long)domain->wrl_credit, (long)wrl_reserve);
-
- if (domain->wrl_credit < 0) {
- if (!domain->wrl_delay_logged) {
- domain->wrl_delay_logged = true;
- WRL_LOG(now, "domain %ld is affected\n",
- (long)domain->domid);
- } else if (!wrl_log_last_warning) {
- WRL_LOG(now, "rate limiting restarts\n");
- }
- wrl_log_last_warning = now.sec;
- }
-}
-
-void wrl_log_periodic(struct wrl_timestampt now)
-{
- if (wrl_log_last_warning &&
- (now.sec - wrl_log_last_warning) > WRL_LOGEVERY) {
- WRL_LOG(now, "not in force recently\n");
- wrl_log_last_warning = 0;
- }
-}
-
-void wrl_apply_debit_direct(struct connection *conn)
-{
- if (!conn)
- /* some writes are generated internally */
- return;
-
- if (conn->transaction)
- /* these are accounted for when the transaction ends */
- return;
-
- if (!wrl_ntransactions)
- /* we don't conflict with anyone */
- return;
-
- wrl_apply_debit_actual(conn->domain);
-}
-
-void wrl_apply_debit_trans_commit(struct connection *conn)
-{
- if (wrl_ntransactions <= 1)
- /* our own transaction appears in the counter */
- return;
-
- wrl_apply_debit_actual(conn->domain);
-}
-
const char *dump_state_connections(FILE *fp)
{
const char *ret = NULL;
diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h
index 5123453f6c..89be643de4 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -163,9 +163,6 @@ struct wrl_timestampt {
extern long wrl_ntransactions;
void wrl_gettime_now(struct wrl_timestampt *now_ts);
-void wrl_domain_new(struct domain *domain);
-void wrl_domain_destroy(struct domain *domain);
-void wrl_credit_update(struct domain *domain, struct wrl_timestampt now);
void wrl_check_timeout(struct domain *domain,
struct wrl_timestampt now,
int *ptimeout);
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 06/16] tools/xenstore: switch write limiting to use millisecond time base
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (4 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 05/16] tools/xenstore: make some write limit functions static Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 18:02 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 07/16] tools/xenstore: remove stale TODO file Juergen Gross
` (10 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
There is no need to keep struct wrl_timestampt, as it serves the same
purpose as the more simple time base provided by get_now().
Move some more stuff from xenstored_domain.h into xenstored_domain.c
as it is being used nowhere else.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/xenstored_core.c | 8 ++---
tools/xenstore/xenstored_core.h | 7 ++--
tools/xenstore/xenstored_domain.c | 56 +++++++++++++------------------
tools/xenstore/xenstored_domain.h | 21 ++----------
4 files changed, 32 insertions(+), 60 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c467a704a1..e8f46495de 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -192,7 +192,7 @@ void reopen_log(void)
}
}
-static uint64_t get_now_msec(void)
+uint64_t get_now_msec(void)
{
struct timespec now_ts;
@@ -510,7 +510,6 @@ fail:
static void initialize_fds(int *p_sock_pollfd_idx, int *ptimeout)
{
struct connection *conn;
- struct wrl_timestampt now;
uint64_t msecs;
if (fds)
@@ -530,13 +529,12 @@ static void initialize_fds(int *p_sock_pollfd_idx, int *ptimeout)
xce_pollfd_idx = set_fd(xenevtchn_fd(xce_handle),
POLLIN|POLLPRI);
- wrl_gettime_now(&now);
- wrl_log_periodic(now);
msecs = get_now_msec();
+ wrl_log_periodic(msecs);
list_for_each_entry(conn, &connections, list) {
if (conn->domain) {
- wrl_check_timeout(conn->domain, now, ptimeout);
+ wrl_check_timeout(conn->domain, msecs, ptimeout);
check_event_timeout(conn, msecs, ptimeout);
if (conn_can_read(conn) ||
(conn_can_write(conn) &&
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 92d5b50f3c..84a611cbb5 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -47,10 +47,6 @@
/* DEFAULT_BUFFER_SIZE should be large enough for each errno string. */
#define DEFAULT_BUFFER_SIZE 16
-typedef int32_t wrl_creditt;
-#define WRL_CREDIT_MAX (1000*1000*1000)
-/* ^ satisfies non-overflow condition for wrl_xfer_credit */
-
struct xs_state_connection;
struct buffered_data
@@ -320,6 +316,9 @@ extern bool keep_orphans;
extern unsigned int timeout_watch_event_msec;
+/* Get internal time in milliseconds. */
+uint64_t get_now_msec(void);
+
/* Map the kernel's xenstore page. */
void *xenbus_map(void);
void unmap_xenbus(void *interface);
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index b21bdf6194..60d3aa1ddb 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -99,6 +99,8 @@ struct quota soft_quotas[ACC_N] = {
},
};
+typedef int32_t wrl_creditt;
+
struct domain
{
/* The id of this domain */
@@ -139,7 +141,7 @@ struct domain
/* write rate limit */
wrl_creditt wrl_credit; /* [ -wrl_config_writecost, +_dburst ] */
- struct wrl_timestampt wrl_timestamp;
+ uint64_t wrl_timestamp;
bool wrl_delay_logged;
};
@@ -157,6 +159,17 @@ struct changed_domain
static struct hashtable *domhash;
+/* Write rate limiting */
+
+/* Satisfies non-overflow condition for wrl_xfer_credit. */
+#define WRL_CREDIT_MAX (1000*1000*1000)
+#define WRL_FACTOR 1000 /* for fixed-point arithmetic */
+#define WRL_RATE 200
+#define WRL_DBURST 10
+#define WRL_GBURST 1000
+#define WRL_NEWDOMS 5
+#define WRL_LOGEVERY 120 /* seconds */
+
static wrl_creditt wrl_config_writecost = WRL_FACTOR;
static wrl_creditt wrl_config_rate = WRL_RATE * WRL_FACTOR;
static wrl_creditt wrl_config_dburst = WRL_DBURST * WRL_FACTOR;
@@ -176,19 +189,6 @@ do { \
trace("wrl: " __VA_ARGS__); \
} while (0)
-void wrl_gettime_now(struct wrl_timestampt *now_wt)
-{
- struct timespec now_ts;
- int r;
-
- r = clock_gettime(CLOCK_MONOTONIC, &now_ts);
- if (r)
- barf_perror("Could not find time (clock_gettime failed)");
-
- now_wt->sec = now_ts.tv_sec;
- now_wt->msec = now_ts.tv_nsec / 1000000;
-}
-
static void wrl_xfer_credit(wrl_creditt *debit, wrl_creditt debit_floor,
wrl_creditt *credit, wrl_creditt credit_ceil)
/*
@@ -215,7 +215,7 @@ static void wrl_xfer_credit(wrl_creditt *debit, wrl_creditt debit_floor,
static void wrl_domain_new(struct domain *domain)
{
domain->wrl_credit = 0;
- wrl_gettime_now(&domain->wrl_timestamp);
+ domain->wrl_timestamp = get_now_msec();
wrl_ndomains++;
/* Steal up to DBURST from the reserve */
wrl_xfer_credit(&wrl_reserve, -wrl_config_newdoms_dburst,
@@ -234,7 +234,7 @@ static void wrl_domain_destroy(struct domain *domain)
&wrl_reserve, wrl_config_dburst);
}
-static void wrl_credit_update(struct domain *domain, struct wrl_timestampt now)
+static void wrl_credit_update(struct domain *domain, uint64_t now)
{
/*
* We want to calculate
@@ -243,18 +243,12 @@ static void wrl_credit_update(struct domain *domain, struct wrl_timestampt now)
* To avoid rounding errors from constantly adding small
* amounts of credit, we only add credit for whole milliseconds.
*/
- long seconds = now.sec - domain->wrl_timestamp.sec;
- long milliseconds = now.msec - domain->wrl_timestamp.msec;
long msec;
int64_t denom, num;
wrl_creditt surplus;
- seconds = MIN(seconds, 1000*1000); /* arbitrary, prevents overflow */
- msec = seconds * 1000 + milliseconds;
-
- if (msec < 0)
- /* shouldn't happen with CLOCK_MONOTONIC */
- msec = 0;
+ /* Prevent overflow by limiting to 32 bits. */
+ msec = MIN(now - domain->wrl_timestamp, 1000 * 1000 * 1000);
/* 32x32 -> 64 cannot overflow */
denom = (int64_t)msec * wrl_config_rate;
@@ -286,9 +280,7 @@ static void wrl_credit_update(struct domain *domain, struct wrl_timestampt now)
(long)wrl_reserve, (long)surplus);
}
-void wrl_check_timeout(struct domain *domain,
- struct wrl_timestampt now,
- int *ptimeout)
+void wrl_check_timeout(struct domain *domain, uint64_t now, int *ptimeout)
{
uint64_t num, denom;
int wakeup;
@@ -325,13 +317,13 @@ void wrl_check_timeout(struct domain *domain,
void wrl_apply_debit_actual(struct domain *domain)
{
- struct wrl_timestampt now;
+ uint64_t now;
if (!domain || !domain_is_unprivileged(domain->conn))
/* sockets and privileged domain escape the write rate limit */
return;
- wrl_gettime_now(&now);
+ now = get_now_msec();
wrl_credit_update(domain, now);
domain->wrl_credit -= wrl_config_writecost;
@@ -346,14 +338,14 @@ void wrl_apply_debit_actual(struct domain *domain)
} else if (!wrl_log_last_warning) {
WRL_LOG(now, "rate limiting restarts\n");
}
- wrl_log_last_warning = now.sec;
+ wrl_log_last_warning = now / 1000;
}
}
-void wrl_log_periodic(struct wrl_timestampt now)
+void wrl_log_periodic(uint64_t now)
{
if (wrl_log_last_warning &&
- (now.sec - wrl_log_last_warning) > WRL_LOGEVERY) {
+ (now / 1000 - wrl_log_last_warning) > WRL_LOGEVERY) {
WRL_LOG(now, "not in force recently\n");
wrl_log_last_warning = 0;
}
diff --git a/tools/xenstore/xenstored_domain.h b/tools/xenstore/xenstored_domain.h
index 89be643de4..bf63f3fcc6 100644
--- a/tools/xenstore/xenstored_domain.h
+++ b/tools/xenstore/xenstored_domain.h
@@ -146,27 +146,10 @@ void domain_reset_global_acc(void);
bool domain_max_chk(const struct connection *conn, unsigned int what,
unsigned int val);
-/* Write rate limiting */
-
-#define WRL_FACTOR 1000 /* for fixed-point arithmetic */
-#define WRL_RATE 200
-#define WRL_DBURST 10
-#define WRL_GBURST 1000
-#define WRL_NEWDOMS 5
-#define WRL_LOGEVERY 120 /* seconds */
-
-struct wrl_timestampt {
- time_t sec;
- int msec;
-};
-
extern long wrl_ntransactions;
-void wrl_gettime_now(struct wrl_timestampt *now_ts);
-void wrl_check_timeout(struct domain *domain,
- struct wrl_timestampt now,
- int *ptimeout);
-void wrl_log_periodic(struct wrl_timestampt now);
+void wrl_check_timeout(struct domain *domain, uint64_t now, int *ptimeout);
+void wrl_log_periodic(uint64_t now);
void wrl_apply_debit_direct(struct connection *conn);
void wrl_apply_debit_trans_commit(struct connection *conn);
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 07/16] tools/xenstore: remove stale TODO file
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (5 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 06/16] tools/xenstore: switch write limiting to use millisecond time base Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-05-30 8:54 ` [PATCH v3 08/16] tools/xenstore: remove unused events list Juergen Gross
` (9 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD, Julien Grall
The TODO file is not really helpful any longer. It contains only
entries which no longer apply or it is unknown what they are meant
for ("Dynamic/supply nodes", "Remove assumption that rename doesn't
fail").
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
tools/xenstore/TODO | 10 ----------
1 file changed, 10 deletions(-)
delete mode 100644 tools/xenstore/TODO
diff --git a/tools/xenstore/TODO b/tools/xenstore/TODO
deleted file mode 100644
index 71d5bbbf50..0000000000
--- a/tools/xenstore/TODO
+++ /dev/null
@@ -1,10 +0,0 @@
-TODO in no particular order. Some of these will never be done. There
-are omissions of important but necessary things. It is up to the
-reader to fill in the blanks.
-
-- Timeout failed watch responses
-- Dynamic/supply nodes
-- Persistant storage of introductions, watches and transactions, so daemon can restart
-- Remove assumption that rename doesn't fail
-- Multi-root transactions, for setting up front and back ends at same time.
-
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 08/16] tools/xenstore: remove unused events list
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (6 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 07/16] tools/xenstore: remove stale TODO file Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-05-30 8:54 ` [PATCH v3 09/16] tools/xenstore: remove support of file backed data base Juergen Gross
` (8 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD, Julien Grall
struct watch contains an unused struct list_head events. Remove it.
Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
---
tools/xenstore/xenstored_watch.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c
index e8eb35de02..4195c59e17 100644
--- a/tools/xenstore/xenstored_watch.c
+++ b/tools/xenstore/xenstored_watch.c
@@ -36,9 +36,6 @@ struct watch
/* Watches on this connection */
struct list_head list;
- /* Current outstanding events applying to this watch. */
- struct list_head events;
-
/* Offset into path for skipping prefix (used for relative paths). */
unsigned int prefix_len;
@@ -205,8 +202,6 @@ static struct watch *add_watch(struct connection *conn, char *path, char *token,
watch->prefix_len = relative ? strlen(get_implicit_path(conn)) + 1 : 0;
- INIT_LIST_HEAD(&watch->events);
-
domain_watch_inc(conn);
list_add_tail(&watch->list, &conn->watches);
talloc_set_destructor(watch, destroy_watch);
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 09/16] tools/xenstore: remove support of file backed data base
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (7 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 08/16] tools/xenstore: remove unused events list Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 18:04 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h Juergen Gross
` (7 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD
In order to prepare the replacement of TDB with direct accessible nodes
in memory, remove the support for a file backed data base.
This allows to remove xs_tdb_dump, too.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
.gitignore | 1 -
tools/xenstore/Makefile | 5 +-
tools/xenstore/xenstored_core.c | 18 ++-----
tools/xenstore/xs_tdb_dump.c | 86 ---------------------------------
4 files changed, 4 insertions(+), 106 deletions(-)
delete mode 100644 tools/xenstore/xs_tdb_dump.c
diff --git a/.gitignore b/.gitignore
index beac034784..26f532e77c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -248,7 +248,6 @@ tools/xenstore/xenstore-rm
tools/xenstore/xenstore-watch
tools/xenstore/xenstore-write
tools/xenstore/xenstored
-tools/xenstore/xs_tdb_dump
tools/xentop/xentop
tools/xentrace/xentrace_setsize
tools/xentrace/tbctl
diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index ce7a68178f..56723139a1 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -29,7 +29,7 @@ CLIENTS += xenstore-write xenstore-ls xenstore-watch
TARGETS := xenstore $(CLIENTS) xenstore-control
ifeq ($(XENSTORE_XENSTORED),y)
-TARGETS += xs_tdb_dump xenstored
+TARGETS += xenstored
endif
.PHONY: all
@@ -50,9 +50,6 @@ xenstore: xenstore_client.o xs_lib.o
xenstore-control: xenstore_control.o
$(CC) $(LDFLAGS) $^ $(LDLIBS) -o $@ $(APPEND_LDFLAGS)
-xs_tdb_dump: xs_tdb_dump.o utils.o tdb.o talloc.o
- $(CC) $(LDFLAGS) $^ -o $@ $(APPEND_LDFLAGS)
-
.PHONY: clean
clean::
$(RM) $(TARGETS) $(DEPS_RM)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index e8f46495de..07fbac55ac 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2301,8 +2301,6 @@ static void accept_connection(int sock)
}
#endif
-static int tdb_flags = TDB_INTERNAL | TDB_NOLOCK;
-
/* We create initial nodes manually. */
static void manual_node(const char *name, const char *child)
{
@@ -2354,14 +2352,11 @@ void setup_structure(bool live_update)
{
char *tdbname;
- tdbname = talloc_strdup(talloc_autofree_context(), xs_daemon_tdb());
+ tdbname = talloc_strdup(talloc_autofree_context(), "/dev/mem");
if (!tdbname)
barf_perror("Could not create tdbname");
- if (!(tdb_flags & TDB_INTERNAL))
- unlink(tdbname);
-
- tdb_ctx = tdb_open_ex(tdbname, 7919, tdb_flags,
+ tdb_ctx = tdb_open_ex(tdbname, 7919, TDB_INTERNAL | TDB_NOLOCK,
O_RDWR | O_CREAT | O_EXCL | O_CLOEXEC,
0640, &tdb_logger, NULL);
if (!tdb_ctx)
@@ -2659,8 +2654,6 @@ static void usage(void)
" watch-event: time a watch-event is kept pending\n"
" -R, --no-recovery to request that no recovery should be attempted when\n"
" the store is corrupted (debug only),\n"
-" -I, --internal-db [on|off] store database in memory, not on disk, default is\n"
-" memory, with \"--internal-db off\" it is on disk\n"
" -K, --keep-orphans don't delete nodes owned by a domain when the\n"
" domain is deleted (this is a security risk!)\n"
" -V, --verbose to request verbose execution.\n");
@@ -2687,7 +2680,6 @@ static struct option options[] = {
{ "quota-soft", 1, NULL, 'q' },
{ "timeout", 1, NULL, 'w' },
{ "no-recovery", 0, NULL, 'R' },
- { "internal-db", 2, NULL, 'I' },
{ "keep-orphans", 0, NULL, 'K' },
{ "verbose", 0, NULL, 'V' },
{ "watch-nb", 1, NULL, 'W' },
@@ -2811,7 +2803,7 @@ int main(int argc, char *argv[])
orig_argv = argv;
while ((opt = getopt_long(argc, argv,
- "DE:F:HI::KNPS:t:A:M:Q:q:T:RVW:w:U",
+ "DE:F:H::KNPS:t:A:M:Q:q:T:RVW:w:U",
options, NULL)) != -1) {
switch (opt) {
case 'D':
@@ -2848,10 +2840,6 @@ int main(int argc, char *argv[])
if (set_trace_switch(optarg))
barf("Illegal trace switch \"%s\"\n", optarg);
break;
- case 'I':
- if (optarg && !strcmp(optarg, "off"))
- tdb_flags = 0;
- break;
case 'K':
keep_orphans = true;
break;
diff --git a/tools/xenstore/xs_tdb_dump.c b/tools/xenstore/xs_tdb_dump.c
deleted file mode 100644
index 5d2db392b4..0000000000
--- a/tools/xenstore/xs_tdb_dump.c
+++ /dev/null
@@ -1,86 +0,0 @@
-/* Simple program to dump out all records of TDB */
-#include <stdint.h>
-#include <stdlib.h>
-#include <fcntl.h>
-#include <stdio.h>
-#include <stdarg.h>
-#include <string.h>
-#include <sys/types.h>
-#include "xenstore_lib.h"
-#include "tdb.h"
-#include "talloc.h"
-#include "utils.h"
-
-static uint32_t total_size(struct xs_tdb_record_hdr *hdr)
-{
- return sizeof(*hdr) + hdr->num_perms * sizeof(struct xs_permissions)
- + hdr->datalen + hdr->childlen;
-}
-
-static char perm_to_char(unsigned int perm)
-{
- return perm == XS_PERM_READ ? 'r' :
- perm == XS_PERM_WRITE ? 'w' :
- perm == XS_PERM_NONE ? '-' :
- perm == (XS_PERM_READ|XS_PERM_WRITE) ? 'b' :
- '?';
-}
-
-static void tdb_logger(TDB_CONTEXT *tdb, int level, const char * fmt, ...)
-{
- va_list ap;
-
- va_start(ap, fmt);
- vfprintf(stderr, fmt, ap);
- va_end(ap);
-}
-
-int main(int argc, char *argv[])
-{
- TDB_DATA key;
- TDB_CONTEXT *tdb;
-
- if (argc != 2)
- barf("Usage: xs_tdb_dump <tdbfile>");
-
- tdb = tdb_open_ex(talloc_strdup(NULL, argv[1]), 0, 0, O_RDONLY, 0,
- &tdb_logger, NULL);
- if (!tdb)
- barf_perror("Could not open %s", argv[1]);
-
- key = tdb_firstkey(tdb);
- while (key.dptr) {
- TDB_DATA data;
- struct xs_tdb_record_hdr *hdr;
-
- data = tdb_fetch(tdb, key);
- hdr = (void *)data.dptr;
- if (data.dsize < sizeof(*hdr))
- fprintf(stderr, "%.*s: BAD truncated\n",
- (int)key.dsize, key.dptr);
- else if (data.dsize != total_size(hdr))
- fprintf(stderr, "%.*s: BAD length %zu for %u/%u/%u (%u)\n",
- (int)key.dsize, key.dptr, data.dsize,
- hdr->num_perms, hdr->datalen,
- hdr->childlen, total_size(hdr));
- else {
- unsigned int i;
- char *p;
-
- printf("%.*s: ", (int)key.dsize, key.dptr);
- for (i = 0; i < hdr->num_perms; i++)
- printf("%s%c%u",
- i == 0 ? "" : ",",
- perm_to_char(hdr->perms[i].perms),
- hdr->perms[i].id);
- p = (void *)&hdr->perms[hdr->num_perms];
- printf(" %.*s\n", hdr->datalen, p);
- p += hdr->datalen;
- for (i = 0; i < hdr->childlen; i += strlen(p+i)+1)
- printf("\t-> %s\n", p+i);
- }
- key = tdb_nextkey(tdb, key);
- }
- return 0;
-}
-
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 09/16] tools/xenstore: remove support of file backed data base
2023-05-30 8:54 ` [PATCH v3 09/16] tools/xenstore: remove support of file backed data base Juergen Gross
@ 2023-06-09 18:04 ` Julien Grall
0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-09 18:04 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Andrew Cooper, George Dunlap, Jan Beulich, Stefano Stabellini,
Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> In order to prepare the replacement of TDB with direct accessible nodes
> in memory, remove the support for a file backed data base.
>
> This allows to remove xs_tdb_dump, too.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (8 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 09/16] tools/xenstore: remove support of file backed data base Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 18:09 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 11/16] tools/libs/store: make libxenstore independent of utils.h Juergen Gross
` (6 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Replace the usage of the xenstore private list.h header with the
common xen_list.h one.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
---
tools/libs/store/xs.c | 56 +++++++++++++++++++++----------------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 7a9a8b1656..3813b69ae2 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -35,13 +35,13 @@
#include <errno.h>
#include "xenstore.h"
#include "xs_lib.h"
-#include "list.h"
#include "utils.h"
#include <xentoolcore_internal.h>
+#include <xen_list.h>
struct xs_stored_msg {
- struct list_head list;
+ XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
struct xsd_sockmsg hdr;
char *body;
};
@@ -70,7 +70,7 @@ struct xs_handle {
* A list of fired watch messages, protected by a mutex. Users can
* wait on the conditional variable until a watch is pending.
*/
- struct list_head watch_list;
+ XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
pthread_mutex_t watch_mutex;
pthread_cond_t watch_condvar;
@@ -84,7 +84,7 @@ struct xs_handle {
* because we serialise requests. The requester can wait on the
* conditional variable for its response.
*/
- struct list_head reply_list;
+ XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
pthread_mutex_t reply_mutex;
pthread_cond_t reply_condvar;
@@ -133,8 +133,8 @@ static void *read_thread(void *arg);
struct xs_handle {
int fd;
Xentoolcore__Active_Handle tc_ah; /* for restrict */
- struct list_head reply_list;
- struct list_head watch_list;
+ XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
+ XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
/* Clients can select() on this pipe to wait for a watch to fire. */
int watch_pipe[2];
/* Filtering watch event in unwatch function? */
@@ -180,7 +180,7 @@ int xs_fileno(struct xs_handle *h)
if ((h->watch_pipe[0] == -1) && (pipe(h->watch_pipe) != -1)) {
/* Kick things off if the watch list is already non-empty. */
- if (!list_empty(&h->watch_list))
+ if (!XEN_TAILQ_EMPTY(&h->watch_list))
while (write(h->watch_pipe[1], &c, 1) != 1)
continue;
}
@@ -262,8 +262,8 @@ static struct xs_handle *get_handle(const char *connect_to)
if (h->fd == -1)
goto err;
- INIT_LIST_HEAD(&h->reply_list);
- INIT_LIST_HEAD(&h->watch_list);
+ XEN_TAILQ_INIT(&h->reply_list);
+ XEN_TAILQ_INIT(&h->watch_list);
/* Watch pipe is allocated on demand in xs_fileno(). */
h->watch_pipe[0] = h->watch_pipe[1] = -1;
@@ -329,12 +329,12 @@ struct xs_handle *xs_open(unsigned long flags)
static void close_free_msgs(struct xs_handle *h) {
struct xs_stored_msg *msg, *tmsg;
- list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
+ XEN_TAILQ_FOREACH_SAFE(msg, &h->reply_list, list, tmsg) {
free(msg->body);
free(msg);
}
- list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
+ XEN_TAILQ_FOREACH_SAFE(msg, &h->watch_list, list, tmsg) {
free(msg->body);
free(msg);
}
@@ -459,17 +459,17 @@ static void *read_reply(
mutex_lock(&h->reply_mutex);
#ifdef USE_PTHREAD
- while (list_empty(&h->reply_list) && read_from_thread && h->fd != -1)
+ while (XEN_TAILQ_EMPTY(&h->reply_list) && read_from_thread && h->fd != -1)
condvar_wait(&h->reply_condvar, &h->reply_mutex);
#endif
- if (list_empty(&h->reply_list)) {
+ if (XEN_TAILQ_EMPTY(&h->reply_list)) {
mutex_unlock(&h->reply_mutex);
errno = EINVAL;
return NULL;
}
- msg = list_top(&h->reply_list, struct xs_stored_msg, list);
- list_del(&msg->list);
- assert(list_empty(&h->reply_list));
+ msg = XEN_TAILQ_FIRST(&h->reply_list);
+ XEN_TAILQ_REMOVE(&h->reply_list, msg, list);
+ assert(XEN_TAILQ_EMPTY(&h->reply_list));
mutex_unlock(&h->reply_mutex);
*type = msg->hdr.type;
@@ -883,7 +883,7 @@ static void xs_maybe_clear_watch_pipe(struct xs_handle *h)
{
char c;
- if (list_empty(&h->watch_list) && (h->watch_pipe[0] != -1))
+ if (XEN_TAILQ_EMPTY(&h->watch_list) && (h->watch_pipe[0] != -1))
while (read(h->watch_pipe[0], &c, 1) != 1)
continue;
}
@@ -907,7 +907,7 @@ static char **read_watch_internal(struct xs_handle *h, unsigned int *num,
* we haven't called xs_watch. Presumably the application
* will do so later; in the meantime we just block.
*/
- while (list_empty(&h->watch_list) && h->fd != -1) {
+ while (XEN_TAILQ_EMPTY(&h->watch_list) && h->fd != -1) {
if (nonblocking) {
mutex_unlock(&h->watch_mutex);
errno = EAGAIN;
@@ -925,13 +925,13 @@ static char **read_watch_internal(struct xs_handle *h, unsigned int *num,
#endif /* !defined(USE_PTHREAD) */
- if (list_empty(&h->watch_list)) {
+ if (XEN_TAILQ_EMPTY(&h->watch_list)) {
mutex_unlock(&h->watch_mutex);
errno = EINVAL;
return NULL;
}
- msg = list_top(&h->watch_list, struct xs_stored_msg, list);
- list_del(&msg->list);
+ msg = XEN_TAILQ_FIRST(&h->watch_list);
+ XEN_TAILQ_REMOVE(&h->watch_list, msg, list);
xs_maybe_clear_watch_pipe(h);
mutex_unlock(&h->watch_mutex);
@@ -1007,12 +1007,12 @@ bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
/* Filter the watch list to remove potential message */
mutex_lock(&h->watch_mutex);
- if (list_empty(&h->watch_list)) {
+ if (XEN_TAILQ_EMPTY(&h->watch_list)) {
mutex_unlock(&h->watch_mutex);
return res;
}
- list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
+ XEN_TAILQ_FOREACH_SAFE(msg, &h->watch_list, list, tmsg) {
assert(msg->hdr.type == XS_WATCH_EVENT);
s = msg->body;
@@ -1034,7 +1034,7 @@ bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
if (l_token && !strcmp(token, l_token) &&
l_path && xs_path_is_subpath(path, l_path)) {
- list_del(&msg->list);
+ XEN_TAILQ_REMOVE(&h->watch_list, msg, list);
free(msg);
}
}
@@ -1290,12 +1290,12 @@ static int read_message(struct xs_handle *h, int nonblocking)
cleanup_push(pthread_mutex_unlock, &h->watch_mutex);
/* Kick users out of their select() loop. */
- if (list_empty(&h->watch_list) &&
+ if (XEN_TAILQ_EMPTY(&h->watch_list) &&
(h->watch_pipe[1] != -1))
while (write(h->watch_pipe[1], body, 1) != 1) /* Cancellation point */
continue;
- list_add_tail(&msg->list, &h->watch_list);
+ XEN_TAILQ_INSERT_TAIL(&h->watch_list, msg, list);
condvar_signal(&h->watch_condvar);
@@ -1304,13 +1304,13 @@ static int read_message(struct xs_handle *h, int nonblocking)
mutex_lock(&h->reply_mutex);
/* There should only ever be one response pending! */
- if (!list_empty(&h->reply_list)) {
+ if (!XEN_TAILQ_EMPTY(&h->reply_list)) {
mutex_unlock(&h->reply_mutex);
saved_errno = EEXIST;
goto error_freebody;
}
- list_add_tail(&msg->list, &h->reply_list);
+ XEN_TAILQ_INSERT_TAIL(&h->reply_list, msg, list);
condvar_signal(&h->reply_condvar);
mutex_unlock(&h->reply_mutex);
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-05-30 8:54 ` [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h Juergen Gross
@ 2023-06-09 18:09 ` Julien Grall
2023-06-12 7:02 ` Juergen Gross
0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2023-06-09 18:09 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> Replace the usage of the xenstore private list.h header with the
> common xen_list.h one.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
> ---
> V3:
> - new patch
> ---
> tools/libs/store/xs.c | 56 +++++++++++++++++++++----------------------
> 1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
> index 7a9a8b1656..3813b69ae2 100644
> --- a/tools/libs/store/xs.c
> +++ b/tools/libs/store/xs.c
> @@ -35,13 +35,13 @@
> #include <errno.h>
> #include "xenstore.h"
> #include "xs_lib.h"
> -#include "list.h"
> #include "utils.h"
>
> #include <xentoolcore_internal.h>
> +#include <xen_list.h>
>
> struct xs_stored_msg {
> - struct list_head list;
> + XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
I have expected us to use to XEN_LIST_*. Can you explain why you didn't
use them?
> struct xsd_sockmsg hdr;
> char *body;
> };
> @@ -70,7 +70,7 @@ struct xs_handle {
> * A list of fired watch messages, protected by a mutex. Users can
> * wait on the conditional variable until a watch is pending.
> */
> - struct list_head watch_list;
> + XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
> pthread_mutex_t watch_mutex;
> pthread_cond_t watch_condvar;
>
> @@ -84,7 +84,7 @@ struct xs_handle {
> * because we serialise requests. The requester can wait on the
> * conditional variable for its response.
> */
> - struct list_head reply_list;
> + XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
> pthread_mutex_t reply_mutex;
> pthread_cond_t reply_condvar;
>
> @@ -133,8 +133,8 @@ static void *read_thread(void *arg);
> struct xs_handle {
> int fd;
> Xentoolcore__Active_Handle tc_ah; /* for restrict */
> - struct list_head reply_list;
> - struct list_head watch_list;
> + XEN_TAILQ_HEAD(, struct xs_stored_msg) reply_list;
> + XEN_TAILQ_HEAD(, struct xs_stored_msg) watch_list;
> /* Clients can select() on this pipe to wait for a watch to fire. */
> int watch_pipe[2];
> /* Filtering watch event in unwatch function? */
> @@ -180,7 +180,7 @@ int xs_fileno(struct xs_handle *h)
>
> if ((h->watch_pipe[0] == -1) && (pipe(h->watch_pipe) != -1)) {
> /* Kick things off if the watch list is already non-empty. */
> - if (!list_empty(&h->watch_list))
> + if (!XEN_TAILQ_EMPTY(&h->watch_list))
> while (write(h->watch_pipe[1], &c, 1) != 1)
> continue;
> }
> @@ -262,8 +262,8 @@ static struct xs_handle *get_handle(const char *connect_to)
> if (h->fd == -1)
> goto err;
>
> - INIT_LIST_HEAD(&h->reply_list);
> - INIT_LIST_HEAD(&h->watch_list);
> + XEN_TAILQ_INIT(&h->reply_list);
> + XEN_TAILQ_INIT(&h->watch_list);
>
> /* Watch pipe is allocated on demand in xs_fileno(). */
> h->watch_pipe[0] = h->watch_pipe[1] = -1;
> @@ -329,12 +329,12 @@ struct xs_handle *xs_open(unsigned long flags)
> static void close_free_msgs(struct xs_handle *h) {
> struct xs_stored_msg *msg, *tmsg;
>
> - list_for_each_entry_safe(msg, tmsg, &h->reply_list, list) {
> + XEN_TAILQ_FOREACH_SAFE(msg, &h->reply_list, list, tmsg) {
> free(msg->body);
> free(msg);
> }
>
> - list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
> + XEN_TAILQ_FOREACH_SAFE(msg, &h->watch_list, list, tmsg) {
> free(msg->body);
> free(msg);
> }
> @@ -459,17 +459,17 @@ static void *read_reply(
>
> mutex_lock(&h->reply_mutex);
> #ifdef USE_PTHREAD
> - while (list_empty(&h->reply_list) && read_from_thread && h->fd != -1)
> + while (XEN_TAILQ_EMPTY(&h->reply_list) && read_from_thread && h->fd != -1)
> condvar_wait(&h->reply_condvar, &h->reply_mutex);
> #endif
> - if (list_empty(&h->reply_list)) {
> + if (XEN_TAILQ_EMPTY(&h->reply_list)) {
> mutex_unlock(&h->reply_mutex);
> errno = EINVAL;
> return NULL;
> }
> - msg = list_top(&h->reply_list, struct xs_stored_msg, list);
> - list_del(&msg->list);
> - assert(list_empty(&h->reply_list));
> + msg = XEN_TAILQ_FIRST(&h->reply_list);
> + XEN_TAILQ_REMOVE(&h->reply_list, msg, list);
> + assert(XEN_TAILQ_EMPTY(&h->reply_list));
> mutex_unlock(&h->reply_mutex);
>
> *type = msg->hdr.type;
> @@ -883,7 +883,7 @@ static void xs_maybe_clear_watch_pipe(struct xs_handle *h)
> {
> char c;
>
> - if (list_empty(&h->watch_list) && (h->watch_pipe[0] != -1))
> + if (XEN_TAILQ_EMPTY(&h->watch_list) && (h->watch_pipe[0] != -1))
> while (read(h->watch_pipe[0], &c, 1) != 1)
> continue;
> }
> @@ -907,7 +907,7 @@ static char **read_watch_internal(struct xs_handle *h, unsigned int *num,
> * we haven't called xs_watch. Presumably the application
> * will do so later; in the meantime we just block.
> */
> - while (list_empty(&h->watch_list) && h->fd != -1) {
> + while (XEN_TAILQ_EMPTY(&h->watch_list) && h->fd != -1) {
> if (nonblocking) {
> mutex_unlock(&h->watch_mutex);
> errno = EAGAIN;
> @@ -925,13 +925,13 @@ static char **read_watch_internal(struct xs_handle *h, unsigned int *num,
>
> #endif /* !defined(USE_PTHREAD) */
>
> - if (list_empty(&h->watch_list)) {
> + if (XEN_TAILQ_EMPTY(&h->watch_list)) {
> mutex_unlock(&h->watch_mutex);
> errno = EINVAL;
> return NULL;
> }
> - msg = list_top(&h->watch_list, struct xs_stored_msg, list);
> - list_del(&msg->list);
> + msg = XEN_TAILQ_FIRST(&h->watch_list);
> + XEN_TAILQ_REMOVE(&h->watch_list, msg, list);
>
> xs_maybe_clear_watch_pipe(h);
> mutex_unlock(&h->watch_mutex);
> @@ -1007,12 +1007,12 @@ bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
> /* Filter the watch list to remove potential message */
> mutex_lock(&h->watch_mutex);
>
> - if (list_empty(&h->watch_list)) {
> + if (XEN_TAILQ_EMPTY(&h->watch_list)) {
> mutex_unlock(&h->watch_mutex);
> return res;
> }
>
> - list_for_each_entry_safe(msg, tmsg, &h->watch_list, list) {
> + XEN_TAILQ_FOREACH_SAFE(msg, &h->watch_list, list, tmsg) {
> assert(msg->hdr.type == XS_WATCH_EVENT);
>
> s = msg->body;
> @@ -1034,7 +1034,7 @@ bool xs_unwatch(struct xs_handle *h, const char *path, const char *token)
>
> if (l_token && !strcmp(token, l_token) &&
> l_path && xs_path_is_subpath(path, l_path)) {
> - list_del(&msg->list);
> + XEN_TAILQ_REMOVE(&h->watch_list, msg, list);
> free(msg);
> }
> }
> @@ -1290,12 +1290,12 @@ static int read_message(struct xs_handle *h, int nonblocking)
> cleanup_push(pthread_mutex_unlock, &h->watch_mutex);
>
> /* Kick users out of their select() loop. */
> - if (list_empty(&h->watch_list) &&
> + if (XEN_TAILQ_EMPTY(&h->watch_list) &&
> (h->watch_pipe[1] != -1))
> while (write(h->watch_pipe[1], body, 1) != 1) /* Cancellation point */
> continue;
>
> - list_add_tail(&msg->list, &h->watch_list);
> + XEN_TAILQ_INSERT_TAIL(&h->watch_list, msg, list);
>
> condvar_signal(&h->watch_condvar);
>
> @@ -1304,13 +1304,13 @@ static int read_message(struct xs_handle *h, int nonblocking)
> mutex_lock(&h->reply_mutex);
>
> /* There should only ever be one response pending! */
> - if (!list_empty(&h->reply_list)) {
> + if (!XEN_TAILQ_EMPTY(&h->reply_list)) {
> mutex_unlock(&h->reply_mutex);
> saved_errno = EEXIST;
> goto error_freebody;
> }
>
> - list_add_tail(&msg->list, &h->reply_list);
> + XEN_TAILQ_INSERT_TAIL(&h->reply_list, msg, list);
> condvar_signal(&h->reply_condvar);
>
> mutex_unlock(&h->reply_mutex);
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-06-09 18:09 ` Julien Grall
@ 2023-06-12 7:02 ` Juergen Gross
2023-06-12 10:34 ` Julien Grall
0 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-06-12 7:02 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Wei Liu, Anthony PERARD
[-- Attachment #1.1.1: Type: text/plain, Size: 1108 bytes --]
On 09.06.23 20:09, Julien Grall wrote:
> Hi Juergen,
>
> On 30/05/2023 09:54, Juergen Gross wrote:
>> Replace the usage of the xenstore private list.h header with the
>> common xen_list.h one.
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> ---
>> V3:
>> - new patch
>> ---
>> tools/libs/store/xs.c | 56 +++++++++++++++++++++----------------------
>> 1 file changed, 28 insertions(+), 28 deletions(-)
>>
>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
>> index 7a9a8b1656..3813b69ae2 100644
>> --- a/tools/libs/store/xs.c
>> +++ b/tools/libs/store/xs.c
>> @@ -35,13 +35,13 @@
>> #include <errno.h>
>> #include "xenstore.h"
>> #include "xs_lib.h"
>> -#include "list.h"
>> #include "utils.h"
>> #include <xentoolcore_internal.h>
>> +#include <xen_list.h>
>> struct xs_stored_msg {
>> - struct list_head list;
>> + XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
>
> I have expected us to use to XEN_LIST_*. Can you explain why you didn't use them?
XEN_LIST_* doesn't provide a list_add_tail() replacement.
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-06-12 7:02 ` Juergen Gross
@ 2023-06-12 10:34 ` Julien Grall
2023-06-12 10:37 ` Juergen Gross
0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2023-06-12 10:34 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
Hi,
On 12/06/2023 08:02, Juergen Gross wrote:
> On 09.06.23 20:09, Julien Grall wrote:
>> Hi Juergen,
>>
>> On 30/05/2023 09:54, Juergen Gross wrote:
>>> Replace the usage of the xenstore private list.h header with the
>>> common xen_list.h one.
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>> ---
>>> V3:
>>> - new patch
>>> ---
>>> tools/libs/store/xs.c | 56 +++++++++++++++++++++----------------------
>>> 1 file changed, 28 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
>>> index 7a9a8b1656..3813b69ae2 100644
>>> --- a/tools/libs/store/xs.c
>>> +++ b/tools/libs/store/xs.c
>>> @@ -35,13 +35,13 @@
>>> #include <errno.h>
>>> #include "xenstore.h"
>>> #include "xs_lib.h"
>>> -#include "list.h"
>>> #include "utils.h"
>>> #include <xentoolcore_internal.h>
>>> +#include <xen_list.h>
>>> struct xs_stored_msg {
>>> - struct list_head list;
>>> + XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
>>
>> I have expected us to use to XEN_LIST_*. Can you explain why you
>> didn't use them?
>
> XEN_LIST_* doesn't provide a list_add_tail() replacement.
Ok. Did you look at whether list_add_tail() could be replaced with
adding the element at the head?
Anyway, I can understand that this would not be a straight swap. But it
would be good to have some rationale in the commit message as this helps
understanding the choice.
With that:
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-06-12 10:34 ` Julien Grall
@ 2023-06-12 10:37 ` Juergen Gross
2023-06-12 10:39 ` Julien Grall
0 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-06-12 10:37 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Wei Liu, Anthony PERARD
[-- Attachment #1.1.1: Type: text/plain, Size: 1803 bytes --]
On 12.06.23 12:34, Julien Grall wrote:
> Hi,
>
> On 12/06/2023 08:02, Juergen Gross wrote:
>> On 09.06.23 20:09, Julien Grall wrote:
>>> Hi Juergen,
>>>
>>> On 30/05/2023 09:54, Juergen Gross wrote:
>>>> Replace the usage of the xenstore private list.h header with the
>>>> common xen_list.h one.
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>> ---
>>>> V3:
>>>> - new patch
>>>> ---
>>>> tools/libs/store/xs.c | 56 +++++++++++++++++++++----------------------
>>>> 1 file changed, 28 insertions(+), 28 deletions(-)
>>>>
>>>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
>>>> index 7a9a8b1656..3813b69ae2 100644
>>>> --- a/tools/libs/store/xs.c
>>>> +++ b/tools/libs/store/xs.c
>>>> @@ -35,13 +35,13 @@
>>>> #include <errno.h>
>>>> #include "xenstore.h"
>>>> #include "xs_lib.h"
>>>> -#include "list.h"
>>>> #include "utils.h"
>>>> #include <xentoolcore_internal.h>
>>>> +#include <xen_list.h>
>>>> struct xs_stored_msg {
>>>> - struct list_head list;
>>>> + XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
>>>
>>> I have expected us to use to XEN_LIST_*. Can you explain why you didn't use
>>> them?
>>
>> XEN_LIST_* doesn't provide a list_add_tail() replacement.
>
> Ok. Did you look at whether list_add_tail() could be replaced with adding the
> element at the head?
>
> Anyway, I can understand that this would not be a straight swap. But it would be
> good to have some rationale in the commit message as this helps understanding
> the choice.
Okay, I'll add: "Use the XEN_TAILQ type list, as it allows to directly swap the
related macros/functions without having to change the logic."
>
> With that:
>
> Acked-by: Julien Grall <jgrall@amazon.com>
Thanks,
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h
2023-06-12 10:37 ` Juergen Gross
@ 2023-06-12 10:39 ` Julien Grall
0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-12 10:39 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
On 12/06/2023 11:37, Juergen Gross wrote:
> On 12.06.23 12:34, Julien Grall wrote:
>> Hi,
>>
>> On 12/06/2023 08:02, Juergen Gross wrote:
>>> On 09.06.23 20:09, Julien Grall wrote:
>>>> Hi Juergen,
>>>>
>>>> On 30/05/2023 09:54, Juergen Gross wrote:
>>>>> Replace the usage of the xenstore private list.h header with the
>>>>> common xen_list.h one.
>>>>>
>>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>>> ---
>>>>> V3:
>>>>> - new patch
>>>>> ---
>>>>> tools/libs/store/xs.c | 56
>>>>> +++++++++++++++++++++----------------------
>>>>> 1 file changed, 28 insertions(+), 28 deletions(-)
>>>>>
>>>>> diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
>>>>> index 7a9a8b1656..3813b69ae2 100644
>>>>> --- a/tools/libs/store/xs.c
>>>>> +++ b/tools/libs/store/xs.c
>>>>> @@ -35,13 +35,13 @@
>>>>> #include <errno.h>
>>>>> #include "xenstore.h"
>>>>> #include "xs_lib.h"
>>>>> -#include "list.h"
>>>>> #include "utils.h"
>>>>> #include <xentoolcore_internal.h>
>>>>> +#include <xen_list.h>
>>>>> struct xs_stored_msg {
>>>>> - struct list_head list;
>>>>> + XEN_TAILQ_ENTRY(struct xs_stored_msg) list;
>>>>
>>>> I have expected us to use to XEN_LIST_*. Can you explain why you
>>>> didn't use them?
>>>
>>> XEN_LIST_* doesn't provide a list_add_tail() replacement.
>>
>> Ok. Did you look at whether list_add_tail() could be replaced with
>> adding the element at the head?
>>
>> Anyway, I can understand that this would not be a straight swap. But
>> it would be good to have some rationale in the commit message as this
>> helps understanding the choice.
>
> Okay, I'll add: "Use the XEN_TAILQ type list, as it allows to directly
> swap the
> related macros/functions without having to change the logic."
Thanks. I am happy to add it whilst committing.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 11/16] tools/libs/store: make libxenstore independent of utils.h
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (9 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 10/16] tools/libs/store: use xen_list.h instead of xenstore/list.h Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-09 18:10 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 12/16] tools/xenstore: remove no longer needed functions from xs_lib.c Juergen Gross
` (5 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
There is no real need for including tools/xenstore/utils.h from
libxenstore, as only streq() and ARRAY_SIZE() are obtained via that
header.
streq() is just !strcmp(), and ARRAY_SIZE() is brought in via
xen-tools/common-macros.h.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
---
tools/libs/store/xs.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 3813b69ae2..76ffb1be45 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -33,9 +33,9 @@
#include <signal.h>
#include <stdint.h>
#include <errno.h>
+#include <xen-tools/common-macros.h>
#include "xenstore.h"
#include "xs_lib.h"
-#include "utils.h"
#include <xentoolcore_internal.h>
#include <xen_list.h>
@@ -437,7 +437,7 @@ static int get_error(const char *errorstring)
{
unsigned int i;
- for (i = 0; !streq(errorstring, xsd_errors[i].errstring); i++)
+ for (i = 0; strcmp(errorstring, xsd_errors[i].errstring); i++)
if (i == ARRAY_SIZE(xsd_errors) - 1)
return EINVAL;
return xsd_errors[i].errnum;
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 12/16] tools/xenstore: remove no longer needed functions from xs_lib.c
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (10 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 11/16] tools/libs/store: make libxenstore independent of utils.h Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-15 21:00 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 13/16] tools/xenstore: replace xs_lib.c with a header Juergen Gross
` (4 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
xs_daemon_tdb() in xs_lib.c is no longer used at all, so it can be
removed. xs_domain_dev() and xs_write_all() are not used by xenstored,
so they can be moved to tools/libs/store/xs.c.
xs_daemon_rootdir() is used by xenstored only and it only calls
xs_daemon_rundir(), so replace its use cases with xs_daemon_rundir()
and remove it from xs_lib.c.
xs_daemon_socket_ro() is needed in libxenstore only, so move it to
tools/libs/store/xs.c.
Move functions used by xenstore-client only to xenstore_client.c.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V2:
- move xs_write_all(), too
V3:
- remove xs_daemon_rootdir(), move xs_daemon_socket_ro()
---
tools/include/xenstore.h | 3 +
tools/include/xenstore_lib.h | 3 -
tools/libs/store/xs.c | 43 ++++++++
tools/xenstore/xenstore_client.c | 129 ++++++++++++++++++++++
tools/xenstore/xenstored_control.c | 4 +-
tools/xenstore/xenstored_core.c | 3 +-
tools/xenstore/xs_lib.c | 166 -----------------------------
tools/xenstore/xs_lib.h | 19 ----
8 files changed, 178 insertions(+), 192 deletions(-)
diff --git a/tools/include/xenstore.h b/tools/include/xenstore.h
index 2b3f69fb61..a442252849 100644
--- a/tools/include/xenstore.h
+++ b/tools/include/xenstore.h
@@ -113,6 +113,9 @@ 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);
+/* Simple write function: loops for you. */
+bool xs_write_all(int fd, const void *data, unsigned int len);
+
/* Create a new directory.
* Returns false on failure, or success if it already exists.
*/
diff --git a/tools/include/xenstore_lib.h b/tools/include/xenstore_lib.h
index 2266009ec1..43eec87379 100644
--- a/tools/include/xenstore_lib.h
+++ b/tools/include/xenstore_lib.h
@@ -47,9 +47,6 @@ const char *xs_daemon_rundir(void);
const char *xs_daemon_socket(void);
const char *xs_daemon_socket_ro(void);
-/* Simple write function: loops for you. */
-bool xs_write_all(int fd, const void *data, unsigned int len);
-
/* Convert strings to permissions. False if a problem. */
bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num,
const char *strings);
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index 76ffb1be45..bb93246bfb 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -311,6 +311,26 @@ struct xs_handle *xs_domain_open(void)
return xs_open(0);
}
+static const char *xs_domain_dev(void)
+{
+ char *s = getenv("XENSTORED_PATH");
+ if (s)
+ return s;
+#if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__)
+ return "/dev/xen/xenbus";
+#elif defined(__linux__)
+ if (access("/dev/xen/xenbus", F_OK) == 0)
+ return "/dev/xen/xenbus";
+ return "/proc/xen/xenbus";
+#elif defined(__NetBSD__)
+ return "/kern/xen/xenbus";
+#elif defined(__FreeBSD__)
+ return "/dev/xen/xenstore";
+#else
+ return "/dev/xen/xenbus";
+#endif
+}
+
struct xs_handle *xs_open(unsigned long flags)
{
struct xs_handle *xsh = NULL;
@@ -431,6 +451,24 @@ out_false:
#ifdef XSTEST
#define read_all read_all_choice
#define xs_write_all write_all_choice
+#else
+/* Simple routine for writing to sockets, etc. */
+bool xs_write_all(int fd, const void *data, unsigned int len)
+{
+ while (len) {
+ int done;
+
+ done = write(fd, data, len);
+ if (done < 0 && errno == EINTR)
+ continue;
+ if (done <= 0)
+ return false;
+ data += done;
+ len -= done;
+ }
+
+ return true;
+}
#endif
static int get_error(const char *errorstring)
@@ -1328,6 +1366,11 @@ error:
return ret;
}
+const char *xs_daemon_socket_ro(void)
+{
+ return xs_daemon_socket();
+}
+
#ifdef USE_PTHREAD
static void *read_thread(void *arg)
{
diff --git a/tools/xenstore/xenstore_client.c b/tools/xenstore/xenstore_client.c
index 0628ba275e..8ff8abf12a 100644
--- a/tools/xenstore/xenstore_client.c
+++ b/tools/xenstore/xenstore_client.c
@@ -8,6 +8,7 @@
*
*/
+#include <assert.h>
#include <err.h>
#include <errno.h>
#include <fcntl.h>
@@ -40,12 +41,140 @@ enum mode {
MODE_watch,
};
+/* Sanitising (quoting) possibly-binary strings. */
+struct expanding_buffer {
+ char *buf;
+ int avail;
+};
+
static char *output_buf = NULL;
static int output_pos = 0;
static struct expanding_buffer ebuf;
static int output_size = 0;
+/* Ensure that given expanding buffer has at least min_avail characters. */
+static char *expanding_buffer_ensure(struct expanding_buffer *ebuf,
+ int min_avail)
+{
+ int want;
+ char *got;
+
+ if ( ebuf->avail >= min_avail )
+ return ebuf->buf;
+
+ if ( min_avail >= INT_MAX/3 )
+ return 0;
+
+ want = ebuf->avail + min_avail + 10;
+ got = realloc(ebuf->buf, want);
+ if ( !got )
+ return 0;
+
+ ebuf->buf = got;
+ ebuf->avail = want;
+ return ebuf->buf;
+}
+
+/* sanitise_value() may return NULL if malloc fails. */
+static char *sanitise_value(struct expanding_buffer *ebuf,
+ const char *val, unsigned len)
+{
+ int used, remain, c;
+ unsigned char *ip;
+
+#define ADD(c) (ebuf->buf[used++] = (c))
+#define ADDF(f,c) (used += sprintf(ebuf->buf+used, (f), (c)))
+
+ assert(len < INT_MAX/5);
+
+ ip = (unsigned char *)val;
+ used = 0;
+ remain = len;
+
+ if ( !expanding_buffer_ensure(ebuf, remain + 1) )
+ return NULL;
+
+ while ( remain-- > 0 )
+ {
+ c= *ip++;
+
+ if ( c >= ' ' && c <= '~' && c != '\\' )
+ {
+ ADD(c);
+ continue;
+ }
+
+ if ( !expanding_buffer_ensure(ebuf, used + remain + 5) )
+ /* for "<used>\\nnn<remain>\0" */
+ return 0;
+
+ ADD('\\');
+ switch (c)
+ {
+ case '\t': ADD('t'); break;
+ case '\n': ADD('n'); break;
+ case '\r': ADD('r'); break;
+ case '\\': ADD('\\'); break;
+ default:
+ if ( c < 010 ) ADDF("%03o", c);
+ else ADDF("x%02x", c);
+ }
+ }
+
+ ADD(0);
+ assert(used <= ebuf->avail);
+ return ebuf->buf;
+
+#undef ADD
+#undef ADDF
+}
+
+/* *out_len_r on entry is ignored; out must be at least strlen(in)+1 bytes. */
+static void unsanitise_value(char *out, unsigned *out_len_r, const char *in)
+{
+ const char *ip;
+ char *op;
+ unsigned c;
+ int n;
+
+ for ( ip = in, op = out; (c = *ip++); *op++ = c )
+ {
+ if ( c == '\\' )
+ {
+ c = *ip++;
+
+#define GETF(f) do \
+{ \
+ n = 0; \
+ sscanf(ip, f "%n", &c, &n); \
+ ip += n; \
+} while ( 0 )
+
+ switch ( c )
+ {
+ case 't': c= '\t'; break;
+ case 'n': c= '\n'; break;
+ case 'r': c= '\r'; break;
+ case '\\': c= '\\'; break;
+ case 'x': GETF("%2x"); break;
+ case '0': case '4':
+ case '1': case '5':
+ case '2': case '6':
+ case '3': case '7': --ip; GETF("%3o"); break;
+ case 0: --ip; break;
+ default:;
+ }
+#undef GETF
+ }
+ }
+
+ *op = 0;
+
+ if ( out_len_r )
+ *out_len_r = op - out;
+}
+
/* make sure there is at least 'len' more space in output_buf */
static void expand_buffer(size_t len)
{
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index ed80924ee4..6ef800ff64 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -560,7 +560,7 @@ static FILE *lu_dump_open(const void *ctx)
char *filename;
int fd;
- filename = talloc_asprintf(ctx, "%s/state_dump", xs_daemon_rootdir());
+ filename = talloc_asprintf(ctx, "%s/state_dump", xs_daemon_rundir());
if (!filename)
return NULL;
@@ -583,7 +583,7 @@ static void lu_get_dump_state(struct lu_dump_state *state)
state->size = 0;
state->filename = talloc_asprintf(NULL, "%s/state_dump",
- xs_daemon_rootdir());
+ xs_daemon_rundir());
if (!state->filename)
barf("Allocation failure");
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 07fbac55ac..a78cbf1116 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2888,10 +2888,9 @@ int main(int argc, char *argv[])
reopen_log();
- /* make sure xenstored directories exist */
+ /* Make sure xenstored directory exists. */
/* Errors ignored here, will be reported when we open files */
mkdir(xs_daemon_rundir(), 0755);
- mkdir(xs_daemon_rootdir(), 0755);
if (dofork) {
openlog("xenstored", 0, LOG_DAEMON);
diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c
index b9941c567c..826fa7b5a8 100644
--- a/tools/xenstore/xs_lib.c
+++ b/tools/xenstore/xs_lib.c
@@ -32,11 +32,6 @@ const char *xs_daemon_rundir(void)
return (s ? s : XEN_RUN_STORED);
}
-const char *xs_daemon_rootdir(void)
-{
- return xs_daemon_rundir();
-}
-
static const char *xs_daemon_path(void)
{
static char buf[PATH_MAX];
@@ -49,61 +44,11 @@ static const char *xs_daemon_path(void)
return buf;
}
-const char *xs_daemon_tdb(void)
-{
- static char buf[PATH_MAX];
- snprintf(buf, sizeof(buf), "%s/tdb", xs_daemon_rootdir());
- return buf;
-}
-
const char *xs_daemon_socket(void)
{
return xs_daemon_path();
}
-const char *xs_daemon_socket_ro(void)
-{
- return xs_daemon_path();
-}
-
-const char *xs_domain_dev(void)
-{
- char *s = getenv("XENSTORED_PATH");
- if (s)
- return s;
-#if defined(__RUMPUSER_XEN__) || defined(__RUMPRUN__)
- return "/dev/xen/xenbus";
-#elif defined(__linux__)
- if (access("/dev/xen/xenbus", F_OK) == 0)
- return "/dev/xen/xenbus";
- return "/proc/xen/xenbus";
-#elif defined(__NetBSD__)
- return "/kern/xen/xenbus";
-#elif defined(__FreeBSD__)
- return "/dev/xen/xenstore";
-#else
- return "/dev/xen/xenbus";
-#endif
-}
-
-/* Simple routines for writing to sockets, etc. */
-bool xs_write_all(int fd, const void *data, unsigned int len)
-{
- while (len) {
- int done;
-
- done = write(fd, data, len);
- if (done < 0 && errno == EINTR)
- continue;
- if (done <= 0)
- return false;
- data += done;
- len -= done;
- }
-
- return true;
-}
-
/* Convert strings to permissions. False if a problem. */
bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num,
const char *strings)
@@ -179,114 +124,3 @@ unsigned int xs_count_strings(const char *strings, unsigned int len)
return num;
}
-
-char *expanding_buffer_ensure(struct expanding_buffer *ebuf, int min_avail)
-{
- int want;
- char *got;
-
- if (ebuf->avail >= min_avail)
- return ebuf->buf;
-
- if (min_avail >= INT_MAX/3)
- return 0;
-
- want = ebuf->avail + min_avail + 10;
- got = realloc(ebuf->buf, want);
- if (!got)
- return 0;
-
- ebuf->buf = got;
- ebuf->avail = want;
- return ebuf->buf;
-}
-
-char *sanitise_value(struct expanding_buffer *ebuf,
- const char *val, unsigned len)
-{
- int used, remain, c;
- unsigned char *ip;
-
-#define ADD(c) (ebuf->buf[used++] = (c))
-#define ADDF(f,c) (used += sprintf(ebuf->buf+used, (f), (c)))
-
- assert(len < INT_MAX/5);
-
- ip = (unsigned char *)val;
- used = 0;
- remain = len;
-
- if (!expanding_buffer_ensure(ebuf, remain + 1))
- return NULL;
-
- while (remain-- > 0) {
- c= *ip++;
-
- if (c >= ' ' && c <= '~' && c != '\\') {
- ADD(c);
- continue;
- }
-
- if (!expanding_buffer_ensure(ebuf, used + remain + 5))
- /* for "<used>\\nnn<remain>\0" */
- return 0;
-
- ADD('\\');
- switch (c) {
- case '\t': ADD('t'); break;
- case '\n': ADD('n'); break;
- case '\r': ADD('r'); break;
- case '\\': ADD('\\'); break;
- default:
- if (c < 010) ADDF("%03o", c);
- else ADDF("x%02x", c);
- }
- }
-
- ADD(0);
- assert(used <= ebuf->avail);
- return ebuf->buf;
-
-#undef ADD
-#undef ADDF
-}
-
-void unsanitise_value(char *out, unsigned *out_len_r, const char *in)
-{
- const char *ip;
- char *op;
- unsigned c;
- int n;
-
- for (ip = in, op = out; (c = *ip++); *op++ = c) {
- if (c == '\\') {
- c = *ip++;
-
-#define GETF(f) do { \
- n = 0; \
- sscanf(ip, f "%n", &c, &n); \
- ip += n; \
- } while (0)
-
- switch (c) {
- case 't': c= '\t'; break;
- case 'n': c= '\n'; break;
- case 'r': c= '\r'; break;
- case '\\': c= '\\'; break;
- case 'x': GETF("%2x"); break;
- case '0': case '4':
- case '1': case '5':
- case '2': case '6':
- case '3': case '7': --ip; GETF("%3o"); break;
- case 0: --ip; break;
- default:;
- }
-#undef GETF
- }
- }
-
- *op = 0;
-
- if (out_len_r)
- *out_len_r = op - out;
-}
diff --git a/tools/xenstore/xs_lib.h b/tools/xenstore/xs_lib.h
index efa05997d6..5b9874d741 100644
--- a/tools/xenstore/xs_lib.h
+++ b/tools/xenstore/xs_lib.h
@@ -21,10 +21,6 @@
#include "xenstore_lib.h"
-const char *xs_daemon_rootdir(void);
-const char *xs_domain_dev(void);
-const char *xs_daemon_tdb(void);
-
/* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
bool xs_perm_to_string(const struct xs_permissions *perm,
char *buffer, size_t buf_len);
@@ -32,19 +28,4 @@ bool xs_perm_to_string(const struct xs_permissions *perm,
/* Given a string and a length, count how many strings (nul terms). */
unsigned int xs_count_strings(const char *strings, unsigned int len);
-/* Sanitising (quoting) possibly-binary strings. */
-struct expanding_buffer {
- char *buf;
- int avail;
-};
-
-/* Ensure that given expanding buffer has at least min_avail characters. */
-char *expanding_buffer_ensure(struct expanding_buffer *, int min_avail);
-
-/* sanitise_value() may return NULL if malloc fails. */
-char *sanitise_value(struct expanding_buffer *, const char *val, unsigned len);
-
-/* *out_len_r on entry is ignored; out must be at least strlen(in)+1 bytes. */
-void unsanitise_value(char *out, unsigned *out_len_r, const char *in);
-
#endif /* XS_LIB_H */
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 12/16] tools/xenstore: remove no longer needed functions from xs_lib.c
2023-05-30 8:54 ` [PATCH v3 12/16] tools/xenstore: remove no longer needed functions from xs_lib.c Juergen Gross
@ 2023-06-15 21:00 ` Julien Grall
0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-15 21:00 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> xs_daemon_tdb() in xs_lib.c is no longer used at all, so it can be
> removed. xs_domain_dev() and xs_write_all() are not used by xenstored,
> so they can be moved to tools/libs/store/xs.c.
>
> xs_daemon_rootdir() is used by xenstored only and it only calls
> xs_daemon_rundir(), so replace its use cases with xs_daemon_rundir()
> and remove it from xs_lib.c.
>
> xs_daemon_socket_ro() is needed in libxenstore only, so move it to
> tools/libs/store/xs.c.
>
> Move functions used by xenstore-client only to xenstore_client.c.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 13/16] tools/xenstore: replace xs_lib.c with a header
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (11 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 12/16] tools/xenstore: remove no longer needed functions from xs_lib.c Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-15 21:03 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 14/16] tools/xenstore: split out environment specific live update code Juergen Gross
` (3 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Jan Beulich,
Julien Grall, Stefano Stabellini, Wei Liu, Anthony PERARD
Instead of including the same small C source in multiple binaries from
2 source directories, use a header file with inline functions as a
replacement.
As some of the functions are exported by libxenstore, rename the inline
functions from xs_*() do xenstore_*() and add xs_*() wrappers to
libxenstore.
With that no sources required to build libxenstore are left in
tools/xenstore, so the file COPYING can be removed now.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- new patch
---
MAINTAINERS | 1 +
tools/include/xen-tools/xenstore-common.h | 128 ++++++
tools/libs/store/Makefile | 4 -
tools/libs/store/xs.c | 28 +-
tools/xenstore/COPYING | 514 ----------------------
tools/xenstore/Makefile | 2 +-
tools/xenstore/Makefile.common | 2 +-
tools/xenstore/xenstore_client.c | 4 +-
tools/xenstore/xenstored_control.c | 9 +-
tools/xenstore/xenstored_core.c | 15 +-
tools/xenstore/xs_lib.c | 126 ------
tools/xenstore/xs_lib.h | 31 --
12 files changed, 168 insertions(+), 696 deletions(-)
create mode 100644 tools/include/xen-tools/xenstore-common.h
delete mode 100644 tools/xenstore/COPYING
delete mode 100644 tools/xenstore/xs_lib.c
delete mode 100644 tools/xenstore/xs_lib.h
diff --git a/MAINTAINERS b/MAINTAINERS
index f2f1881b32..751703373d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -657,6 +657,7 @@ F: tools/helpers/init-xenstore-domain.c
F: tools/include/xenstore-compat/
F: tools/include/xenstore.h
F: tools/include/xenstore_lib.h
+F: tools/include/xen-tools/xenstore-common.h
F: tools/libs/store/
F: tools/xenstore/
diff --git a/tools/include/xen-tools/xenstore-common.h b/tools/include/xen-tools/xenstore-common.h
new file mode 100644
index 0000000000..8cb097a8fc
--- /dev/null
+++ b/tools/include/xen-tools/xenstore-common.h
@@ -0,0 +1,128 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Common functions for libxenstore, xenstored and xenstore-clients. */
+
+#ifndef __XEN_TOOLS_XENSTORE_COMMON__
+#define __XEN_TOOLS_XENSTORE_COMMON__
+
+#include <limits.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <xenstore_lib.h>
+
+static inline const char *xenstore_daemon_rundir(void)
+{
+ char *s = getenv("XENSTORED_RUNDIR");
+
+ return s ? s : XEN_RUN_STORED;
+}
+
+static inline const char *xenstore_daemon_path(void)
+{
+ static char buf[PATH_MAX];
+ char *s = getenv("XENSTORED_PATH");
+
+ if ( s )
+ return s;
+
+ if ( snprintf(buf, sizeof(buf), "%s/socket", xenstore_daemon_rundir()) >=
+ PATH_MAX )
+ return NULL;
+
+ return buf;
+}
+
+/* Convert strings to permissions. False if a problem. */
+static inline bool xenstore_strings_to_perms(struct xs_permissions *perms,
+ unsigned int num,
+ const char *strings)
+{
+ const char *p;
+ char *end;
+ unsigned int i;
+
+ for ( p = strings, i = 0; i < num; i++ )
+ {
+ /* "r", "w", or "b" for both. */
+ switch ( *p )
+ {
+ case 'r':
+ perms[i].perms = XS_PERM_READ;
+ break;
+
+ case 'w':
+ perms[i].perms = XS_PERM_WRITE;
+ break;
+
+ case 'b':
+ perms[i].perms = XS_PERM_READ|XS_PERM_WRITE;
+ break;
+
+ case 'n':
+ perms[i].perms = XS_PERM_NONE;
+ break;
+
+ default:
+ errno = EINVAL;
+ return false;
+ }
+
+ p++;
+ perms[i].id = strtol(p, &end, 0);
+ if ( *end || !*p )
+ {
+ errno = EINVAL;
+ return false;
+ }
+
+ p = end + 1;
+ }
+
+ return true;
+}
+
+/* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
+static inline bool xenstore_perm_to_string(const struct xs_permissions *perm,
+ char *buffer, size_t buf_len)
+{
+ switch ( (int)perm->perms & ~XS_PERM_IGNORE )
+ {
+ case XS_PERM_WRITE:
+ *buffer = 'w';
+ break;
+
+ case XS_PERM_READ:
+ *buffer = 'r';
+ break;
+
+ case XS_PERM_READ|XS_PERM_WRITE:
+ *buffer = 'b';
+ break;
+
+ case XS_PERM_NONE:
+ *buffer = 'n';
+ break;
+
+ default:
+ errno = EINVAL;
+ return false;
+ }
+
+ snprintf(buffer + 1, buf_len - 1, "%i", (int)perm->id);
+
+ return true;
+}
+
+/* Given a string and a length, count how many strings (nul terms). */
+static inline unsigned int xenstore_count_strings(const char *strings,
+ unsigned int len)
+{
+ unsigned int num;
+ const char *p;
+
+ for ( p = strings, num = 0; p < strings + len; p++ )
+ if ( *p == '\0' )
+ num++;
+
+ return num;
+}
+#endif /* __XEN_TOOLS_XENSTORE_COMMON__ */
diff --git a/tools/libs/store/Makefile b/tools/libs/store/Makefile
index daed9d148f..c1a1508713 100644
--- a/tools/libs/store/Makefile
+++ b/tools/libs/store/Makefile
@@ -9,7 +9,6 @@ ifeq ($(CONFIG_Linux),y)
LDLIBS += -ldl
endif
-OBJS-y += xs_lib.o
OBJS-y += xs.o
LIBHEADER = xenstore.h xenstore_lib.h
@@ -21,9 +20,6 @@ CFLAGS += -include $(XEN_ROOT)/tools/config.h
CFLAGS += $(CFLAGS_libxentoolcore)
CFLAGS += -DXEN_RUN_STORED="\"$(XEN_RUN_STORED)\""
-vpath xs_lib.c $(XEN_ROOT)/tools/xenstore
-CFLAGS += -iquote $(XEN_ROOT)/tools/xenstore
-
xs.opic: CFLAGS += -DUSE_PTHREAD
ifeq ($(CONFIG_Linux),y)
xs.opic: CFLAGS += -DUSE_DLSYM
diff --git a/tools/libs/store/xs.c b/tools/libs/store/xs.c
index bb93246bfb..140b9a2839 100644
--- a/tools/libs/store/xs.c
+++ b/tools/libs/store/xs.c
@@ -34,8 +34,8 @@
#include <stdint.h>
#include <errno.h>
#include <xen-tools/common-macros.h>
+#include <xen-tools/xenstore-common.h>
#include "xenstore.h"
-#include "xs_lib.h"
#include <xentoolcore_internal.h>
#include <xen_list.h>
@@ -627,7 +627,7 @@ static char **xs_directory_common(char *strings, unsigned int len,
char *p, **ret;
/* Count the strings. */
- *num = xs_count_strings(strings, len);
+ *num = xenstore_count_strings(strings, len);
/* Transfer to one big alloc for easy freeing. */
ret = malloc(*num * sizeof(char *) + len);
@@ -775,7 +775,7 @@ struct xs_permissions *xs_get_permissions(struct xs_handle *h,
return NULL;
/* Count the strings: each one perms then domid. */
- *num = xs_count_strings(strings, len);
+ *num = xenstore_count_strings(strings, len);
/* Transfer to one big alloc for easy freeing. */
ret = malloc(*num * sizeof(struct xs_permissions));
@@ -784,7 +784,7 @@ struct xs_permissions *xs_get_permissions(struct xs_handle *h,
return NULL;
}
- if (!xs_strings_to_perms(ret, *num, strings)) {
+ if (!xenstore_strings_to_perms(ret, *num, strings)) {
free_no_errno(ret);
ret = NULL;
}
@@ -811,7 +811,7 @@ bool xs_set_permissions(struct xs_handle *h,
for (i = 0; i < num_perms; i++) {
char buffer[MAX_STRLEN(unsigned int)+1];
- if (!xs_perm_to_string(&perms[i], buffer, sizeof(buffer)))
+ if (!xenstore_perm_to_string(&perms[i], buffer, sizeof(buffer)))
goto unwind;
iov[i+1].iov_base = strdup(buffer);
@@ -977,7 +977,7 @@ static char **read_watch_internal(struct xs_handle *h, unsigned int *num,
assert(msg->hdr.type == XS_WATCH_EVENT);
strings = msg->body;
- num_strings = xs_count_strings(strings, msg->hdr.len);
+ num_strings = xenstore_count_strings(strings, msg->hdr.len);
ret = malloc(sizeof(char*) * num_strings + msg->hdr.len);
if (!ret) {
@@ -1366,11 +1366,27 @@ error:
return ret;
}
+const char *xs_daemon_socket(void)
+{
+ return xenstore_daemon_path();
+}
+
const char *xs_daemon_socket_ro(void)
{
return xs_daemon_socket();
}
+const char *xs_daemon_rundir(void)
+{
+ return xenstore_daemon_rundir();
+}
+
+bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num,
+ const char *strings)
+{
+ return xenstore_strings_to_perms(perms, num, strings);
+}
+
#ifdef USE_PTHREAD
static void *read_thread(void *arg)
{
diff --git a/tools/xenstore/COPYING b/tools/xenstore/COPYING
deleted file mode 100644
index c764b2e329..0000000000
--- a/tools/xenstore/COPYING
+++ /dev/null
@@ -1,514 +0,0 @@
-This license (LGPL) applies to the xenstore library which interfaces
-with the xenstore daemon (as stated in xs.c, xenstore.h, xs_lib.c and
-xenstore_lib.h). The remaining files in the directory are licensed as
-stated in the comments (as of this writing, GPL, see ../../COPYING).
-
-
- GNU LESSER GENERAL PUBLIC LICENSE
- Version 2.1, February 1999
-
- Copyright (C) 1991, 1999 Free Software Foundation, Inc.
- 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
- Everyone is permitted to copy and distribute verbatim copies
- of this license document, but changing it is not allowed.
-
-[This is the first released version of the Lesser GPL. It also counts
- as the successor of the GNU Library Public License, version 2, hence
- the version number 2.1.]
-
- Preamble
-
- The licenses for most software are designed to take away your
-freedom to share and change it. By contrast, the GNU General Public
-Licenses are intended to guarantee your freedom to share and change
-free software--to make sure the software is free for all its users.
-
- This license, the Lesser General Public License, applies to some
-specially designated software packages--typically libraries--of the
-Free Software Foundation and other authors who decide to use it. You
-can use it too, but we suggest you first think carefully about whether
-this license or the ordinary General Public License is the better
-strategy to use in any particular case, based on the explanations
-below.
-
- When we speak of free software, we are referring to freedom of use,
-not price. Our General Public Licenses are designed to make sure that
-you have the freedom to distribute copies of free software (and charge
-for this service if you wish); that you receive source code or can get
-it if you want it; that you can change the software and use pieces of
-it in new free programs; and that you are informed that you can do
-these things.
-
- To protect your rights, we need to make restrictions that forbid
-distributors to deny you these rights or to ask you to surrender these
-rights. These restrictions translate to certain responsibilities for
-you if you distribute copies of the library or if you modify it.
-
- For example, if you distribute copies of the library, whether gratis
-or for a fee, you must give the recipients all the rights that we gave
-you. You must make sure that they, too, receive or can get the source
-code. If you link other code with the library, you must provide
-complete object files to the recipients, so that they can relink them
-with the library after making changes to the library and recompiling
-it. And you must show them these terms so they know their rights.
-
- We protect your rights with a two-step method: (1) we copyright the
-library, and (2) we offer you this license, which gives you legal
-permission to copy, distribute and/or modify the library.
-
- To protect each distributor, we want to make it very clear that
-there is no warranty for the free library. Also, if the library is
-modified by someone else and passed on, the recipients should know
-that what they have is not the original version, so that the original
-author's reputation will not be affected by problems that might be
-introduced by others.
-\f
- Finally, software patents pose a constant threat to the existence of
-any free program. We wish to make sure that a company cannot
-effectively restrict the users of a free program by obtaining a
-restrictive license from a patent holder. Therefore, we insist that
-any patent license obtained for a version of the library must be
-consistent with the full freedom of use specified in this license.
-
- Most GNU software, including some libraries, is covered by the
-ordinary GNU General Public License. This license, the GNU Lesser
-General Public License, applies to certain designated libraries, and
-is quite different from the ordinary General Public License. We use
-this license for certain libraries in order to permit linking those
-libraries into non-free programs.
-
- When a program is linked with a library, whether statically or using
-a shared library, the combination of the two is legally speaking a
-combined work, a derivative of the original library. The ordinary
-General Public License therefore permits such linking only if the
-entire combination fits its criteria of freedom. The Lesser General
-Public License permits more lax criteria for linking other code with
-the library.
-
- We call this license the "Lesser" General Public License because it
-does Less to protect the user's freedom than the ordinary General
-Public License. It also provides other free software developers Less
-of an advantage over competing non-free programs. These disadvantages
-are the reason we use the ordinary General Public License for many
-libraries. However, the Lesser license provides advantages in certain
-special circumstances.
-
- For example, on rare occasions, there may be a special need to
-encourage the widest possible use of a certain library, so that it
-becomes a de-facto standard. To achieve this, non-free programs must
-be allowed to use the library. A more frequent case is that a free
-library does the same job as widely used non-free libraries. In this
-case, there is little to gain by limiting the free library to free
-software only, so we use the Lesser General Public License.
-
- In other cases, permission to use a particular library in non-free
-programs enables a greater number of people to use a large body of
-free software. For example, permission to use the GNU C Library in
-non-free programs enables many more people to use the whole GNU
-operating system, as well as its variant, the GNU/Linux operating
-system.
-
- Although the Lesser General Public License is Less protective of the
-users' freedom, it does ensure that the user of a program that is
-linked with the Library has the freedom and the wherewithal to run
-that program using a modified version of the Library.
-
- The precise terms and conditions for copying, distribution and
-modification follow. Pay close attention to the difference between a
-"work based on the library" and a "work that uses the library". The
-former contains code derived from the library, whereas the latter must
-be combined with the library in order to run.
-\f
- GNU LESSER GENERAL PUBLIC LICENSE
- TERMS AND CONDITIONS FOR COPYING, DISTRIBUTION AND MODIFICATION
-
- 0. This License Agreement applies to any software library or other
-program which contains a notice placed by the copyright holder or
-other authorized party saying it may be distributed under the terms of
-this Lesser General Public License (also called "this License").
-Each licensee is addressed as "you".
-
- A "library" means a collection of software functions and/or data
-prepared so as to be conveniently linked with application programs
-(which use some of those functions and data) to form executables.
-
- The "Library", below, refers to any such software library or work
-which has been distributed under these terms. A "work based on the
-Library" means either the Library or any derivative work under
-copyright law: that is to say, a work containing the Library or a
-portion of it, either verbatim or with modifications and/or translated
-straightforwardly into another language. (Hereinafter, translation is
-included without limitation in the term "modification".)
-
- "Source code" for a work means the preferred form of the work for
-making modifications to it. For a library, complete source code means
-all the source code for all modules it contains, plus any associated
-interface definition files, plus the scripts used to control
-compilation and installation of the library.
-
- Activities other than copying, distribution and modification are not
-covered by this License; they are outside its scope. The act of
-running a program using the Library is not restricted, and output from
-such a program is covered only if its contents constitute a work based
-on the Library (independent of the use of the Library in a tool for
-writing it). Whether that is true depends on what the Library does
-and what the program that uses the Library does.
-
- 1. You may copy and distribute verbatim copies of the Library's
-complete source code as you receive it, in any medium, provided that
-you conspicuously and appropriately publish on each copy an
-appropriate copyright notice and disclaimer of warranty; keep intact
-all the notices that refer to this License and to the absence of any
-warranty; and distribute a copy of this License along with the
-Library.
-
- You may charge a fee for the physical act of transferring a copy,
-and you may at your option offer warranty protection in exchange for a
-fee.
-\f
- 2. You may modify your copy or copies of the Library or any portion
-of it, thus forming a work based on the Library, and copy and
-distribute such modifications or work under the terms of Section 1
-above, provided that you also meet all of these conditions:
-
- a) The modified work must itself be a software library.
-
- b) You must cause the files modified to carry prominent notices
- stating that you changed the files and the date of any change.
-
- c) You must cause the whole of the work to be licensed at no
- charge to all third parties under the terms of this License.
-
- d) If a facility in the modified Library refers to a function or a
- table of data to be supplied by an application program that uses
- the facility, other than as an argument passed when the facility
- is invoked, then you must make a good faith effort to ensure that,
- in the event an application does not supply such function or
- table, the facility still operates, and performs whatever part of
- its purpose remains meaningful.
-
- (For example, a function in a library to compute square roots has
- a purpose that is entirely well-defined independent of the
- application. Therefore, Subsection 2d requires that any
- application-supplied function or table used by this function must
- be optional: if the application does not supply it, the square
- root function must still compute square roots.)
-
-These requirements apply to the modified work as a whole. If
-identifiable sections of that work are not derived from the Library,
-and can be reasonably considered independent and separate works in
-themselves, then this License, and its terms, do not apply to those
-sections when you distribute them as separate works. But when you
-distribute the same sections as part of a whole which is a work based
-on the Library, the distribution of the whole must be on the terms of
-this License, whose permissions for other licensees extend to the
-entire whole, and thus to each and every part regardless of who wrote
-it.
-
-Thus, it is not the intent of this section to claim rights or contest
-your rights to work written entirely by you; rather, the intent is to
-exercise the right to control the distribution of derivative or
-collective works based on the Library.
-
-In addition, mere aggregation of another work not based on the Library
-with the Library (or with a work based on the Library) on a volume of
-a storage or distribution medium does not bring the other work under
-the scope of this License.
-
- 3. You may opt to apply the terms of the ordinary GNU General Public
-License instead of this License to a given copy of the Library. To do
-this, you must alter all the notices that refer to this License, so
-that they refer to the ordinary GNU General Public License, version 2,
-instead of to this License. (If a newer version than version 2 of the
-ordinary GNU General Public License has appeared, then you can specify
-that version instead if you wish.) Do not make any other change in
-these notices.
-\f
- Once this change is made in a given copy, it is irreversible for
-that copy, so the ordinary GNU General Public License applies to all
-subsequent copies and derivative works made from that copy.
-
- This option is useful when you wish to copy part of the code of
-the Library into a program that is not a library.
-
- 4. You may copy and distribute the Library (or a portion or
-derivative of it, under Section 2) in object code or executable form
-under the terms of Sections 1 and 2 above provided that you accompany
-it with the complete corresponding machine-readable source code, which
-must be distributed under the terms of Sections 1 and 2 above on a
-medium customarily used for software interchange.
-
- If distribution of object code is made by offering access to copy
-from a designated place, then offering equivalent access to copy the
-source code from the same place satisfies the requirement to
-distribute the source code, even though third parties are not
-compelled to copy the source along with the object code.
-
- 5. A program that contains no derivative of any portion of the
-Library, but is designed to work with the Library by being compiled or
-linked with it, is called a "work that uses the Library". Such a
-work, in isolation, is not a derivative work of the Library, and
-therefore falls outside the scope of this License.
-
- However, linking a "work that uses the Library" with the Library
-creates an executable that is a derivative of the Library (because it
-contains portions of the Library), rather than a "work that uses the
-library". The executable is therefore covered by this License.
-Section 6 states terms for distribution of such executables.
-
- When a "work that uses the Library" uses material from a header file
-that is part of the Library, the object code for the work may be a
-derivative work of the Library even though the source code is not.
-Whether this is true is especially significant if the work can be
-linked without the Library, or if the work is itself a library. The
-threshold for this to be true is not precisely defined by law.
-
- If such an object file uses only numerical parameters, data
-structure layouts and accessors, and small macros and small inline
-functions (ten lines or less in length), then the use of the object
-file is unrestricted, regardless of whether it is legally a derivative
-work. (Executables containing this object code plus portions of the
-Library will still fall under Section 6.)
-
- Otherwise, if the work is a derivative of the Library, you may
-distribute the object code for the work under the terms of Section 6.
-Any executables containing that work also fall under Section 6,
-whether or not they are linked directly with the Library itself.
-\f
- 6. As an exception to the Sections above, you may also combine or
-link a "work that uses the Library" with the Library to produce a
-work containing portions of the Library, and distribute that work
-under terms of your choice, provided that the terms permit
-modification of the work for the customer's own use and reverse
-engineering for debugging such modifications.
-
- You must give prominent notice with each copy of the work that the
-Library is used in it and that the Library and its use are covered by
-this License. You must supply a copy of this License. If the work
-during execution displays copyright notices, you must include the
-copyright notice for the Library among them, as well as a reference
-directing the user to the copy of this License. Also, you must do one
-of these things:
-
- a) Accompany the work with the complete corresponding
- machine-readable source code for the Library including whatever
- changes were used in the work (which must be distributed under
- Sections 1 and 2 above); and, if the work is an executable linked
- with the Library, with the complete machine-readable "work that
- uses the Library", as object code and/or source code, so that the
- user can modify the Library and then relink to produce a modified
- executable containing the modified Library. (It is understood
- that the user who changes the contents of definitions files in the
- Library will not necessarily be able to recompile the application
- to use the modified definitions.)
-
- b) Use a suitable shared library mechanism for linking with the
- Library. A suitable mechanism is one that (1) uses at run time a
- copy of the library already present on the user's computer system,
- rather than copying library functions into the executable, and (2)
- will operate properly with a modified version of the library, if
- the user installs one, as long as the modified version is
- interface-compatible with the version that the work was made with.
-
- c) Accompany the work with a written offer, valid for at least
- three years, to give the same user the materials specified in
- Subsection 6a, above, for a charge no more than the cost of
- performing this distribution.
-
- d) If distribution of the work is made by offering access to copy
- from a designated place, offer equivalent access to copy the above
- specified materials from the same place.
-
- e) Verify that the user has already received a copy of these
- materials or that you have already sent this user a copy.
-
- For an executable, the required form of the "work that uses the
-Library" must include any data and utility programs needed for
-reproducing the executable from it. However, as a special exception,
-the materials to be distributed need not include anything that is
-normally distributed (in either source or binary form) with the major
-components (compiler, kernel, and so on) of the operating system on
-which the executable runs, unless that component itself accompanies
-the executable.
-
- It may happen that this requirement contradicts the license
-restrictions of other proprietary libraries that do not normally
-accompany the operating system. Such a contradiction means you cannot
-use both them and the Library together in an executable that you
-distribute.
-\f
- 7. You may place library facilities that are a work based on the
-Library side-by-side in a single library together with other library
-facilities not covered by this License, and distribute such a combined
-library, provided that the separate distribution of the work based on
-the Library and of the other library facilities is otherwise
-permitted, and provided that you do these two things:
-
- a) Accompany the combined library with a copy of the same work
- based on the Library, uncombined with any other library
- facilities. This must be distributed under the terms of the
- Sections above.
-
- b) Give prominent notice with the combined library of the fact
- that part of it is a work based on the Library, and explaining
- where to find the accompanying uncombined form of the same work.
-
- 8. You may not copy, modify, sublicense, link with, or distribute
-the Library except as expressly provided under this License. Any
-attempt otherwise to copy, modify, sublicense, link with, or
-distribute the Library is void, and will automatically terminate your
-rights under this License. However, parties who have received copies,
-or rights, from you under this License will not have their licenses
-terminated so long as such parties remain in full compliance.
-
- 9. You are not required to accept this License, since you have not
-signed it. However, nothing else grants you permission to modify or
-distribute the Library or its derivative works. These actions are
-prohibited by law if you do not accept this License. Therefore, by
-modifying or distributing the Library (or any work based on the
-Library), you indicate your acceptance of this License to do so, and
-all its terms and conditions for copying, distributing or modifying
-the Library or works based on it.
-
- 10. Each time you redistribute the Library (or any work based on the
-Library), the recipient automatically receives a license from the
-original licensor to copy, distribute, link with or modify the Library
-subject to these terms and conditions. You may not impose any further
-restrictions on the recipients' exercise of the rights granted herein.
-You are not responsible for enforcing compliance by third parties with
-this License.
-\f
- 11. If, as a consequence of a court judgment or allegation of patent
-infringement or for any other reason (not limited to patent issues),
-conditions are imposed on you (whether by court order, agreement or
-otherwise) that contradict the conditions of this License, they do not
-excuse you from the conditions of this License. If you cannot
-distribute so as to satisfy simultaneously your obligations under this
-License and any other pertinent obligations, then as a consequence you
-may not distribute the Library at all. For example, if a patent
-license would not permit royalty-free redistribution of the Library by
-all those who receive copies directly or indirectly through you, then
-the only way you could satisfy both it and this License would be to
-refrain entirely from distribution of the Library.
-
-If any portion of this section is held invalid or unenforceable under
-any particular circumstance, the balance of the section is intended to
-apply, and the section as a whole is intended to apply in other
-circumstances.
-
-It is not the purpose of this section to induce you to infringe any
-patents or other property right claims or to contest validity of any
-such claims; this section has the sole purpose of protecting the
-integrity of the free software distribution system which is
-implemented by public license practices. Many people have made
-generous contributions to the wide range of software distributed
-through that system in reliance on consistent application of that
-system; it is up to the author/donor to decide if he or she is willing
-to distribute software through any other system and a licensee cannot
-impose that choice.
-
-This section is intended to make thoroughly clear what is believed to
-be a consequence of the rest of this License.
-
- 12. If the distribution and/or use of the Library is restricted in
-certain countries either by patents or by copyrighted interfaces, the
-original copyright holder who places the Library under this License
-may add an explicit geographical distribution limitation excluding those
-countries, so that distribution is permitted only in or among
-countries not thus excluded. In such case, this License incorporates
-the limitation as if written in the body of this License.
-
- 13. The Free Software Foundation may publish revised and/or new
-versions of the Lesser General Public License from time to time.
-Such new versions will be similar in spirit to the present version,
-but may differ in detail to address new problems or concerns.
-
-Each version is given a distinguishing version number. If the Library
-specifies a version number of this License which applies to it and
-"any later version", you have the option of following the terms and
-conditions either of that version or of any later version published by
-the Free Software Foundation. If the Library does not specify a
-license version number, you may choose any version ever published by
-the Free Software Foundation.
-\f
- 14. If you wish to incorporate parts of the Library into other free
-programs whose distribution conditions are incompatible with these,
-write to the author to ask for permission. For software which is
-copyrighted by the Free Software Foundation, write to the Free
-Software Foundation; we sometimes make exceptions for this. Our
-decision will be guided by the two goals of preserving the free status
-of all derivatives of our free software and of promoting the sharing
-and reuse of software generally.
-
- NO WARRANTY
-
- 15. BECAUSE THE LIBRARY IS LICENSED FREE OF CHARGE, THERE IS NO
-WARRANTY FOR THE LIBRARY, TO THE EXTENT PERMITTED BY APPLICABLE LAW.
-EXCEPT WHEN OTHERWISE STATED IN WRITING THE COPYRIGHT HOLDERS AND/OR
-OTHER PARTIES PROVIDE THE LIBRARY "AS IS" WITHOUT WARRANTY OF ANY
-KIND, EITHER EXPRESSED OR IMPLIED, INCLUDING, BUT NOT LIMITED TO, THE
-IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
-PURPOSE. THE ENTIRE RISK AS TO THE QUALITY AND PERFORMANCE OF THE
-LIBRARY IS WITH YOU. SHOULD THE LIBRARY PROVE DEFECTIVE, YOU ASSUME
-THE COST OF ALL NECESSARY SERVICING, REPAIR OR CORRECTION.
-
- 16. IN NO EVENT UNLESS REQUIRED BY APPLICABLE LAW OR AGREED TO IN
-WRITING WILL ANY COPYRIGHT HOLDER, OR ANY OTHER PARTY WHO MAY MODIFY
-AND/OR REDISTRIBUTE THE LIBRARY AS PERMITTED ABOVE, BE LIABLE TO YOU
-FOR DAMAGES, INCLUDING ANY GENERAL, SPECIAL, INCIDENTAL OR
-CONSEQUENTIAL DAMAGES ARISING OUT OF THE USE OR INABILITY TO USE THE
-LIBRARY (INCLUDING BUT NOT LIMITED TO LOSS OF DATA OR DATA BEING
-RENDERED INACCURATE OR LOSSES SUSTAINED BY YOU OR THIRD PARTIES OR A
-FAILURE OF THE LIBRARY TO OPERATE WITH ANY OTHER SOFTWARE), EVEN IF
-SUCH HOLDER OR OTHER PARTY HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH
-DAMAGES.
-
- END OF TERMS AND CONDITIONS
-\f
- How to Apply These Terms to Your New Libraries
-
- If you develop a new library, and you want it to be of the greatest
-possible use to the public, we recommend making it free software that
-everyone can redistribute and change. You can do so by permitting
-redistribution under these terms (or, alternatively, under the terms
-of the ordinary General Public License).
-
- To apply these terms, attach the following notices to the library.
-It is safest to attach them to the start of each source file to most
-effectively convey the exclusion of warranty; and each file should
-have at least the "copyright" line and a pointer to where the full
-notice is found.
-
-
- <one line to give the library's name and a brief idea of what it does.>
- Copyright (C) <year> <name of author>
-
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with this library; If not, see <http://www.gnu.org/licenses/>.
-
-Also add information on how to contact you by electronic and paper mail.
-
-You should also get your employer (if you work as a programmer) or
-your school, if any, to sign a "copyright disclaimer" for the library,
-if necessary. Here is a sample; alter the names:
-
- Yoyodyne, Inc., hereby disclaims all copyright interest in the
- library `Frob' (a library for tweaking knobs) written by James
- Random Hacker.
-
- <signature of Ty Coon>, 1 April 1990
- Ty Coon, President of Vice
-
-That's all there is to it!
-
-
diff --git a/tools/xenstore/Makefile b/tools/xenstore/Makefile
index 56723139a1..dc39b6cb31 100644
--- a/tools/xenstore/Makefile
+++ b/tools/xenstore/Makefile
@@ -44,7 +44,7 @@ xenstored: $(XENSTORED_OBJS-y)
$(CLIENTS): xenstore
ln -f xenstore $@
-xenstore: xenstore_client.o xs_lib.o
+xenstore: xenstore_client.o
$(CC) $(LDFLAGS) $^ $(LDLIBS) -o $@ $(APPEND_LDFLAGS)
xenstore-control: xenstore_control.o
diff --git a/tools/xenstore/Makefile.common b/tools/xenstore/Makefile.common
index b18f95c103..f71c9bfd55 100644
--- a/tools/xenstore/Makefile.common
+++ b/tools/xenstore/Makefile.common
@@ -2,7 +2,7 @@
XENSTORED_OBJS-y := xenstored_core.o xenstored_watch.o xenstored_domain.o
XENSTORED_OBJS-y += xenstored_transaction.o xenstored_control.o
-XENSTORED_OBJS-y += xs_lib.o talloc.o utils.o tdb.o hashtable.o
+XENSTORED_OBJS-y += talloc.o utils.o tdb.o hashtable.o
XENSTORED_OBJS-$(CONFIG_Linux) += xenstored_posix.o
XENSTORED_OBJS-$(CONFIG_NetBSD) += xenstored_posix.o
diff --git a/tools/xenstore/xenstore_client.c b/tools/xenstore/xenstore_client.c
index 8ff8abf12a..96f97ac77e 100644
--- a/tools/xenstore/xenstore_client.c
+++ b/tools/xenstore/xenstore_client.c
@@ -23,7 +23,7 @@
#include <sys/ioctl.h>
-#include "xs_lib.h"
+#include <xen-tools/xenstore-common.h>
#define PATH_SEP '/'
#define MAX_PATH_LEN 256
@@ -361,7 +361,7 @@ static void do_ls(struct xs_handle *h, char *path, int cur_depth, int show_perms
for (i = 0; i < nperms; i++) {
if (i)
putchar(',');
- xs_perm_to_string(perms+i, buf, sizeof(buf));
+ xenstore_perm_to_string(perms+i, buf, sizeof(buf));
fputs(buf, stdout);
}
putchar(')');
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 6ef800ff64..0d131e2ebc 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -31,10 +31,10 @@
#include <fcntl.h>
#include <unistd.h>
#include <xenctrl.h>
+#include <xen-tools/xenstore-common.h>
#include "utils.h"
#include "talloc.h"
-#include "xs_lib.h"
#include "xenstored_core.h"
#include "xenstored_control.h"
#include "xenstored_domain.h"
@@ -560,7 +560,8 @@ static FILE *lu_dump_open(const void *ctx)
char *filename;
int fd;
- filename = talloc_asprintf(ctx, "%s/state_dump", xs_daemon_rundir());
+ filename = talloc_asprintf(ctx, "%s/state_dump",
+ xenstore_daemon_rundir());
if (!filename)
return NULL;
@@ -583,7 +584,7 @@ static void lu_get_dump_state(struct lu_dump_state *state)
state->size = 0;
state->filename = talloc_asprintf(NULL, "%s/state_dump",
- xs_daemon_rundir());
+ xenstore_daemon_rundir());
if (!state->filename)
barf("Allocation failure");
@@ -1017,7 +1018,7 @@ int do_control(const void *ctx, struct connection *conn,
if (cmd == ARRAY_SIZE(cmds))
return EINVAL;
- num = xs_count_strings(in->buffer, in->used);
+ num = xenstore_count_strings(in->buffer, in->used);
if (cmds[cmd].max_pars)
num = min(num, cmds[cmd].max_pars);
vec = talloc_array(ctx, char *, num);
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index a78cbf1116..62deee9cb9 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -42,11 +42,11 @@
#include <setjmp.h>
#include <xenevtchn.h>
+#include <xen-tools/xenstore-common.h>
#include "utils.h"
#include "list.h"
#include "talloc.h"
-#include "xs_lib.h"
#include "xenstored_core.h"
#include "xenstored_watch.h"
#include "xenstored_transaction.h"
@@ -1201,7 +1201,8 @@ static char *perms_to_strings(const void *ctx, const struct node_perms *perms,
char buffer[MAX_STRLEN(unsigned int) + 1];
for (*len = 0, i = 0; i < perms->num; i++) {
- if (!xs_perm_to_string(&perms->p[i], buffer, sizeof(buffer)))
+ if (!xenstore_perm_to_string(&perms->p[i], buffer,
+ sizeof(buffer)))
return NULL;
strings = talloc_realloc(ctx, strings, char,
@@ -1279,7 +1280,7 @@ static int send_directory_part(const void *ctx, struct connection *conn,
struct node *node;
char gen[24];
- if (xs_count_strings(in->buffer, in->used) != 2)
+ if (xenstore_count_strings(in->buffer, in->used) != 2)
return EINVAL;
/* First arg is node name. */
@@ -1766,7 +1767,7 @@ static int do_set_perms(const void *ctx, struct connection *conn,
char *name, *permstr;
struct node *node;
- perms.num = xs_count_strings(in->buffer, in->used);
+ perms.num = xenstore_count_strings(in->buffer, in->used);
if (perms.num < 2)
return EINVAL;
@@ -1779,7 +1780,7 @@ static int do_set_perms(const void *ctx, struct connection *conn,
perms.p = talloc_array(ctx, struct xs_permissions, perms.num);
if (!perms.p)
return ENOMEM;
- if (!xs_strings_to_perms(perms.p, perms.num, permstr))
+ if (!xenstore_strings_to_perms(perms.p, perms.num, permstr))
return errno;
if (domain_alloc_permrefs(&perms) < 0)
@@ -2575,7 +2576,7 @@ static void destroy_fds(void)
static void init_sockets(void)
{
struct sockaddr_un addr;
- const char *soc_str = xs_daemon_socket();
+ const char *soc_str = xenstore_daemon_path();
if (!soc_str)
barf_perror("Failed to obtain xs domain socket");
@@ -2890,7 +2891,7 @@ int main(int argc, char *argv[])
/* Make sure xenstored directory exists. */
/* Errors ignored here, will be reported when we open files */
- mkdir(xs_daemon_rundir(), 0755);
+ mkdir(xenstore_daemon_rundir(), 0755);
if (dofork) {
openlog("xenstored", 0, LOG_DAEMON);
diff --git a/tools/xenstore/xs_lib.c b/tools/xenstore/xs_lib.c
deleted file mode 100644
index 826fa7b5a8..0000000000
--- a/tools/xenstore/xs_lib.c
+++ /dev/null
@@ -1,126 +0,0 @@
-/*
- Common routines between Xen store user library and daemon.
- Copyright (C) 2005 Rusty Russell IBM Corporation
-
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with this library; If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#include <assert.h>
-#include <unistd.h>
-#include <stdio.h>
-#include <string.h>
-#include <stdlib.h>
-#include <errno.h>
-#include "xs_lib.h"
-
-/* Common routines for the Xen store daemon and client library. */
-
-const char *xs_daemon_rundir(void)
-{
- char *s = getenv("XENSTORED_RUNDIR");
- return (s ? s : XEN_RUN_STORED);
-}
-
-static const char *xs_daemon_path(void)
-{
- static char buf[PATH_MAX];
- char *s = getenv("XENSTORED_PATH");
- if (s)
- return s;
- if (snprintf(buf, sizeof(buf), "%s/socket",
- xs_daemon_rundir()) >= PATH_MAX)
- return NULL;
- return buf;
-}
-
-const char *xs_daemon_socket(void)
-{
- return xs_daemon_path();
-}
-
-/* Convert strings to permissions. False if a problem. */
-bool xs_strings_to_perms(struct xs_permissions *perms, unsigned int num,
- const char *strings)
-{
- const char *p;
- char *end;
- unsigned int i;
-
- for (p = strings, i = 0; i < num; i++) {
- /* "r", "w", or "b" for both. */
- switch (*p) {
- case 'r':
- perms[i].perms = XS_PERM_READ;
- break;
- case 'w':
- perms[i].perms = XS_PERM_WRITE;
- break;
- case 'b':
- perms[i].perms = XS_PERM_READ|XS_PERM_WRITE;
- break;
- case 'n':
- perms[i].perms = XS_PERM_NONE;
- break;
- default:
- errno = EINVAL;
- return false;
- }
- p++;
- perms[i].id = strtol(p, &end, 0);
- if (*end || !*p) {
- errno = EINVAL;
- return false;
- }
- p = end + 1;
- }
- return true;
-}
-
-/* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
-bool xs_perm_to_string(const struct xs_permissions *perm,
- char *buffer, size_t buf_len)
-{
- switch ((int)perm->perms & ~XS_PERM_IGNORE) {
- case XS_PERM_WRITE:
- *buffer = 'w';
- break;
- case XS_PERM_READ:
- *buffer = 'r';
- break;
- case XS_PERM_READ|XS_PERM_WRITE:
- *buffer = 'b';
- break;
- case XS_PERM_NONE:
- *buffer = 'n';
- break;
- default:
- errno = EINVAL;
- return false;
- }
- snprintf(buffer+1, buf_len-1, "%i", (int)perm->id);
- return true;
-}
-
-/* Given a string and a length, count how many strings (nul terms). */
-unsigned int xs_count_strings(const char *strings, unsigned int len)
-{
- unsigned int num;
- const char *p;
-
- for (p = strings, num = 0; p < strings + len; p++)
- if (*p == '\0')
- num++;
-
- return num;
-}
diff --git a/tools/xenstore/xs_lib.h b/tools/xenstore/xs_lib.h
deleted file mode 100644
index 5b9874d741..0000000000
--- a/tools/xenstore/xs_lib.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/*
- Common routines between Xen store user library and daemon.
- Copyright (C) 2005 Rusty Russell IBM Corporation
-
- This library is free software; you can redistribute it and/or
- modify it under the terms of the GNU Lesser General Public
- License as published by the Free Software Foundation; either
- version 2.1 of the License, or (at your option) any later version.
-
- This library is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
- Lesser General Public License for more details.
-
- You should have received a copy of the GNU Lesser General Public
- License along with this library; If not, see <http://www.gnu.org/licenses/>.
-*/
-
-#ifndef XS_LIB_H
-#define XS_LIB_H
-
-#include "xenstore_lib.h"
-
-/* Convert permissions to a string (up to len MAX_STRLEN(unsigned int)+1). */
-bool xs_perm_to_string(const struct xs_permissions *perm,
- char *buffer, size_t buf_len);
-
-/* Given a string and a length, count how many strings (nul terms). */
-unsigned int xs_count_strings(const char *strings, unsigned int len);
-
-#endif /* XS_LIB_H */
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 13/16] tools/xenstore: replace xs_lib.c with a header
2023-05-30 8:54 ` [PATCH v3 13/16] tools/xenstore: replace xs_lib.c with a header Juergen Gross
@ 2023-06-15 21:03 ` Julien Grall
0 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-15 21:03 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Andrew Cooper, George Dunlap, Jan Beulich, Stefano Stabellini,
Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> Instead of including the same small C source in multiple binaries from
> 2 source directories, use a header file with inline functions as a
> replacement.
>
> As some of the functions are exported by libxenstore, rename the inline
> functions from xs_*() do xenstore_*() and add xs_*() wrappers to
> libxenstore.
>
> With that no sources required to build libxenstore are left in
> tools/xenstore, so the file COPYING can be removed now.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Julien Grall <jgrall@amazon.com>
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 14/16] tools/xenstore: split out environment specific live update code
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (12 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 13/16] tools/xenstore: replace xs_lib.c with a header Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-19 17:55 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 15/16] tools/xenstore: split out rest of live update control code Juergen Gross
` (2 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Instead of using #ifdef in xenstored_control.c split out the code of
environment specific functions (daemon or Mini-OS) to dedicated source
files.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/Makefile.common | 8 +-
tools/xenstore/xenstored_control.c | 253 +--------------------------
tools/xenstore/xenstored_lu.h | 56 ++++++
tools/xenstore/xenstored_lu_daemon.c | 133 ++++++++++++++
tools/xenstore/xenstored_lu_minios.c | 121 +++++++++++++
5 files changed, 317 insertions(+), 254 deletions(-)
create mode 100644 tools/xenstore/xenstored_lu.h
create mode 100644 tools/xenstore/xenstored_lu_daemon.c
create mode 100644 tools/xenstore/xenstored_lu_minios.c
diff --git a/tools/xenstore/Makefile.common b/tools/xenstore/Makefile.common
index f71c9bfd55..c42796fe34 100644
--- a/tools/xenstore/Makefile.common
+++ b/tools/xenstore/Makefile.common
@@ -4,10 +4,10 @@ XENSTORED_OBJS-y := xenstored_core.o xenstored_watch.o xenstored_domain.o
XENSTORED_OBJS-y += xenstored_transaction.o xenstored_control.o
XENSTORED_OBJS-y += talloc.o utils.o tdb.o hashtable.o
-XENSTORED_OBJS-$(CONFIG_Linux) += xenstored_posix.o
-XENSTORED_OBJS-$(CONFIG_NetBSD) += xenstored_posix.o
-XENSTORED_OBJS-$(CONFIG_FreeBSD) += xenstored_posix.o
-XENSTORED_OBJS-$(CONFIG_MiniOS) += xenstored_minios.o
+XENSTORED_OBJS-$(CONFIG_Linux) += xenstored_posix.o xenstored_lu_daemon.o
+XENSTORED_OBJS-$(CONFIG_NetBSD) += xenstored_posix.o xenstored_lu_daemon.o
+XENSTORED_OBJS-$(CONFIG_FreeBSD) += xenstored_posix.o xenstored_lu_daemon.o
+XENSTORED_OBJS-$(CONFIG_MiniOS) += xenstored_minios.o xenstored_lu_minios.o
# Include configure output (config.h)
CFLAGS += -include $(XEN_ROOT)/tools/config.h
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index 0d131e2ebc..b61b41f16c 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -38,63 +38,13 @@
#include "xenstored_core.h"
#include "xenstored_control.h"
#include "xenstored_domain.h"
+#include "xenstored_lu.h"
#include "xenstored_watch.h"
-/* Mini-OS only knows about MAP_ANON. */
-#ifndef MAP_ANONYMOUS
-#define MAP_ANONYMOUS MAP_ANON
-#endif
-
#ifndef NO_LIVE_UPDATE
-struct live_update {
- /* For verification the correct connection is acting. */
- struct connection *conn;
-
- /* Pointer to the command used to request LU */
- struct buffered_data *in;
+struct live_update *lu_status;
-#ifdef __MINIOS__
- void *kernel;
- unsigned int kernel_size;
- unsigned int kernel_off;
-
- void *dump_state;
- unsigned long dump_size;
-#else
- char *filename;
-#endif
-
- char *cmdline;
-
- /* Start parameters. */
- bool force;
- unsigned int timeout;
- time_t started_at;
-};
-
-static struct live_update *lu_status;
-
-struct lu_dump_state {
- void *buf;
- unsigned int size;
-#ifndef __MINIOS__
- int fd;
- char *filename;
-#endif
-};
-
-static int lu_destroy(void *data)
-{
-#ifdef __MINIOS__
- if (lu_status->dump_state)
- munmap(lu_status->dump_state, lu_status->dump_size);
-#endif
- lu_status = NULL;
-
- return 0;
-}
-
-static const char *lu_begin(struct connection *conn)
+const char *lu_begin(struct connection *conn)
{
if (lu_status)
return "live-update session already active.";
@@ -431,203 +381,6 @@ static const char *lu_cmdline(const void *ctx, struct connection *conn,
return NULL;
}
-#ifdef __MINIOS__
-static const char *lu_binary_alloc(const void *ctx, struct connection *conn,
- unsigned long size)
-{
- const char *ret;
-
- syslog(LOG_INFO, "live-update: binary size %lu\n", size);
-
- ret = lu_begin(conn);
- if (ret)
- return ret;
-
- lu_status->kernel = talloc_size(lu_status, size);
- if (!lu_status->kernel)
- return "Allocation failure.";
-
- lu_status->kernel_size = size;
- lu_status->kernel_off = 0;
-
- errno = 0;
- return NULL;
-}
-
-static const char *lu_binary_save(const void *ctx, struct connection *conn,
- unsigned int size, const char *data)
-{
- if (!lu_status || lu_status->conn != conn)
- return "Not in live-update session.";
-
- if (lu_status->kernel_off + size > lu_status->kernel_size)
- return "Too much kernel data.";
-
- memcpy(lu_status->kernel + lu_status->kernel_off, data, size);
- lu_status->kernel_off += size;
-
- errno = 0;
- return NULL;
-}
-
-static const char *lu_arch(const void *ctx, struct connection *conn,
- char **vec, int num)
-{
- if (num == 2 && !strcmp(vec[0], "-b"))
- return lu_binary_alloc(ctx, conn, atol(vec[1]));
- if (num > 2 && !strcmp(vec[0], "-d"))
- return lu_binary_save(ctx, conn, atoi(vec[1]), vec[2]);
-
- errno = EINVAL;
- return NULL;
-}
-
-static FILE *lu_dump_open(const void *ctx)
-{
- lu_status->dump_size = ROUNDUP(talloc_total_size(NULL) * 2,
- XC_PAGE_SHIFT);
- lu_status->dump_state = mmap(NULL, lu_status->dump_size,
- PROT_READ | PROT_WRITE,
- MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
- if (lu_status->dump_state == MAP_FAILED)
- return NULL;
-
- return fmemopen(lu_status->dump_state, lu_status->dump_size, "w");
-}
-
-static void lu_dump_close(FILE *fp)
-{
- size_t size;
-
- size = ftell(fp);
- size = ROUNDUP(size, XC_PAGE_SHIFT);
- munmap(lu_status->dump_state + size, lu_status->dump_size - size);
- lu_status->dump_size = size;
-
- fclose(fp);
-}
-
-static void lu_get_dump_state(struct lu_dump_state *state)
-{
-}
-
-static void lu_close_dump_state(struct lu_dump_state *state)
-{
-}
-
-static char *lu_exec(const void *ctx, int argc, char **argv)
-{
- return "NYI";
-}
-#else
-static const char *lu_binary(const void *ctx, struct connection *conn,
- const char *filename)
-{
- const char *ret;
- struct stat statbuf;
-
- syslog(LOG_INFO, "live-update: binary %s\n", filename);
-
- if (stat(filename, &statbuf))
- return "File not accessible.";
- if (!(statbuf.st_mode & (S_IXOTH | S_IXGRP | S_IXUSR)))
- return "File not executable.";
-
- ret = lu_begin(conn);
- if (ret)
- return ret;
-
- lu_status->filename = talloc_strdup(lu_status, filename);
- if (!lu_status->filename)
- return "Allocation failure.";
-
- errno = 0;
- return NULL;
-}
-
-static const char *lu_arch(const void *ctx, struct connection *conn,
- char **vec, int num)
-{
- if (num == 2 && !strcmp(vec[0], "-f"))
- return lu_binary(ctx, conn, vec[1]);
-
- errno = EINVAL;
- return NULL;
-}
-
-static FILE *lu_dump_open(const void *ctx)
-{
- char *filename;
- int fd;
-
- filename = talloc_asprintf(ctx, "%s/state_dump",
- xenstore_daemon_rundir());
- if (!filename)
- return NULL;
-
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
- if (fd < 0)
- return NULL;
-
- return fdopen(fd, "w");
-}
-
-static void lu_dump_close(FILE *fp)
-{
- fclose(fp);
-}
-
-static void lu_get_dump_state(struct lu_dump_state *state)
-{
- struct stat statbuf;
-
- state->size = 0;
-
- state->filename = talloc_asprintf(NULL, "%s/state_dump",
- xenstore_daemon_rundir());
- if (!state->filename)
- barf("Allocation failure");
-
- state->fd = open(state->filename, O_RDONLY);
- if (state->fd < 0)
- return;
- if (fstat(state->fd, &statbuf) != 0)
- goto out_close;
- state->size = statbuf.st_size;
-
- state->buf = mmap(NULL, state->size, PROT_READ, MAP_PRIVATE,
- state->fd, 0);
- if (state->buf == MAP_FAILED) {
- state->size = 0;
- goto out_close;
- }
-
- return;
-
- out_close:
- close(state->fd);
-}
-
-static void lu_close_dump_state(struct lu_dump_state *state)
-{
- assert(state->filename != NULL);
-
- munmap(state->buf, state->size);
- close(state->fd);
-
- unlink(state->filename);
- talloc_free(state->filename);
-}
-
-static char *lu_exec(const void *ctx, int argc, char **argv)
-{
- argv[0] = lu_status->filename;
- execvp(argv[0], argv);
-
- return "Error activating new binary.";
-}
-#endif
-
static bool lu_check_lu_allowed(void)
{
struct connection *conn;
diff --git a/tools/xenstore/xenstored_lu.h b/tools/xenstore/xenstored_lu.h
new file mode 100644
index 0000000000..d2f8e4e57c
--- /dev/null
+++ b/tools/xenstore/xenstored_lu.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: MIT */
+
+/*
+ * Live Update interfaces for Xen Store Daemon.
+ * Copyright (C) 2022 Juergen Gross, SUSE LLC
+ */
+
+#ifndef NO_LIVE_UPDATE
+struct live_update {
+ /* For verification the correct connection is acting. */
+ struct connection *conn;
+
+ /* Pointer to the command used to request LU */
+ struct buffered_data *in;
+
+#ifdef __MINIOS__
+ void *kernel;
+ unsigned int kernel_size;
+ unsigned int kernel_off;
+
+ void *dump_state;
+ unsigned long dump_size;
+#else
+ char *filename;
+#endif
+
+ char *cmdline;
+
+ /* Start parameters. */
+ bool force;
+ unsigned int timeout;
+ time_t started_at;
+};
+
+struct lu_dump_state {
+ void *buf;
+ unsigned int size;
+#ifndef __MINIOS__
+ int fd;
+ char *filename;
+#endif
+};
+
+extern struct live_update *lu_status;
+
+/* Live update private interfaces. */
+void lu_get_dump_state(struct lu_dump_state *state);
+void lu_close_dump_state(struct lu_dump_state *state);
+FILE *lu_dump_open(const void *ctx);
+void lu_dump_close(FILE *fp);
+char *lu_exec(const void *ctx, int argc, char **argv);
+const char *lu_arch(const void *ctx, struct connection *conn, char **vec,
+ int num);
+const char *lu_begin(struct connection *conn);
+int lu_destroy(void *data);
+#endif
diff --git a/tools/xenstore/xenstored_lu_daemon.c b/tools/xenstore/xenstored_lu_daemon.c
new file mode 100644
index 0000000000..952d9f0b25
--- /dev/null
+++ b/tools/xenstore/xenstored_lu_daemon.c
@@ -0,0 +1,133 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * Live Update for Xen Store Daemon.
+ * Copyright (C) 2022 Juergen Gross, SUSE LLC
+ */
+
+#include <assert.h>
+#include <stdio.h>
+#include <syslog.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <xen-tools/xenstore-common.h>
+
+#include "talloc.h"
+#include "xenstored_core.h"
+#include "xenstored_lu.h"
+
+#ifndef NO_LIVE_UPDATE
+void lu_get_dump_state(struct lu_dump_state *state)
+{
+ struct stat statbuf;
+
+ state->size = 0;
+
+ state->filename = talloc_asprintf(NULL, "%s/state_dump",
+ xenstore_daemon_rundir());
+ if (!state->filename)
+ barf("Allocation failure");
+
+ state->fd = open(state->filename, O_RDONLY);
+ if (state->fd < 0)
+ return;
+ if (fstat(state->fd, &statbuf) != 0)
+ goto out_close;
+ state->size = statbuf.st_size;
+
+ state->buf = mmap(NULL, state->size, PROT_READ, MAP_PRIVATE,
+ state->fd, 0);
+ if (state->buf == MAP_FAILED) {
+ state->size = 0;
+ goto out_close;
+ }
+
+ return;
+
+ out_close:
+ close(state->fd);
+}
+
+void lu_close_dump_state(struct lu_dump_state *state)
+{
+ assert(state->filename != NULL);
+
+ munmap(state->buf, state->size);
+ close(state->fd);
+
+ unlink(state->filename);
+ talloc_free(state->filename);
+}
+
+FILE *lu_dump_open(const void *ctx)
+{
+ char *filename;
+ int fd;
+
+ filename = talloc_asprintf(ctx, "%s/state_dump",
+ xenstore_daemon_rundir());
+ if (!filename)
+ return NULL;
+
+ fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
+ if (fd < 0)
+ return NULL;
+
+ return fdopen(fd, "w");
+}
+
+void lu_dump_close(FILE *fp)
+{
+ fclose(fp);
+}
+
+char *lu_exec(const void *ctx, int argc, char **argv)
+{
+ argv[0] = lu_status->filename;
+ execvp(argv[0], argv);
+
+ return "Error activating new binary.";
+}
+
+int lu_destroy(void *data)
+{
+ lu_status = NULL;
+
+ return 0;
+}
+
+static const char *lu_binary(const void *ctx, struct connection *conn,
+ const char *filename)
+{
+ const char *ret;
+ struct stat statbuf;
+
+ syslog(LOG_INFO, "live-update: binary %s\n", filename);
+
+ if (stat(filename, &statbuf))
+ return "File not accessible.";
+ if (!(statbuf.st_mode & (S_IXOTH | S_IXGRP | S_IXUSR)))
+ return "File not executable.";
+
+ ret = lu_begin(conn);
+ if (ret)
+ return ret;
+
+ lu_status->filename = talloc_strdup(lu_status, filename);
+ if (!lu_status->filename)
+ return "Allocation failure.";
+
+ errno = 0;
+ return NULL;
+}
+
+const char *lu_arch(const void *ctx, struct connection *conn, char **vec,
+ int num)
+{
+ if (num == 2 && !strcmp(vec[0], "-f"))
+ return lu_binary(ctx, conn, vec[1]);
+
+ errno = EINVAL;
+ return NULL;
+}
+#endif
diff --git a/tools/xenstore/xenstored_lu_minios.c b/tools/xenstore/xenstored_lu_minios.c
new file mode 100644
index 0000000000..0bec8a0037
--- /dev/null
+++ b/tools/xenstore/xenstored_lu_minios.c
@@ -0,0 +1,121 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * Live Update for Xen Store Daemon.
+ * Copyright (C) 2022 Juergen Gross, SUSE LLC
+ */
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <syslog.h>
+#include <sys/mman.h>
+#include <xenctrl.h>
+#include <xen-tools/common-macros.h>
+
+#include "talloc.h"
+#include "xenstored_lu.h"
+
+/* Mini-OS only knows about MAP_ANON. */
+#ifndef MAP_ANONYMOUS
+#define MAP_ANONYMOUS MAP_ANON
+#endif
+
+#ifndef NO_LIVE_UPDATE
+void lu_get_dump_state(struct lu_dump_state *state)
+{
+}
+
+void lu_close_dump_state(struct lu_dump_state *state)
+{
+}
+
+FILE *lu_dump_open(const void *ctx)
+{
+ lu_status->dump_size = ROUNDUP(talloc_total_size(NULL) * 2,
+ XC_PAGE_SHIFT);
+ lu_status->dump_state = mmap(NULL, lu_status->dump_size,
+ PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ if (lu_status->dump_state == MAP_FAILED)
+ return NULL;
+
+ return fmemopen(lu_status->dump_state, lu_status->dump_size, "w");
+}
+
+void lu_dump_close(FILE *fp)
+{
+ size_t size;
+
+ size = ftell(fp);
+ size = ROUNDUP(size, XC_PAGE_SHIFT);
+ munmap(lu_status->dump_state + size, lu_status->dump_size - size);
+ lu_status->dump_size = size;
+
+ fclose(fp);
+}
+
+char *lu_exec(const void *ctx, int argc, char **argv)
+{
+ return "NYI";
+}
+
+int lu_destroy(void *data)
+{
+ if (lu_status->dump_state)
+ munmap(lu_status->dump_state, lu_status->dump_size);
+ lu_status = NULL;
+
+ return 0;
+}
+
+static const char *lu_binary_alloc(const void *ctx, struct connection *conn,
+ unsigned long size)
+{
+ const char *ret;
+
+ syslog(LOG_INFO, "live-update: binary size %lu\n", size);
+
+ ret = lu_begin(conn);
+ if (ret)
+ return ret;
+
+ lu_status->kernel = talloc_size(lu_status, size);
+ if (!lu_status->kernel)
+ return "Allocation failure.";
+
+ lu_status->kernel_size = size;
+ lu_status->kernel_off = 0;
+
+ errno = 0;
+ return NULL;
+}
+
+static const char *lu_binary_save(const void *ctx, struct connection *conn,
+ unsigned int size, const char *data)
+{
+ if (!lu_status || lu_status->conn != conn)
+ return "Not in live-update session.";
+
+ if (lu_status->kernel_off + size > lu_status->kernel_size)
+ return "Too much kernel data.";
+
+ memcpy(lu_status->kernel + lu_status->kernel_off, data, size);
+ lu_status->kernel_off += size;
+
+ errno = 0;
+ return NULL;
+}
+
+const char *lu_arch(const void *ctx, struct connection *conn, char **vec,
+ int num)
+{
+ if (num == 2 && !strcmp(vec[0], "-b"))
+ return lu_binary_alloc(ctx, conn, atol(vec[1]));
+ if (num > 2 && !strcmp(vec[0], "-d"))
+ return lu_binary_save(ctx, conn, atoi(vec[1]), vec[2]);
+
+ errno = EINVAL;
+ return NULL;
+}
+#endif
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 14/16] tools/xenstore: split out environment specific live update code
2023-05-30 8:54 ` [PATCH v3 14/16] tools/xenstore: split out environment specific live update code Juergen Gross
@ 2023-06-19 17:55 ` Julien Grall
2023-06-20 8:22 ` Juergen Gross
0 siblings, 1 reply; 35+ messages in thread
From: Julien Grall @ 2023-06-19 17:55 UTC (permalink / raw)
To: Juergen Gross, xen-devel; +Cc: Wei Liu, Anthony PERARD
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> -static struct live_update *lu_status;
> -
> -struct lu_dump_state {
> - void *buf;
> - unsigned int size;
> -#ifndef __MINIOS__
> - int fd;
> - char *filename;
> -#endif
> -};
> -
> -static int lu_destroy(void *data)
> -{
> -#ifdef __MINIOS__
> - if (lu_status->dump_state)
> - munmap(lu_status->dump_state, lu_status->dump_size);
> -#endif
> - lu_status = NULL;
> -
> - return 0;
> -}
I think moving all lu_destroy() out of xenstored_control.c is a mistake
because we now need to remember that any common change in lu_begin() may
need an update in the two implementation of lu_destroy().
Even if this seems pointless for a few lines, it would be best to split
lu_destroy() in two parts: one common and one per lu backend.
The rest of the changes look good to me.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [PATCH v3 14/16] tools/xenstore: split out environment specific live update code
2023-06-19 17:55 ` Julien Grall
@ 2023-06-20 8:22 ` Juergen Gross
0 siblings, 0 replies; 35+ messages in thread
From: Juergen Gross @ 2023-06-20 8:22 UTC (permalink / raw)
To: Julien Grall, xen-devel; +Cc: Wei Liu, Anthony PERARD
[-- Attachment #1.1.1: Type: text/plain, Size: 1049 bytes --]
On 19.06.23 19:55, Julien Grall wrote:
> Hi Juergen,
>
> On 30/05/2023 09:54, Juergen Gross wrote:
>> -static struct live_update *lu_status;
>> -
>> -struct lu_dump_state {
>> - void *buf;
>> - unsigned int size;
>> -#ifndef __MINIOS__
>> - int fd;
>> - char *filename;
>> -#endif
>> -};
>> -
>> -static int lu_destroy(void *data)
>> -{
>> -#ifdef __MINIOS__
>> - if (lu_status->dump_state)
>> - munmap(lu_status->dump_state, lu_status->dump_size);
>> -#endif
>> - lu_status = NULL;
>> -
>> - return 0;
>> -}
>
> I think moving all lu_destroy() out of xenstored_control.c is a mistake because
> we now need to remember that any common change in lu_begin() may need an update
> in the two implementation of lu_destroy().
>
> Even if this seems pointless for a few lines, it would be best to split
> lu_destroy() in two parts: one common and one per lu backend.
Okay, fine with me.
>
> The rest of the changes look good to me.
Thanks,
Juergen
[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH v3 15/16] tools/xenstore: split out rest of live update control code
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (13 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 14/16] tools/xenstore: split out environment specific live update code Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-19 18:03 ` Julien Grall
2023-05-30 8:54 ` [PATCH v3 16/16] tools/xenstore: remove unused stuff from list.h Juergen Gross
2023-06-09 18:46 ` [PATCH v3 00/16] tools/xenstore: more cleanups Julien Grall
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Move the rest of live update related code from xenstored_control.c to
a dedicated new source file.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/Makefile.common | 2 +-
tools/xenstore/xenstored_control.c | 409 -----------------------------
tools/xenstore/xenstored_control.h | 8 -
tools/xenstore/xenstored_core.c | 1 +
tools/xenstore/xenstored_lu.c | 400 ++++++++++++++++++++++++++++
tools/xenstore/xenstored_lu.h | 25 ++
6 files changed, 427 insertions(+), 418 deletions(-)
create mode 100644 tools/xenstore/xenstored_lu.c
diff --git a/tools/xenstore/Makefile.common b/tools/xenstore/Makefile.common
index c42796fe34..657a16849e 100644
--- a/tools/xenstore/Makefile.common
+++ b/tools/xenstore/Makefile.common
@@ -1,7 +1,7 @@
# Makefile shared with stubdom
XENSTORED_OBJS-y := xenstored_core.o xenstored_watch.o xenstored_domain.o
-XENSTORED_OBJS-y += xenstored_transaction.o xenstored_control.o
+XENSTORED_OBJS-y += xenstored_transaction.o xenstored_control.o xenstored_lu.o
XENSTORED_OBJS-y += talloc.o utils.o tdb.o hashtable.o
XENSTORED_OBJS-$(CONFIG_Linux) += xenstored_posix.o xenstored_lu_daemon.o
diff --git a/tools/xenstore/xenstored_control.c b/tools/xenstore/xenstored_control.c
index b61b41f16c..145a0e5aff 100644
--- a/tools/xenstore/xenstored_control.c
+++ b/tools/xenstore/xenstored_control.c
@@ -16,21 +16,13 @@
along with this program; If not, see <http://www.gnu.org/licenses/>.
*/
-#include <assert.h>
-#include <ctype.h>
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
-#include <syslog.h>
-#include <time.h>
#include <sys/types.h>
-#include <sys/stat.h>
-#include <sys/mman.h>
-#include <fcntl.h>
#include <unistd.h>
-#include <xenctrl.h>
#include <xen-tools/xenstore-common.h>
#include "utils.h"
@@ -39,69 +31,6 @@
#include "xenstored_control.h"
#include "xenstored_domain.h"
#include "xenstored_lu.h"
-#include "xenstored_watch.h"
-
-#ifndef NO_LIVE_UPDATE
-struct live_update *lu_status;
-
-const char *lu_begin(struct connection *conn)
-{
- if (lu_status)
- return "live-update session already active.";
-
- lu_status = talloc_zero(conn, struct live_update);
- if (!lu_status)
- return "Allocation failure.";
- lu_status->conn = conn;
- talloc_set_destructor(lu_status, lu_destroy);
-
- return NULL;
-}
-
-struct connection *lu_get_connection(void)
-{
- return lu_status ? lu_status->conn : NULL;
-}
-
-unsigned int lu_write_response(FILE *fp)
-{
- struct xsd_sockmsg msg;
-
- assert(lu_status);
-
- msg = lu_status->in->hdr.msg;
-
- msg.len = sizeof("OK");
- if (fp && fwrite(&msg, sizeof(msg), 1, fp) != 1)
- return 0;
- if (fp && fwrite("OK", msg.len, 1, fp) != 1)
- return 0;
-
- return sizeof(msg) + msg.len;
-}
-
-bool lu_is_pending(void)
-{
- return lu_status != NULL;
-}
-
-#else
-struct connection *lu_get_connection(void)
-{
- return NULL;
-}
-
-unsigned int lu_write_response(FILE *fp)
-{
- /* Unsupported */
- return 0;
-}
-
-bool lu_is_pending(void)
-{
- return false;
-}
-#endif
struct cmd_s {
char *cmd;
@@ -352,344 +281,6 @@ static int do_control_print(const void *ctx, struct connection *conn,
return 0;
}
-#ifndef NO_LIVE_UPDATE
-static const char *lu_abort(const void *ctx, struct connection *conn)
-{
- syslog(LOG_INFO, "live-update: abort\n");
-
- if (!lu_status)
- return "No live-update session active.";
-
- /* Destructor will do the real abort handling. */
- talloc_free(lu_status);
-
- return NULL;
-}
-
-static const char *lu_cmdline(const void *ctx, struct connection *conn,
- const char *cmdline)
-{
- syslog(LOG_INFO, "live-update: cmdline %s\n", cmdline);
-
- if (!lu_status || lu_status->conn != conn)
- return "Not in live-update session.";
-
- lu_status->cmdline = talloc_strdup(lu_status, cmdline);
- if (!lu_status->cmdline)
- return "Allocation failure.";
-
- return NULL;
-}
-
-static bool lu_check_lu_allowed(void)
-{
- struct connection *conn;
- time_t now = time(NULL);
- unsigned int ta_total = 0, ta_long = 0;
-
- list_for_each_entry(conn, &connections, list) {
- if (conn->ta_start_time) {
- ta_total++;
- if (now - conn->ta_start_time >= lu_status->timeout)
- ta_long++;
- }
- }
-
- /*
- * Allow LiveUpdate if one of the following conditions is met:
- * - There is no active transactions
- * - All transactions are long running (e.g. they have been
- * active for more than lu_status->timeout sec) and the admin as
- * requested to force the operation.
- */
- return ta_total ? (lu_status->force && ta_long == ta_total) : true;
-}
-
-static const char *lu_reject_reason(const void *ctx)
-{
- char *ret = NULL;
- struct connection *conn;
- time_t now = time(NULL);
-
- list_for_each_entry(conn, &connections, list) {
- unsigned long tdiff = now - conn->ta_start_time;
-
- if (conn->ta_start_time && (tdiff >= lu_status->timeout)) {
- ret = talloc_asprintf(ctx, "%s\nDomain %u: %ld s",
- ret ? : "Domains with long running transactions:",
- conn->id, tdiff);
- }
- }
-
- return ret ? (const char *)ret : "Overlapping transactions";
-}
-
-static const char *lu_dump_state(const void *ctx, struct connection *conn)
-{
- FILE *fp;
- const char *ret;
- struct xs_state_record_header end;
- struct xs_state_preamble pre;
-
- fp = lu_dump_open(ctx);
- if (!fp)
- return "Dump state open error";
-
- memcpy(pre.ident, XS_STATE_IDENT, sizeof(pre.ident));
- pre.version = htobe32(XS_STATE_VERSION);
- pre.flags = XS_STATE_FLAGS;
- if (fwrite(&pre, sizeof(pre), 1, fp) != 1) {
- ret = "Dump write error";
- goto out;
- }
-
- ret = dump_state_global(fp);
- if (ret)
- goto out;
- ret = dump_state_connections(fp);
- if (ret)
- goto out;
- ret = dump_state_nodes(fp, ctx);
- if (ret)
- goto out;
-
- end.type = XS_STATE_TYPE_END;
- end.length = 0;
- if (fwrite(&end, sizeof(end), 1, fp) != 1)
- ret = "Dump write error";
-
- out:
- lu_dump_close(fp);
-
- return ret;
-}
-
-void lu_read_state(void)
-{
- struct lu_dump_state state = {};
- struct xs_state_record_header *head;
- void *ctx = talloc_new(NULL); /* Work context for subfunctions. */
- struct xs_state_preamble *pre;
-
- syslog(LOG_INFO, "live-update: read state\n");
- lu_get_dump_state(&state);
- if (state.size == 0)
- barf_perror("No state found after live-update");
-
- pre = state.buf;
- if (memcmp(pre->ident, XS_STATE_IDENT, sizeof(pre->ident)) ||
- pre->version != htobe32(XS_STATE_VERSION) ||
- pre->flags != XS_STATE_FLAGS)
- barf("Unknown record identifier");
- for (head = state.buf + sizeof(*pre);
- head->type != XS_STATE_TYPE_END &&
- (void *)head - state.buf < state.size;
- head = (void *)head + sizeof(*head) + head->length) {
- switch (head->type) {
- case XS_STATE_TYPE_GLOBAL:
- read_state_global(ctx, head + 1);
- break;
- case XS_STATE_TYPE_CONN:
- read_state_connection(ctx, head + 1);
- break;
- case XS_STATE_TYPE_WATCH:
- read_state_watch(ctx, head + 1);
- break;
- case XS_STATE_TYPE_TA:
- xprintf("live-update: ignore transaction record\n");
- break;
- case XS_STATE_TYPE_NODE:
- read_state_node(ctx, head + 1);
- break;
- default:
- xprintf("live-update: unknown state record %08x\n",
- head->type);
- break;
- }
- }
-
- lu_close_dump_state(&state);
-
- talloc_free(ctx);
-
- /*
- * We may have missed the VIRQ_DOM_EXC notification and a domain may
- * have died while we were live-updating. So check all the domains are
- * still alive.
- */
- check_domains();
-}
-
-static const char *lu_activate_binary(const void *ctx)
-{
- int argc;
- char **argv;
- unsigned int i;
-
- if (lu_status->cmdline) {
- argc = 4; /* At least one arg + progname + "-U" + NULL. */
- for (i = 0; lu_status->cmdline[i]; i++)
- if (isspace(lu_status->cmdline[i]))
- argc++;
- argv = talloc_array(ctx, char *, argc);
- if (!argv)
- return "Allocation failure.";
-
- i = 0;
- argc = 1;
- argv[1] = strtok(lu_status->cmdline, " \t");
- while (argv[argc]) {
- if (!strcmp(argv[argc], "-U"))
- i = 1;
- argc++;
- argv[argc] = strtok(NULL, " \t");
- }
-
- if (!i) {
- argv[argc++] = "-U";
- argv[argc] = NULL;
- }
- } else {
- for (i = 0; i < orig_argc; i++)
- if (!strcmp(orig_argv[i], "-U"))
- break;
-
- argc = orig_argc;
- argv = talloc_array(ctx, char *, orig_argc + 2);
- if (!argv)
- return "Allocation failure.";
-
- memcpy(argv, orig_argv, orig_argc * sizeof(*argv));
- if (i == orig_argc)
- argv[argc++] = "-U";
- argv[argc] = NULL;
- }
-
- domain_deinit();
-
- return lu_exec(ctx, argc, argv);
-}
-
-static bool do_lu_start(struct delayed_request *req)
-{
- time_t now = time(NULL);
- const char *ret;
- struct buffered_data *saved_in;
- struct connection *conn = req->data;
-
- /*
- * Cancellation may have been requested asynchronously. In this
- * case, lu_status will be NULL.
- */
- if (!lu_status) {
- ret = "Cancellation was requested";
- goto out;
- }
-
- assert(lu_status->conn == conn);
-
- if (!lu_check_lu_allowed()) {
- if (now < lu_status->started_at + lu_status->timeout)
- return false;
- if (!lu_status->force) {
- ret = lu_reject_reason(req);
- goto out;
- }
- }
-
- assert(req->in == lu_status->in);
- /* Dump out internal state, including "OK" for live update. */
- ret = lu_dump_state(req->in, conn);
- if (!ret) {
- /* Perform the activation of new binary. */
- ret = lu_activate_binary(req->in);
- }
-
- /* We will reach this point only in case of failure. */
- out:
- /*
- * send_reply() will send the response for conn->in. Save the current
- * conn->in and restore it afterwards.
- */
- saved_in = conn->in;
- conn->in = req->in;
- send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
- conn->in = saved_in;
- talloc_free(lu_status);
-
- return true;
-}
-
-static const char *lu_start(const void *ctx, struct connection *conn,
- bool force, unsigned int to)
-{
- syslog(LOG_INFO, "live-update: start, force=%d, to=%u\n", force, to);
-
- if (!lu_status || lu_status->conn != conn)
- return "Not in live-update session.";
-
-#ifdef __MINIOS__
- if (lu_status->kernel_size != lu_status->kernel_off)
- return "Kernel not complete.";
-#endif
-
- lu_status->force = force;
- lu_status->timeout = to;
- lu_status->started_at = time(NULL);
- lu_status->in = conn->in;
-
- errno = delay_request(conn, conn->in, do_lu_start, conn, false);
-
- return NULL;
-}
-
-static int do_control_lu(const void *ctx, struct connection *conn,
- char **vec, int num)
-{
- const char *ret = NULL;
- unsigned int i;
- bool force = false;
- unsigned int to = 0;
-
- if (num < 1)
- return EINVAL;
-
- if (!strcmp(vec[0], "-a")) {
- if (num == 1)
- ret = lu_abort(ctx, conn);
- else
- return EINVAL;
- } else if (!strcmp(vec[0], "-c")) {
- if (num == 2)
- ret = lu_cmdline(ctx, conn, vec[1]);
- else
- return EINVAL;
- } else if (!strcmp(vec[0], "-s")) {
- for (i = 1; i < num; i++) {
- if (!strcmp(vec[i], "-F"))
- force = true;
- else if (!strcmp(vec[i], "-t") && i < num - 1) {
- i++;
- to = atoi(vec[i]);
- } else
- return EINVAL;
- }
- ret = lu_start(ctx, conn, force, to);
- if (!ret)
- return errno;
- } else {
- ret = lu_arch(ctx, conn, vec, num);
- if (!ret && errno)
- return errno;
- }
-
- if (!ret)
- ret = "OK";
- send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
- return 0;
-}
-#endif
-
static int do_control_help(const void *, struct connection *, char **, int);
static struct cmd_s cmds[] = {
diff --git a/tools/xenstore/xenstored_control.h b/tools/xenstore/xenstored_control.h
index a8cb76559b..faa955968d 100644
--- a/tools/xenstore/xenstored_control.h
+++ b/tools/xenstore/xenstored_control.h
@@ -18,11 +18,3 @@
int do_control(const void *ctx, struct connection *conn,
struct buffered_data *in);
-void lu_read_state(void);
-
-struct connection *lu_get_connection(void);
-
-/* Write the "OK" response for the live-update command */
-unsigned int lu_write_response(FILE *fp);
-
-bool lu_is_pending(void);
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 62deee9cb9..31a862b715 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -52,6 +52,7 @@
#include "xenstored_transaction.h"
#include "xenstored_domain.h"
#include "xenstored_control.h"
+#include "xenstored_lu.h"
#include "tdb.h"
#ifndef NO_SOCKETS
diff --git a/tools/xenstore/xenstored_lu.c b/tools/xenstore/xenstored_lu.c
new file mode 100644
index 0000000000..783bfd6456
--- /dev/null
+++ b/tools/xenstore/xenstored_lu.c
@@ -0,0 +1,400 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * Live Update interfaces for Xen Store Daemon.
+ * Copyright (C) 2022 Juergen Gross, SUSE LLC
+ */
+
+#include <assert.h>
+#include <ctype.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <syslog.h>
+#include <time.h>
+
+#include "talloc.h"
+#include "xenstored_core.h"
+#include "xenstored_domain.h"
+#include "xenstored_lu.h"
+#include "xenstored_watch.h"
+
+#ifndef NO_LIVE_UPDATE
+struct live_update *lu_status;
+
+const char *lu_begin(struct connection *conn)
+{
+ if (lu_status)
+ return "live-update session already active.";
+
+ lu_status = talloc_zero(conn, struct live_update);
+ if (!lu_status)
+ return "Allocation failure.";
+ lu_status->conn = conn;
+ talloc_set_destructor(lu_status, lu_destroy);
+
+ return NULL;
+}
+
+struct connection *lu_get_connection(void)
+{
+ return lu_status ? lu_status->conn : NULL;
+}
+
+unsigned int lu_write_response(FILE *fp)
+{
+ struct xsd_sockmsg msg;
+
+ assert(lu_status);
+
+ msg = lu_status->in->hdr.msg;
+
+ msg.len = sizeof("OK");
+ if (fp && fwrite(&msg, sizeof(msg), 1, fp) != 1)
+ return 0;
+ if (fp && fwrite("OK", msg.len, 1, fp) != 1)
+ return 0;
+
+ return sizeof(msg) + msg.len;
+}
+
+bool lu_is_pending(void)
+{
+ return lu_status != NULL;
+}
+
+void lu_read_state(void)
+{
+ struct lu_dump_state state = {};
+ struct xs_state_record_header *head;
+ void *ctx = talloc_new(NULL); /* Work context for subfunctions. */
+ struct xs_state_preamble *pre;
+
+ syslog(LOG_INFO, "live-update: read state\n");
+ lu_get_dump_state(&state);
+ if (state.size == 0)
+ barf_perror("No state found after live-update");
+
+ pre = state.buf;
+ if (memcmp(pre->ident, XS_STATE_IDENT, sizeof(pre->ident)) ||
+ pre->version != htobe32(XS_STATE_VERSION) ||
+ pre->flags != XS_STATE_FLAGS)
+ barf("Unknown record identifier");
+ for (head = state.buf + sizeof(*pre);
+ head->type != XS_STATE_TYPE_END &&
+ (void *)head - state.buf < state.size;
+ head = (void *)head + sizeof(*head) + head->length) {
+ switch (head->type) {
+ case XS_STATE_TYPE_GLOBAL:
+ read_state_global(ctx, head + 1);
+ break;
+ case XS_STATE_TYPE_CONN:
+ read_state_connection(ctx, head + 1);
+ break;
+ case XS_STATE_TYPE_WATCH:
+ read_state_watch(ctx, head + 1);
+ break;
+ case XS_STATE_TYPE_TA:
+ xprintf("live-update: ignore transaction record\n");
+ break;
+ case XS_STATE_TYPE_NODE:
+ read_state_node(ctx, head + 1);
+ break;
+ default:
+ xprintf("live-update: unknown state record %08x\n",
+ head->type);
+ break;
+ }
+ }
+
+ lu_close_dump_state(&state);
+
+ talloc_free(ctx);
+
+ /*
+ * We may have missed the VIRQ_DOM_EXC notification and a domain may
+ * have died while we were live-updating. So check all the domains are
+ * still alive.
+ */
+ check_domains();
+}
+
+static const char *lu_abort(const void *ctx, struct connection *conn)
+{
+ syslog(LOG_INFO, "live-update: abort\n");
+
+ if (!lu_status)
+ return "No live-update session active.";
+
+ /* Destructor will do the real abort handling. */
+ talloc_free(lu_status);
+
+ return NULL;
+}
+
+static const char *lu_cmdline(const void *ctx, struct connection *conn,
+ const char *cmdline)
+{
+ syslog(LOG_INFO, "live-update: cmdline %s\n", cmdline);
+
+ if (!lu_status || lu_status->conn != conn)
+ return "Not in live-update session.";
+
+ lu_status->cmdline = talloc_strdup(lu_status, cmdline);
+ if (!lu_status->cmdline)
+ return "Allocation failure.";
+
+ return NULL;
+}
+
+static bool lu_check_lu_allowed(void)
+{
+ struct connection *conn;
+ time_t now = time(NULL);
+ unsigned int ta_total = 0, ta_long = 0;
+
+ list_for_each_entry(conn, &connections, list) {
+ if (conn->ta_start_time) {
+ ta_total++;
+ if (now - conn->ta_start_time >= lu_status->timeout)
+ ta_long++;
+ }
+ }
+
+ /*
+ * Allow LiveUpdate if one of the following conditions is met:
+ * - There is no active transactions
+ * - All transactions are long running (e.g. they have been
+ * active for more than lu_status->timeout sec) and the admin as
+ * requested to force the operation.
+ */
+ return ta_total ? (lu_status->force && ta_long == ta_total) : true;
+}
+
+static const char *lu_reject_reason(const void *ctx)
+{
+ char *ret = NULL;
+ struct connection *conn;
+ time_t now = time(NULL);
+
+ list_for_each_entry(conn, &connections, list) {
+ unsigned long tdiff = now - conn->ta_start_time;
+
+ if (conn->ta_start_time && (tdiff >= lu_status->timeout)) {
+ ret = talloc_asprintf(ctx, "%s\nDomain %u: %ld s",
+ ret ? : "Domains with long running transactions:",
+ conn->id, tdiff);
+ }
+ }
+
+ return ret ? (const char *)ret : "Overlapping transactions";
+}
+
+static const char *lu_dump_state(const void *ctx, struct connection *conn)
+{
+ FILE *fp;
+ const char *ret;
+ struct xs_state_record_header end;
+ struct xs_state_preamble pre;
+
+ fp = lu_dump_open(ctx);
+ if (!fp)
+ return "Dump state open error";
+
+ memcpy(pre.ident, XS_STATE_IDENT, sizeof(pre.ident));
+ pre.version = htobe32(XS_STATE_VERSION);
+ pre.flags = XS_STATE_FLAGS;
+ if (fwrite(&pre, sizeof(pre), 1, fp) != 1) {
+ ret = "Dump write error";
+ goto out;
+ }
+
+ ret = dump_state_global(fp);
+ if (ret)
+ goto out;
+ ret = dump_state_connections(fp);
+ if (ret)
+ goto out;
+ ret = dump_state_nodes(fp, ctx);
+ if (ret)
+ goto out;
+
+ end.type = XS_STATE_TYPE_END;
+ end.length = 0;
+ if (fwrite(&end, sizeof(end), 1, fp) != 1)
+ ret = "Dump write error";
+
+ out:
+ lu_dump_close(fp);
+
+ return ret;
+}
+
+static const char *lu_activate_binary(const void *ctx)
+{
+ int argc;
+ char **argv;
+ unsigned int i;
+
+ if (lu_status->cmdline) {
+ argc = 4; /* At least one arg + progname + "-U" + NULL. */
+ for (i = 0; lu_status->cmdline[i]; i++)
+ if (isspace(lu_status->cmdline[i]))
+ argc++;
+ argv = talloc_array(ctx, char *, argc);
+ if (!argv)
+ return "Allocation failure.";
+
+ i = 0;
+ argc = 1;
+ argv[1] = strtok(lu_status->cmdline, " \t");
+ while (argv[argc]) {
+ if (!strcmp(argv[argc], "-U"))
+ i = 1;
+ argc++;
+ argv[argc] = strtok(NULL, " \t");
+ }
+
+ if (!i) {
+ argv[argc++] = "-U";
+ argv[argc] = NULL;
+ }
+ } else {
+ for (i = 0; i < orig_argc; i++)
+ if (!strcmp(orig_argv[i], "-U"))
+ break;
+
+ argc = orig_argc;
+ argv = talloc_array(ctx, char *, orig_argc + 2);
+ if (!argv)
+ return "Allocation failure.";
+
+ memcpy(argv, orig_argv, orig_argc * sizeof(*argv));
+ if (i == orig_argc)
+ argv[argc++] = "-U";
+ argv[argc] = NULL;
+ }
+
+ domain_deinit();
+
+ return lu_exec(ctx, argc, argv);
+}
+
+static bool do_lu_start(struct delayed_request *req)
+{
+ time_t now = time(NULL);
+ const char *ret;
+ struct buffered_data *saved_in;
+ struct connection *conn = req->data;
+
+ /*
+ * Cancellation may have been requested asynchronously. In this
+ * case, lu_status will be NULL.
+ */
+ if (!lu_status) {
+ ret = "Cancellation was requested";
+ goto out;
+ }
+
+ assert(lu_status->conn == conn);
+
+ if (!lu_check_lu_allowed()) {
+ if (now < lu_status->started_at + lu_status->timeout)
+ return false;
+ if (!lu_status->force) {
+ ret = lu_reject_reason(req);
+ goto out;
+ }
+ }
+
+ assert(req->in == lu_status->in);
+ /* Dump out internal state, including "OK" for live update. */
+ ret = lu_dump_state(req->in, conn);
+ if (!ret) {
+ /* Perform the activation of new binary. */
+ ret = lu_activate_binary(req->in);
+ }
+
+ /* We will reach this point only in case of failure. */
+ out:
+ /*
+ * send_reply() will send the response for conn->in. Save the current
+ * conn->in and restore it afterwards.
+ */
+ saved_in = conn->in;
+ conn->in = req->in;
+ send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
+ conn->in = saved_in;
+ talloc_free(lu_status);
+
+ return true;
+}
+
+static const char *lu_start(const void *ctx, struct connection *conn,
+ bool force, unsigned int to)
+{
+ syslog(LOG_INFO, "live-update: start, force=%d, to=%u\n", force, to);
+
+ if (!lu_status || lu_status->conn != conn)
+ return "Not in live-update session.";
+
+#ifdef __MINIOS__
+ if (lu_status->kernel_size != lu_status->kernel_off)
+ return "Kernel not complete.";
+#endif
+
+ lu_status->force = force;
+ lu_status->timeout = to;
+ lu_status->started_at = time(NULL);
+ lu_status->in = conn->in;
+
+ errno = delay_request(conn, conn->in, do_lu_start, conn, false);
+
+ return NULL;
+}
+
+int do_control_lu(const void *ctx, struct connection *conn, char **vec,
+ int num)
+{
+ const char *ret = NULL;
+ unsigned int i;
+ bool force = false;
+ unsigned int to = 0;
+
+ if (num < 1)
+ return EINVAL;
+
+ if (!strcmp(vec[0], "-a")) {
+ if (num == 1)
+ ret = lu_abort(ctx, conn);
+ else
+ return EINVAL;
+ } else if (!strcmp(vec[0], "-c")) {
+ if (num == 2)
+ ret = lu_cmdline(ctx, conn, vec[1]);
+ else
+ return EINVAL;
+ } else if (!strcmp(vec[0], "-s")) {
+ for (i = 1; i < num; i++) {
+ if (!strcmp(vec[i], "-F"))
+ force = true;
+ else if (!strcmp(vec[i], "-t") && i < num - 1) {
+ i++;
+ to = atoi(vec[i]);
+ } else
+ return EINVAL;
+ }
+ ret = lu_start(ctx, conn, force, to);
+ if (!ret)
+ return errno;
+ } else {
+ ret = lu_arch(ctx, conn, vec, num);
+ if (!ret && errno)
+ return errno;
+ }
+
+ if (!ret)
+ ret = "OK";
+ send_reply(conn, XS_CONTROL, ret, strlen(ret) + 1);
+ return 0;
+}
+#endif
diff --git a/tools/xenstore/xenstored_lu.h b/tools/xenstore/xenstored_lu.h
index d2f8e4e57c..7aa524a07e 100644
--- a/tools/xenstore/xenstored_lu.h
+++ b/tools/xenstore/xenstored_lu.h
@@ -43,6 +43,16 @@ struct lu_dump_state {
extern struct live_update *lu_status;
+struct connection *lu_get_connection(void);
+bool lu_is_pending(void);
+void lu_read_state(void);
+
+/* Write the "OK" response for the live-update command */
+unsigned int lu_write_response(FILE *fp);
+
+int do_control_lu(const void *ctx, struct connection *conn, char **vec,
+ int num);
+
/* Live update private interfaces. */
void lu_get_dump_state(struct lu_dump_state *state);
void lu_close_dump_state(struct lu_dump_state *state);
@@ -53,4 +63,19 @@ const char *lu_arch(const void *ctx, struct connection *conn, char **vec,
int num);
const char *lu_begin(struct connection *conn);
int lu_destroy(void *data);
+#else
+static inline struct connection *lu_get_connection(void)
+{
+ return NULL;
+}
+
+static inline unsigned int lu_write_response(FILE *fp)
+{
+ return 0;
+}
+
+static inline bool lu_is_pending(void)
+{
+ return false;
+}
#endif
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* [PATCH v3 16/16] tools/xenstore: remove unused stuff from list.h
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (14 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 15/16] tools/xenstore: split out rest of live update control code Juergen Gross
@ 2023-05-30 8:54 ` Juergen Gross
2023-06-19 18:04 ` Julien Grall
2023-06-09 18:46 ` [PATCH v3 00/16] tools/xenstore: more cleanups Julien Grall
16 siblings, 1 reply; 35+ messages in thread
From: Juergen Gross @ 2023-05-30 8:54 UTC (permalink / raw)
To: xen-devel; +Cc: Juergen Gross, Wei Liu, Julien Grall, Anthony PERARD
Remove the hlist defines/functions and the rcu related functions from
tools/xenstore/list.h, as they are not used.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
tools/xenstore/list.h | 227 ------------------------------------------
1 file changed, 227 deletions(-)
diff --git a/tools/xenstore/list.h b/tools/xenstore/list.h
index a464a38b61..d722a91220 100644
--- a/tools/xenstore/list.h
+++ b/tools/xenstore/list.h
@@ -88,48 +88,6 @@ static inline void list_add_tail(struct list_head *new, struct list_head *head)
__list_add(new, head->prev, head);
}
-/*
- * Insert a new entry between two known consecutive entries.
- *
- * This is only for internal list manipulation where we know
- * the prev/next entries already!
- */
-static __inline__ void __list_add_rcu(struct list_head * new,
- struct list_head * prev,
- struct list_head * next)
-{
- new->next = next;
- new->prev = prev;
- next->prev = new;
- prev->next = new;
-}
-
-/**
- * list_add_rcu - add a new entry to rcu-protected list
- * @new: new entry to be added
- * @head: list head to add it after
- *
- * Insert a new entry after the specified head.
- * This is good for implementing stacks.
- */
-static __inline__ void list_add_rcu(struct list_head *new, struct list_head *head)
-{
- __list_add_rcu(new, head, head->next);
-}
-
-/**
- * list_add_tail_rcu - add a new entry to rcu-protected list
- * @new: new entry to be added
- * @head: list head to add it before
- *
- * Insert a new entry before the specified head.
- * This is useful for implementing queues.
- */
-static __inline__ void list_add_tail_rcu(struct list_head *new, struct list_head *head)
-{
- __list_add_rcu(new, head->prev, head);
-}
-
/*
* Delete a list entry by making the prev/next entries
* point to each other.
@@ -156,23 +114,6 @@ static inline void list_del(struct list_head *entry)
entry->prev = LIST_POISON2;
}
-/**
- * list_del_rcu - deletes entry from list without re-initialization
- * @entry: the element to delete from the list.
- *
- * Note: list_empty on entry does not return true after this,
- * the entry is in an undefined state. It is useful for RCU based
- * lockfree traversal.
- *
- * In particular, it means that we can not poison the forward
- * pointers that may still be used for walking the list.
- */
-static inline void list_del_rcu(struct list_head *entry)
-{
- __list_del(entry->prev, entry->next);
- entry->prev = LIST_POISON2;
-}
-
/**
* list_del_init - deletes entry from list and reinitialize it.
* @entry: the element to delete from the list.
@@ -339,172 +280,4 @@ static inline void list_splice_init(struct list_head *list,
&pos->member != (head); \
pos = n, n = list_entry(n->member.next, typeof(*n), member))
-
-/*
- * Double linked lists with a single pointer list head.
- * Mostly useful for hash tables where the two pointer list head is
- * too wasteful.
- * You lose the ability to access the tail in O(1).
- */
-
-struct hlist_head {
- struct hlist_node *first;
-};
-
-struct hlist_node {
- struct hlist_node *next, **pprev;
-};
-
-#define HLIST_HEAD_INIT { .first = NULL }
-#define HLIST_HEAD(name) struct hlist_head name = { .first = NULL }
-#define INIT_HLIST_HEAD(ptr) ((ptr)->first = NULL)
-#define INIT_HLIST_NODE(ptr) ((ptr)->next = NULL, (ptr)->pprev = NULL)
-
-static __inline__ int hlist_unhashed(struct hlist_node *h)
-{
- return !h->pprev;
-}
-
-static __inline__ int hlist_empty(struct hlist_head *h)
-{
- return !h->first;
-}
-
-static __inline__ void __hlist_del(struct hlist_node *n)
-{
- struct hlist_node *next = n->next;
- struct hlist_node **pprev = n->pprev;
- *pprev = next;
- if (next)
- next->pprev = pprev;
-}
-
-static __inline__ void hlist_del(struct hlist_node *n)
-{
- __hlist_del(n);
- n->next = LIST_POISON1;
- n->pprev = LIST_POISON2;
-}
-
-/**
- * hlist_del_rcu - deletes entry from hash list without re-initialization
- * @entry: the element to delete from the hash list.
- *
- * Note: list_unhashed() on entry does not return true after this,
- * the entry is in an undefined state. It is useful for RCU based
- * lockfree traversal.
- *
- * In particular, it means that we can not poison the forward
- * pointers that may still be used for walking the hash list.
- */
-static inline void hlist_del_rcu(struct hlist_node *n)
-{
- __hlist_del(n);
- n->pprev = LIST_POISON2;
-}
-
-static __inline__ void hlist_del_init(struct hlist_node *n)
-{
- if (n->pprev) {
- __hlist_del(n);
- INIT_HLIST_NODE(n);
- }
-}
-
-#define hlist_del_rcu_init hlist_del_init
-
-static __inline__ void hlist_add_head(struct hlist_node *n, struct hlist_head *h)
-{
- struct hlist_node *first = h->first;
- n->next = first;
- if (first)
- first->pprev = &n->next;
- h->first = n;
- n->pprev = &h->first;
-}
-
-static __inline__ void hlist_add_head_rcu(struct hlist_node *n, struct hlist_head *h)
-{
- struct hlist_node *first = h->first;
- n->next = first;
- n->pprev = &h->first;
- if (first)
- first->pprev = &n->next;
- h->first = n;
-}
-
-/* next must be != NULL */
-static __inline__ void hlist_add_before(struct hlist_node *n, struct hlist_node *next)
-{
- n->pprev = next->pprev;
- n->next = next;
- next->pprev = &n->next;
- *(n->pprev) = n;
-}
-
-static __inline__ void hlist_add_after(struct hlist_node *n,
- struct hlist_node *next)
-{
- next->next = n->next;
- *(next->pprev) = n;
- n->next = next;
-}
-
-#define hlist_entry(ptr, type, member) container_of(ptr,type,member)
-
-/* Cannot easily do prefetch unfortunately */
-#define hlist_for_each(pos, head) \
- for (pos = (head)->first; pos; pos = pos->next)
-
-#define hlist_for_each_safe(pos, n, head) \
- for (pos = (head)->first; n = pos ? pos->next : 0, pos; \
- pos = n)
-
-/**
- * hlist_for_each_entry - iterate over list of given type
- * @tpos: the type * to use as a loop counter.
- * @pos: the &struct hlist_node to use as a loop counter.
- * @head: the head for your list.
- * @member: the name of the hlist_node within the struct.
- */
-#define hlist_for_each_entry(tpos, pos, head, member) \
- for (pos = (head)->first; \
- pos && ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
- pos = pos->next)
-
-/**
- * hlist_for_each_entry_continue - iterate over a hlist continuing after existing point
- * @tpos: the type * to use as a loop counter.
- * @pos: the &struct hlist_node to use as a loop counter.
- * @member: the name of the hlist_node within the struct.
- */
-#define hlist_for_each_entry_continue(tpos, pos, member) \
- for (pos = (pos)->next; \
- pos && ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
- pos = pos->next)
-
-/**
- * hlist_for_each_entry_from - iterate over a hlist continuing from existing point
- * @tpos: the type * to use as a loop counter.
- * @pos: the &struct hlist_node to use as a loop counter.
- * @member: the name of the hlist_node within the struct.
- */
-#define hlist_for_each_entry_from(tpos, pos, member) \
- for (; pos && ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
- pos = pos->next)
-
-/**
- * hlist_for_each_entry_safe - iterate over list of given type safe against removal of list entry
- * @tpos: the type * to use as a loop counter.
- * @pos: the &struct hlist_node to use as a loop counter.
- * @n: another &struct hlist_node to use as temporary storage
- * @head: the head for your list.
- * @member: the name of the hlist_node within the struct.
- */
-#define hlist_for_each_entry_safe(tpos, pos, n, head, member) \
- for (pos = (head)->first; \
- pos && ({ n = pos->next; 1; }) && \
- ({ tpos = hlist_entry(pos, typeof(*tpos), member); 1;}); \
- pos = n)
-
#endif
--
2.35.3
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [PATCH v3 00/16] tools/xenstore: more cleanups
2023-05-30 8:54 [PATCH v3 00/16] tools/xenstore: more cleanups Juergen Gross
` (15 preceding siblings ...)
2023-05-30 8:54 ` [PATCH v3 16/16] tools/xenstore: remove unused stuff from list.h Juergen Gross
@ 2023-06-09 18:46 ` Julien Grall
16 siblings, 0 replies; 35+ messages in thread
From: Julien Grall @ 2023-06-09 18:46 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Wei Liu, Anthony PERARD, Andrew Cooper, George Dunlap,
Jan Beulich, Stefano Stabellini
Hi Juergen,
On 30/05/2023 09:54, Juergen Gross wrote:
> Some more cleanups of Xenstore.
>
> Based on top of the previous Xenstore series "tools/xenstore: rework
> internal accounting".
>
> Changes in V2:
> - rebase
> - one small modification of patch 10
> - added patches 11-13
>
> Changes in V3:
> - rebase
> - modified patch 4
> - added patches 10, 11 and 13
>
> Juergen Gross (16):
> tools/xenstore: verify command line parameters better
> tools/xenstore: do some cleanup of hashtable.c
> tools/xenstore: modify interface of create_hashtable()
> tools/xenstore: rename hashtable_insert() and let it return 0 on
> success
> tools/xenstore: make some write limit functions static
> tools/xenstore: switch write limiting to use millisecond time base
> tools/xenstore: remove stale TODO file
> tools/xenstore: remove unused events list
> tools/xenstore: remove support of file backed data base
I have committed up to here. I still need to review the rest.
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 35+ messages in thread