Re: patch - cgdconfig do_all mode gracefully ignore detached devices

[Jason High <json.high%gmail.com@localhost>]

Thanks for your feedback!  We cannot fail silently in configure as-is
due to the config/verification flow.  However, if what was meant was
to check for ENXIO in the verification function, then that works, and
is a much simpler solution.  In the below diff, we removed the check
on -U per the issues raised by Robert.  For -C, we simply try to open
the device path.  On error, if we get ENXIO we return 0.
Otherwise we let -1 pass through to be handled as expected in the
flow.  Is this any closer to your liking?  Tested and working as
expected.  Thanks so much for your review

Index: cgdconfig.c
===================================================================
RCS file: /cvsroot/src/sbin/cgdconfig/cgdconfig.c,v
retrieving revision 1.50
diff -b -u -r1.50 cgdconfig.c
--- cgdconfig.c    10 Apr 2019 06:11:37 -0000    1.50
+++ cgdconfig.c    10 Nov 2019 15:09:33 -0000
@@ -125,6 +125,7 @@
 static int     verify_reenter(struct params *);
 static int     verify_mbr(int);
 static int     verify_gpt(int);
+static int     verify_attached(const char * opath);

 __dead static void     usage(void);

@@ -540,6 +541,19 @@
         return -1;
     }

+    /* if called from do_all, then ignore if device is not attached */
+    if (flags == CONFIG_FLAGS_FROMALL) {
+
+        ret = verify_attached(argv[1]);
+
+        /* device unattached */
+        if (ret == 0) {
+            VPRINTF(1, ("skipping %s: device is not attached\n", argv[1]));
+            return 0;
+        }
+        /* pass-through success and errors */
+    }
+
     if ((
       fd = opendisk1(*argv, O_RDWR, cgdname, sizeof(cgdname), 1, prog_open)
         ) != -1) {
@@ -1282,3 +1296,25 @@
     if (setrlimit(RLIMIT_CORE, &rlp) == -1)
         err(EXIT_FAILURE, "Can't disable cores");
 }
+
+static int
+verify_attached(const char * opath)
+ check if opath is accessible
+{
+    int fd;
+    char path[MAXPATHLEN];
+
+    fd = opendisk(opath,O_RDWR,path,sizeof(path),0);
+
+    if (fd == -1) {
+        if (errno == ENXIO) {
+            return 0;
+        }
+        return -1;
+    }
+
+    close(fd);
+
+    return 1;
+}

On Sun, Nov 10, 2019 at 1:42 AM Michael van Elst <mlelstv%serpens.de@localhost> wrote:
>
> On Sun, Nov 10, 2019 at 01:21:21AM -0600, Jason High wrote:
> > For -U, that could work (was actually how we handled it in an earlier
> > version).  For -C, no.  Our goal is to avoid cgd trying to configure a
> > device, like those backed by USB drives, on boot when the backing
> > device isn't attached.
>
> Why? Whether you rely on magic (== non-standard interfaces with
> unspecified semantics) to skip the configuration or the configuration
> just fails silently shouldn't make a difference.
>
>
> Greetings,
> --
>                                 Michael van Elst
> Internet: mlelstv%serpens.de@localhost
>                                 "A potential Snark may lurk in every tree."