On Tue, Sep 22, 2020 at 09:59:36PM +0200, Martin Wilck wrote:
> On Tue, 2020-09-22 at 20:34 +0200, Frank Meinl wrote:
> > This change adds a new configuration option allow_usb_devices. It is
> > disabled by default, so that the behavior of existing setups is not
> > changed. If enabled (via multipath.conf), USB devices will be found
> > during device discovery and can be used for multipath setups.
> >
> > Without this option, multipath currently ignores all USB drives,
> > which
> > is convenient for most users. (refer also to commit 51957eb)
> >
> > However, it can be beneficial to use the multipath dm-module even if
> > there is only a single path to an USB attached storage. In
> > combination
> > with the option 'queue_if_no_path', such a setup survives a temporary
> > device disconnect without any severe consequences for the running
> > applications. All I/O is queued until the device reappears.
>
> Hm. So you want to use multipath just to enable queueing?
> I wonder if that can't be achieved some other way; multipath seems to
> be quite big hammer for this nail. Anyway, I don't think this would
> hurt us, so I don't have good reasons to reject it.
>
> Waiting for others' opinions.
I've actually seen other cases where people are using multipath on
single path devices just for the queuing, and when I thought about it, I
realized that it makes sense. Multipath works with devices as they are,
instead of needing special metadata, like lvm devices. People often
realize that they need this after the device is already set up. Plus,
multipath already deals with devices that have partitions or logical
volumes on top of them. It's also easy to configure exactly how you want
queueing to work. This use case might be a small nail, but it is a nail,
and multipath is a reasonable tool to get the job done.
It doesn't seem too hard to write a dm target that would queue and retry
IO at some configurable interval, for some configurable number of times,
when it failed. But you would also need to copy the work for getting the
device, and any partitons on it, to autoassemble after the frist time
it's set up and to make sure other layers use this device instead of the
underlying device. Or, people can just use multipath.
-Ben
> >
> > Signed-off-by: Frank Meinl <frank.meinl@xxxxxxx>
> > ---
> > libmultipath/config.h | 1 +
> > libmultipath/dict.c | 4 ++++
> > libmultipath/discovery.c | 13 ++++++++++---
> > libmultipath/structs.h | 1 +
> > multipath/multipath.conf.5 | 14 ++++++++++++++
> > 5 files changed, 30 insertions(+), 3 deletions(-)
> >
> > diff --git a/libmultipath/config.h b/libmultipath/config.h
> > index 2bb7153b..290aea58 100644
> > --- a/libmultipath/config.h
> > +++ b/libmultipath/config.h
> > @@ -158,6 +158,7 @@ struct config {
> > unsigned int dev_loss;
> > int log_checker_err;
> > int allow_queueing;
> > + int allow_usb_devices;
> > int find_multipaths;
> > uid_t uid;
> > gid_t gid;
> > diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> > index feabae56..f12c2e5c 100644
> > --- a/libmultipath/dict.c
> > +++ b/libmultipath/dict.c
> > @@ -543,6 +543,9 @@ snprint_def_queue_without_daemon (struct config
> > *conf,
> > declare_def_handler(checker_timeout, set_int)
> > declare_def_snprint(checker_timeout, print_nonzero)
> >
> > +declare_def_handler(allow_usb_devices, set_yes_no)
> > +declare_def_snprint(allow_usb_devices, print_yes_no)
> > +
> > declare_def_handler(flush_on_last_del, set_yes_no_undef)
> > declare_def_snprint_defint(flush_on_last_del, print_yes_no_undef,
> > DEFAULT_FLUSH)
> > declare_ovr_handler(flush_on_last_del, set_yes_no_undef)
> > @@ -1759,6 +1762,7 @@ init_keywords(vector keywords)
> > install_keyword("no_path_retry", &def_no_path_retry_handler,
> > &snprint_def_no_path_retry);
> > install_keyword("queue_without_daemon",
> > &def_queue_without_daemon_handler,
> > &snprint_def_queue_without_daemon);
> > install_keyword("checker_timeout",
> > &def_checker_timeout_handler, &snprint_def_checker_timeout);
> > + install_keyword("allow_usb_devices",
> > &def_allow_usb_devices_handler, &snprint_def_allow_usb_devices);
> > install_keyword("pg_timeout", &deprecated_handler,
> > &snprint_deprecated);
> > install_keyword("flush_on_last_del",
> > &def_flush_on_last_del_handler, &snprint_def_flush_on_last_del);
> > install_keyword("user_friendly_names",
> > &def_user_friendly_names_handler, &snprint_def_user_friendly_names);
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index 2f301ac4..4b615caa 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -375,11 +375,10 @@ sysfs_get_tgt_nodename(struct path *pp, char
> > *node)
> > while (tgtdev) {
> > value = udev_device_get_subsystem(tgtdev);
> > if (value && !strcmp(value, "usb")) {
> > - pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
> > + pp->sg_id.proto_id = SCSI_PROTOCOL_UAS;
>
> How do you know this is UAS? It could as well be usb-storage, no?
> Maybe we need not differentiate the two, but asserting UAS here
> might raise wrong expectations. Maybe you should just call it
> SCSI_PROTOCOL_USB.
>
> > tgtname = udev_device_get_sysname(tgtdev);
> > strlcpy(node, tgtname, NODE_NAME_SIZE);
> > - condlog(3, "%s: skip USB device %s", pp->dev,
> > node);
> > - return 1;
> > + return 0;
> > }
> > tgtdev = udev_device_get_parent(tgtdev);
> > }
> > @@ -2136,6 +2135,14 @@ int pathinfo(struct path *pp, struct config
> > *conf, int mask)
> >
> > if (rc != PATHINFO_OK)
> > return rc;
> > +
> > + if (pp->bus == SYSFS_BUS_SCSI &&
> > + pp->sg_id.proto_id == SCSI_PROTOCOL_UAS &&
> > + !conf->allow_usb_devices) {
> > + condlog(3, "%s: skip USB device %s", pp->dev,
> > + pp->tgt_node_name);
> > + return PATHINFO_SKIPPED;
> > + }
> > }
> >
> > if (mask & DI_BLACKLIST && mask & DI_SYSFS) {
> > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> > index 4afd3e88..284c1e45 100644
> > --- a/libmultipath/structs.h
> > +++ b/libmultipath/structs.h
> > @@ -174,6 +174,7 @@ enum scsi_protocol {
> > SCSI_PROTOCOL_SAS = 6,
> > SCSI_PROTOCOL_ADT = 7, /* Media Changers */
> > SCSI_PROTOCOL_ATA = 8,
> > + SCSI_PROTOCOL_UAS = 9, /* USB Attached SCSI */
> > SCSI_PROTOCOL_UNSPEC = 0xf, /* No specific protocol */
> > };
> >
> > diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> > index 5adaced6..21b3bfb6 100644
> > --- a/multipath/multipath.conf.5
> > +++ b/multipath/multipath.conf.5
> > @@ -643,6 +643,20 @@ The default is: in
> > \fB/sys/block/sd<x>/device/timeout\fR
> > .
> > .
> > .TP
> > +.B allow_usb_devices
> > +If set to
> > +.I no
> > +, all USB devices will be skipped during path discovery. This is
> > convenient
> > +for most users, as it effectively hides all attached USB disks and
> > flash
> > +drives from the multipath application. However, if you intend to use
> > multipath
> > +on USB attached devices, set this to \fIyes\fR.
> > +.RS
> > +.TP
> > +The default is: \fBno\fR
>
> All factual information in the middle sentence ("This is convenient ...
> application") is already present elsewhere. Drop the sentence, please,
> and drop the "however," too.
>
> Martin
>
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
