Allocate the needed file type via alloc_file_type().
Instead of directly accessing the files[] array use get_file_from_fd().
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V3:
- switch to struct file * parameter for callbacks
- use __attribute__((constructor))
---
include/lib.h | 3 +--
lib/sys.c | 18 --------------
lib/xs.c | 65 +++++++++++++++++++++++++++++++++++++------------
xenbus/xenbus.c | 1 +
4 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/include/lib.h b/include/lib.h
index 5838b0d4..c5e47786 100644
--- a/include/lib.h
+++ b/include/lib.h
@@ -169,8 +169,7 @@ extern struct wait_queue_head event_queue;
#define FTYPE_BLK 9
#define FTYPE_TPMFRONT 10
#define FTYPE_TPM_TIS 11
-#define FTYPE_XENBUS 12
-#define FTYPE_N 13
+#define FTYPE_N 12
#define FTYPE_SPARE 16
struct file {
diff --git a/lib/sys.c b/lib/sys.c
index 6ba26294..53870aa4 100644
--- a/lib/sys.c
+++ b/lib/sys.c
@@ -523,11 +523,6 @@ int close(int fd)
switch (file->type) {
default:
break;
-#ifdef CONFIG_LIBXS
- case FTYPE_XENBUS:
- xs_daemon_close((void*)(intptr_t) fd);
- break;
-#endif
#ifdef HAVE_LWIP
case FTYPE_SOCKET:
res = lwip_close(files[fd].fd);
@@ -759,7 +754,6 @@ int closedir(DIR *dir)
static const char *const file_types[] = {
[FTYPE_NONE] = "none",
[FTYPE_CONSOLE] = "console",
- [FTYPE_XENBUS] = "xenbus",
[FTYPE_SOCKET] = "socket",
[FTYPE_TAP] = "net",
[FTYPE_BLK] = "blk",
@@ -947,18 +941,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
n++;
FD_CLR(i, exceptfds);
break;
-#ifdef CONFIG_LIBXS
- case FTYPE_XENBUS:
- if (FD_ISSET(i, readfds)) {
- if (files[i].dev)
- n++;
- else
- FD_CLR(i, readfds);
- }
- FD_CLR(i, writefds);
- FD_CLR(i, exceptfds);
- break;
-#endif
case FTYPE_TAP:
case FTYPE_BLK:
case FTYPE_KBD:
diff --git a/lib/xs.c b/lib/xs.c
index 4af0f960..c12341aa 100644
--- a/lib/xs.c
+++ b/lib/xs.c
@@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
return (intptr_t) h;
}
+static int xs_close_fd(struct file *file)
+{
+ struct xenbus_event *event, *next;
+
+ for (event = file->dev; event; event = next)
+ {
+ next = event->next;
+ free(event);
+ }
+
+ return 0;
+}
+
+static bool xs_can_read(struct file *file)
+{
+ return file && file->dev;
+}
+
+static const struct file_ops xenbus_ops = {
+ .name = "xenbus",
+ .close = xs_close_fd,
+ .select_rd = xs_can_read,
+};
+
+static unsigned int ftype_xenbus;
+
+__attribute__((constructor))
+static void xs_initialize(void)
+{
+ ftype_xenbus = alloc_file_type(&xenbus_ops);
+}
+
struct xs_handle *xs_daemon_open()
{
- int fd = alloc_fd(FTYPE_XENBUS);
- files[fd].dev = NULL;
- printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
+ int fd;
+ struct file *file;
+
+ fd = alloc_fd(ftype_xenbus);
+ file = get_file_from_fd(fd);
+ if ( !file )
+ return NULL;
+
+ file->dev = NULL;
+ printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
return (void*)(intptr_t) fd;
}
void xs_daemon_close(struct xs_handle *h)
{
- int fd = _xs_fileno(h);
- struct xenbus_event *event, *next;
- for (event = files[fd].dev; event; event = next)
- {
- next = event->next;
- free(event);
- }
+ close(_xs_fileno(h));
}
int xs_fileno(struct xs_handle *h)
@@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
bool xs_watch(struct xs_handle *h, const char *path, const char *token)
{
- int fd = _xs_fileno(h);
+ struct file *file = get_file_from_fd(_xs_fileno(h));
+
printk("xs_watch(%s, %s)\n", path, token);
return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
- (xenbus_event_queue *)&files[fd].dev));
+ (xenbus_event_queue *)&file->dev));
}
char **xs_read_watch(struct xs_handle *h, unsigned int *num)
{
- int fd = _xs_fileno(h);
struct xenbus_event *event;
- event = files[fd].dev;
- files[fd].dev = event->next;
+ struct file *file = get_file_from_fd(_xs_fileno(h));
+
+ event = file->dev;
+ file->dev = event->next;
printk("xs_read_watch() -> %s %s\n", event->path, event->token);
*num = 2;
return (char **) &event->path;
diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
index b687678f..785389fb 100644
--- a/xenbus/xenbus.c
+++ b/xenbus/xenbus.c
@@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
void init_xenbus(void)
{
int err;
+
DEBUG("init_xenbus called.\n");
create_thread("xenstore", xenbus_thread_func, NULL);
DEBUG("buf at %p.\n", xenstore_buf);
--
2.26.2
Juergen Gross, le dim. 16 janv. 2022 09:33:19 +0100, a ecrit:
> Allocate the needed file type via alloc_file_type().
>
> Instead of directly accessing the files[] array use get_file_from_fd().
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
> V3:
> - switch to struct file * parameter for callbacks
> - use __attribute__((constructor))
> ---
> include/lib.h | 3 +--
> lib/sys.c | 18 --------------
> lib/xs.c | 65 +++++++++++++++++++++++++++++++++++++------------
> xenbus/xenbus.c | 1 +
> 4 files changed, 52 insertions(+), 35 deletions(-)
>
> diff --git a/include/lib.h b/include/lib.h
> index 5838b0d4..c5e47786 100644
> --- a/include/lib.h
> +++ b/include/lib.h
> @@ -169,8 +169,7 @@ extern struct wait_queue_head event_queue;
> #define FTYPE_BLK 9
> #define FTYPE_TPMFRONT 10
> #define FTYPE_TPM_TIS 11
> -#define FTYPE_XENBUS 12
> -#define FTYPE_N 13
> +#define FTYPE_N 12
> #define FTYPE_SPARE 16
>
> struct file {
> diff --git a/lib/sys.c b/lib/sys.c
> index 6ba26294..53870aa4 100644
> --- a/lib/sys.c
> +++ b/lib/sys.c
> @@ -523,11 +523,6 @@ int close(int fd)
> switch (file->type) {
> default:
> break;
> -#ifdef CONFIG_LIBXS
> - case FTYPE_XENBUS:
> - xs_daemon_close((void*)(intptr_t) fd);
> - break;
> -#endif
> #ifdef HAVE_LWIP
> case FTYPE_SOCKET:
> res = lwip_close(files[fd].fd);
> @@ -759,7 +754,6 @@ int closedir(DIR *dir)
> static const char *const file_types[] = {
> [FTYPE_NONE] = "none",
> [FTYPE_CONSOLE] = "console",
> - [FTYPE_XENBUS] = "xenbus",
> [FTYPE_SOCKET] = "socket",
> [FTYPE_TAP] = "net",
> [FTYPE_BLK] = "blk",
> @@ -947,18 +941,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce
> n++;
> FD_CLR(i, exceptfds);
> break;
> -#ifdef CONFIG_LIBXS
> - case FTYPE_XENBUS:
> - if (FD_ISSET(i, readfds)) {
> - if (files[i].dev)
> - n++;
> - else
> - FD_CLR(i, readfds);
> - }
> - FD_CLR(i, writefds);
> - FD_CLR(i, exceptfds);
> - break;
> -#endif
> case FTYPE_TAP:
> case FTYPE_BLK:
> case FTYPE_KBD:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f960..c12341aa 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
> return (intptr_t) h;
> }
>
> +static int xs_close_fd(struct file *file)
> +{
> + struct xenbus_event *event, *next;
> +
> + for (event = file->dev; event; event = next)
> + {
> + next = event->next;
> + free(event);
> + }
> +
> + return 0;
> +}
> +
> +static bool xs_can_read(struct file *file)
> +{
> + return file && file->dev;
> +}
> +
> +static const struct file_ops xenbus_ops = {
> + .name = "xenbus",
> + .close = xs_close_fd,
> + .select_rd = xs_can_read,
> +};
> +
> +static unsigned int ftype_xenbus;
> +
> +__attribute__((constructor))
> +static void xs_initialize(void)
> +{
> + ftype_xenbus = alloc_file_type(&xenbus_ops);
> +}
> +
> struct xs_handle *xs_daemon_open()
> {
> - int fd = alloc_fd(FTYPE_XENBUS);
> - files[fd].dev = NULL;
> - printk("xs_daemon_open -> %d, %p\n", fd, &files[fd].dev);
> + int fd;
> + struct file *file;
> +
> + fd = alloc_fd(ftype_xenbus);
> + file = get_file_from_fd(fd);
> + if ( !file )
> + return NULL;
> +
> + file->dev = NULL;
> + printk("xs_daemon_open -> %d, %p\n", fd, &file->dev);
> return (void*)(intptr_t) fd;
> }
>
> void xs_daemon_close(struct xs_handle *h)
> {
> - int fd = _xs_fileno(h);
> - struct xenbus_event *event, *next;
> - for (event = files[fd].dev; event; event = next)
> - {
> - next = event->next;
> - free(event);
> - }
> + close(_xs_fileno(h));
> }
>
> int xs_fileno(struct xs_handle *h)
> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>
> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
> {
> - int fd = _xs_fileno(h);
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> printk("xs_watch(%s, %s)\n", path, token);
> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> - (xenbus_event_queue *)&files[fd].dev));
> + (xenbus_event_queue *)&file->dev));
> }
>
> char **xs_read_watch(struct xs_handle *h, unsigned int *num)
> {
> - int fd = _xs_fileno(h);
> struct xenbus_event *event;
> - event = files[fd].dev;
> - files[fd].dev = event->next;
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> + event = file->dev;
> + file->dev = event->next;
> printk("xs_read_watch() -> %s %s\n", event->path, event->token);
> *num = 2;
> return (char **) &event->path;
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678f..785389fb 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
> void init_xenbus(void)
> {
> int err;
> +
> DEBUG("init_xenbus called.\n");
> create_thread("xenstore", xenbus_thread_func, NULL);
> DEBUG("buf at %p.\n", xenstore_buf);
> --
> 2.26.2
>
--
Samuel
RR> Ce que je cherche à démontrer, c'est qu'il est injuste de faire
RR> l'amalgame entre du bulk mail et du courrier non-solicité très ciblé
un suppositoire non reclamé, meme tres bien ciblé, reste un suppositoire.
-+-OS in : Guide du Neuneu d'Usenet - Plein le cul de la pub à neuneu -+-
On 16/01/2022 08:33, Juergen Gross wrote:
> diff --git a/lib/xs.c b/lib/xs.c
> index 4af0f960..c12341aa 100644
> --- a/lib/xs.c
> +++ b/lib/xs.c
> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
> return (intptr_t) h;
> }
>
> +static int xs_close_fd(struct file *file)
> +{
> + struct xenbus_event *event, *next;
> +
> + for (event = file->dev; event; event = next)
> + {
> + next = event->next;
> + free(event);
> + }
> +
> + return 0;
> +}
> +
> +static bool xs_can_read(struct file *file)
> +{
> + return file && file->dev;
Just 'return file->dev;' ?
> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>
> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
> {
> - int fd = _xs_fileno(h);
> + struct file *file = get_file_from_fd(_xs_fileno(h));
> +
> printk("xs_watch(%s, %s)\n", path, token);
> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
> - (xenbus_event_queue *)&files[fd].dev));
> + (xenbus_event_queue *)&file->dev));
This is utterly mad. In particular, close() looks to be very racy with
new watches arriving.
However, can the indentation at least be fixed here as the line is
changing. That's a parameter to xenbus_watch_path_token(), not xs_bool().
> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
> index b687678f..785389fb 100644
> --- a/xenbus/xenbus.c
> +++ b/xenbus/xenbus.c
> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
> void init_xenbus(void)
> {
> int err;
> +
Spurious hunk?
~Andrew
On 18.01.22 15:29, Andrew Cooper wrote:
> On 16/01/2022 08:33, Juergen Gross wrote:
>> diff --git a/lib/xs.c b/lib/xs.c
>> index 4af0f960..c12341aa 100644
>> --- a/lib/xs.c
>> +++ b/lib/xs.c
>> @@ -18,23 +18,56 @@ static inline int _xs_fileno(struct xs_handle *h) {
>> return (intptr_t) h;
>> }
>>
>> +static int xs_close_fd(struct file *file)
>> +{
>> + struct xenbus_event *event, *next;
>> +
>> + for (event = file->dev; event; event = next)
>> + {
>> + next = event->next;
>> + free(event);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool xs_can_read(struct file *file)
>> +{
>> + return file && file->dev;
>
> Just 'return file->dev;' ?
Yes.
>
>> @@ -169,18 +202,20 @@ char **xs_directory(struct xs_handle *h, xs_transaction_t t,
>>
>> bool xs_watch(struct xs_handle *h, const char *path, const char *token)
>> {
>> - int fd = _xs_fileno(h);
>> + struct file *file = get_file_from_fd(_xs_fileno(h));
>> +
>> printk("xs_watch(%s, %s)\n", path, token);
>> return xs_bool(xenbus_watch_path_token(XBT_NULL, path, token,
>> - (xenbus_event_queue *)&files[fd].dev));
>> + (xenbus_event_queue *)&file->dev));
>
> This is utterly mad. In particular, close() looks to be very racy with
> new watches arriving.
In practice it should be no problem, though (closing the file in one
thread while the other one is adding a watch would be rather strange).
Additionally close() for xenbus in Mini-OS is called only when stopping
the domain today.
> However, can the indentation at least be fixed here as the line is
> changing. That's a parameter to xenbus_watch_path_token(), not xs_bool().
Yes, that should be done.
>
>> diff --git a/xenbus/xenbus.c b/xenbus/xenbus.c
>> index b687678f..785389fb 100644
>> --- a/xenbus/xenbus.c
>> +++ b/xenbus/xenbus.c
>> @@ -393,6 +393,7 @@ static int allocate_xenbus_id(void)
>> void init_xenbus(void)
>> {
>> int err;
>> +
>
> Spurious hunk?
Yes.
Juergen
© 2016 - 2024 Red Hat, Inc.