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

Re: [Minios-devel] [UNIKRAFT PATCH 5/6] lib/devfs: Add /dev/null support



On 06.12.19 14:41, Costin Lupu wrote:
This is shamelessly copied and adapted from our implementation
for /dev/random device support.

Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
---
  lib/devfs/Config.uk   |   5 ++
  lib/devfs/Makefile.uk |   2 +
  lib/devfs/null.c      | 115 ++++++++++++++++++++++++++++++++++++++++++
  3 files changed, 122 insertions(+)
  create mode 100644 lib/devfs/null.c

diff --git a/lib/devfs/Config.uk b/lib/devfs/Config.uk
index 6f21c01c..3adc171e 100644
--- a/lib/devfs/Config.uk
+++ b/lib/devfs/Config.uk
@@ -8,4 +8,9 @@ if LIBDEVFS
          bool "Mount /dev during boot"
        depends on LIBVFSCORE_AUTOMOUNT_ROOTFS
          default n
+
+       config LIBDEVS_DEV_NULL
+               bool "/dev/null"
+               depends on LIBDEVFS_AUTOMOUNT
+               default y

Hum, the `null` device should be actually independent of the automount option. You could always provide it because devfs can be in principle mounted somewhere else and still provide `null`.

Instead of adding a hard-dependency, I would set the default to yes as soon as libdevfs automount is enabled:

        config LIBDEVS_DEV_NULL
                bool "null device"
                default y if LIBDEVFS_AUTOMOUNT
                default n

Of course, the default setting should be `off` in order to follow the "default is minimal" principle.

  endif
diff --git a/lib/devfs/Makefile.uk b/lib/devfs/Makefile.uk
index c496fd56..366db677 100644
--- a/lib/devfs/Makefile.uk
+++ b/lib/devfs/Makefile.uk
@@ -3,6 +3,8 @@ $(eval $(call addlib_s,libdevfs,$(CONFIG_LIBDEVFS)))
  CINCLUDES-y += -I$(LIBDEVFS_BASE)/include
LIBDEVFS_CFLAGS-$(call gcc_version_ge,8,0) += -Wno-cast-function-type
+LIBDEVFS_CFLAGS-y += -Wno-unused-parameter
LIBDEVFS_SRCS-y += $(LIBDEVFS_BASE)/device.c
  LIBDEVFS_SRCS-y += $(LIBDEVFS_BASE)/devfs_vnops.c
+LIBDEVFS_SRCS-$(CONFIG_LIBDEVS_DEV_NULL) += $(LIBDEVFS_BASE)/null.c
diff --git a/lib/devfs/null.c b/lib/devfs/null.c
new file mode 100644
index 00000000..38d687d4
--- /dev/null
+++ b/lib/devfs/null.c
@@ -0,0 +1,115 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Costin Lupu <costin.lupu@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 <stdlib.h>
+#include <string.h>
+#include <uk/ctors.h>
+#include <uk/print.h>
+#include <vfscore/uio.h>
+#include <devfs/device.h>
+
+#define DEV_NULL_NAME "null"
+
+/* TODO This can be integrated with random's fill buffer function */
+static ssize_t uk_null_fill_buffer(void *buf, size_t buflen)
+{
+       size_t step, resid, i;
+
+       step = sizeof(unsigned long);
+       resid = buflen % step;
+
+       for (i = 0; i < buflen - resid; i += step)
+               *(unsigned long *)((char *) buf + i) = 0;
+
+       /* fill the remaining bytes of the buffer */
+       for (i = 0; i < resid; i++)
+               *((char *) buf + i) = 0;

memset() would work, too, right? It makes the implementation probably easier and we would not need to integrate it with /dev/randmom. However, your implementation looks like /dev/zero. /dev/null should only return "end-of-file" and no bytes.

+
+       return buflen;
+}
+
+int dev_null_read(struct device *dev, struct uio *uio, int flags)
+{
+       size_t count;
+       char *buf;
+
+       buf = uio->uio_iov->iov_base;
+       count = uio->uio_iov->iov_len;
+
+       uk_null_fill_buffer(buf, count);
+
+       uio->uio_resid = 0;
+       return 0;
+}
+
+int dev_null_open(struct device *device, int mode)
+{
+       return 0;
+}
+
+
+int dev_null_close(struct device *device)
+{
+       return 0;
+}
+
+static struct devops null_devops = {
+       .read = dev_null_read,
+       .open = dev_null_open,
+       .close = dev_null_close,
+};

You should also provide the write function. The input is just dropped. This should be even the same for /dev/zero.

+
+static struct driver drv_null = {
+       .devops = &null_devops,
+       .devsz = 0,
+       .name = DEV_NULL_NAME
+};
+
+static int devfs_register_null(void)
+{
+       struct device *dev;
+
+       uk_pr_info("Register '%s' to devfs\n", DEV_NULL_NAME);
+
+       /* register /dev/null */
+       dev = device_create(&drv_null, DEV_NULL_NAME, D_CHR);
+       if (dev == NULL) {
+               uk_pr_err("Failed to register '%s' to devfs\n", DEV_NULL_NAME);
+               return -1;
+       }
+
+       return 0;
+}
+
+devfs_initcall(devfs_register_null);


I think it is worth to provide /dev/zero, too. You have it actually already implemented ;-). I think it would make sense to keep both: /dev/zero and /dev/null.

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel

 


Rackspace

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