[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Minios-devel] [UNIKRAFT PATCH 2/8] lib/uk9p: Add 9P transport registration
On 19.06.19 09:03, Cristian Banu wrote:
This patch introduces the 9P transport registration and lookup API.
Maybe you could also explain the concept of having multiple of them and
describe what a default transport is and what it is for. In case you
keep it.
Signed-off-by: Cristian Banu <cristb@xxxxxxxxx>
---
lib/uk9p/9pdev_trans.c | 82 +++++++++++++++++++++++++++++++
lib/uk9p/Makefile.uk | 2 +
lib/uk9p/exportsyms.uk | 3 ++
lib/uk9p/include/uk/9pdev_trans.h | 101 ++++++++++++++++++++++++++++++++++++++
4 files changed, 188 insertions(+)
create mode 100644 lib/uk9p/9pdev_trans.c
create mode 100644 lib/uk9p/exportsyms.uk
create mode 100644 lib/uk9p/include/uk/9pdev_trans.h
diff --git a/lib/uk9p/9pdev_trans.c b/lib/uk9p/9pdev_trans.c
new file mode 100644
index 000000000000..60b8a230cbf3
--- /dev/null
+++ b/lib/uk9p/9pdev_trans.c
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Cristian Banu <cristb@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, University Politehnica of Bucharest. All rights
reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#include <errno.h>
+#include <string.h>
+#include <uk/config.h>
+#include <uk/list.h>
+#include <uk/assert.h>
+#include <uk/9pdev_trans.h>
+
+static UK_LIST_HEAD(uk_9pdev_trans_list);
+
+int uk_9pdev_trans_register(struct uk_9pdev_trans *trans)
+{
+ UK_ASSERT(trans);
+ UK_ASSERT(trans->name);
+ UK_ASSERT(trans->a);
+
+ uk_list_add_tail(&trans->_list, &uk_9pdev_trans_list);
+ uk_pr_info("Registered transport %s\n", trans->name);
+
+ return 0;
+}
+
+int uk_9pdev_trans_by_name(const char *name, struct uk_9pdev_trans **trans)
+{
+ struct uk_9pdev_trans *t;
+
+ uk_list_for_each_entry(t, &uk_9pdev_trans_list, _list) {
+ if (!strcmp(t->name, name)) {
+ *trans = t;
+ return 0;
+ }
+ }
+
+ return -ENODEV;
+}
+
+int uk_9pdev_trans_default(struct uk_9pdev_trans **trans)
+{
+ struct uk_9pdev_trans *t;
+
+ uk_list_for_each_entry(t, &uk_9pdev_trans_list, _list) {
+ if (t->is_default) {
+ *trans = t;
+ return 0;
+ }
+ }
Maybe you want to store internally to the library an extra pointer that
points to the current default transport. Then you would not need this
boolean scheme and to traverse the list.
+
+ return -ENODEV;
+}
diff --git a/lib/uk9p/Makefile.uk b/lib/uk9p/Makefile.uk
index fa754440598c..b1071a0e7d3c 100644
--- a/lib/uk9p/Makefile.uk
+++ b/lib/uk9p/Makefile.uk
@@ -2,3 +2,5 @@ $(eval $(call addlib_s,libuk9p,$(CONFIG_LIBUK9P)))
CINCLUDES-$(CONFIG_LIBUK9P) += -I$(LIBUK9P_BASE)/include
CXXINCLUDES-$(CONFIG_LIBUK9P) += -I$(LIBUK9P_BASE)/include
+
+LIBUK9P_SRCS-y += $(LIBUK9P_BASE)/9pdev_trans.c
diff --git a/lib/uk9p/exportsyms.uk b/lib/uk9p/exportsyms.uk
new file mode 100644
index 000000000000..45f487da2fff
--- /dev/null
+++ b/lib/uk9p/exportsyms.uk
@@ -0,0 +1,3 @@
+uk_9pdev_trans_register
+uk_9pdev_trans_by_name
+uk_9pdev_trans_default
diff --git a/lib/uk9p/include/uk/9pdev_trans.h
b/lib/uk9p/include/uk/9pdev_trans.h
new file mode 100644
index 000000000000..a0c6d252361c
--- /dev/null
+++ b/lib/uk9p/include/uk/9pdev_trans.h
@@ -0,0 +1,101 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Cristian Banu <cristb@xxxxxxxxx>
+ *
+ * Copyright (c) 2019, University Politehnica of Bucharest. All rights
reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+#ifndef __UK_9PDEV_TRANS__
+#define __UK_9PDEV_TRANS__
+
+#include <stdbool.h>
+#include <uk/config.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** A structure used to describe a transport. */
+struct uk_9pdev_trans {
+ /*
+ * Transport name (e.g. "virtio", "xen"). This field is reserved for
+ * future use, when multiple transport options are available on the
+ * same platform.
+ */
What is a common case where you have multiple transports? Is it like
multiple shared mount points?
I am wondering if we need the abstraction of multiple transports.
+ const char *name;
+ /* Can the transport be used as a default one? */
+ bool is_default;
+ /* Allocator used for devices which use this transport layer. */
+ struct uk_alloc *a;
+ /* @internal Entry in the list of available transports. */
+ struct uk_list_head _list;
+};
+
+/**
+ * Adds a transport to the available transports list for Unikraft 9P Devices.
+ * This should be called once per driver (once for virtio, once for xen, etc.).
+ *
+ * @param trans
+ * Pointer to the transport structure.
+ * @return
+ * - (0): Successful.
+ * - (< 0): Failed to register the transport layer.
+ */
+int uk_9pdev_trans_register(struct uk_9pdev_trans *trans);
+
+/**
+ * Looks up a transport layer by its name.
+ *
+ * @param name
+ * The transport layer name.
+ * @param trans
+ * Where to store the pointer to the found transport structure.
+ * @return
+ * - (-ENOENT): No transport with the given name.
+ * - (0): Successful.
+ */
+int uk_9pdev_trans_by_name(const char *name, struct uk_9pdev_trans **trans);
+
+/**
+ * Gets the default transport layer.
+ *
+ * @param trans
+ * Where to store the pointer to the found transport structure.
+ * @return
+ * - (-ENOENT): No default transport found.
I saw that your function returns -ENODEV. This description is
mismatching. ;-)
+ * - (0): Successful.
+ */
+int uk_9pdev_trans_default(struct uk_9pdev_trans **trans);
Do you ever expect to return a different error code? What about
returning the pointer (instead of going through an argument) and NULL in
error case? We had a similar API decision in ukalloc
(uk_alloc_get_default()) and uksched, so it would fit here.
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __UK_9PDEV_TRANS__ */
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
