Thanks for the quick chat. Per suggestions, I'm using open rather
than opendisk. Ok?
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:56:40 -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,19 @@
if (setrlimit(RLIMIT_CORE, &rlp) == -1)
err(EXIT_FAILURE, "Can't disable cores");
}
+
+static int
+verify_attached(const char * opath)
+{
+ int fd;
+
+ if (fd = open(opath,O_RDWR) == -1) {
+ if (errno == ENXIO) {
+ return 0;
+ }
+ return -1;
+ }
+ close(fd);
+
+ return 1;
+}
On Sun, Nov 10, 2019 at 9:37 AM Jason High <json.h...@gmail.com> wrote:
>
> 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 <mlel...@serpens.de> 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: mlel...@serpens.de
> > "A potential Snark may lurk in every tree."
