[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [MINIOS PATCH v3 03/12] use alloc_file_type() and get_file_from_fd() in xs



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

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.