<tt><font size=2>Hello Ben,</font></tt>
<br>
<br><tt><font size=2>> > The premise is
that, devices whose "merge_id" is the same have the<br>
> > same "wwid". I think this
premise can be established.<br>
<br>
> So, if you have device where the merge_id doesn't equal
the wwid, you<br>
> call domap on that device, as it it wasn't part of a
merged set?</font></tt>
<br>
<br><tt><font size=2>Maybe I did not say clearly. "merge_id"
is only used for uevents merging, and</font></tt>
<br><tt><font size=2>it has nothing to do with "wwid". I only
primse devices whose "merge_id" are </font></tt>
<br><tt><font size=2>same have same "wwid" (Sorry, maybe </font></tt><font size=2>I
had a misleading statement</font><tt><font size=2> in previous email).
</font></tt>
<br><tt><font size=2>"merge_id" and "wwid" can be same
or not same. The merged uevents are still </font></tt>
<br><tt><font size=2>processed as bellow patch I send previous:</font></tt>
<br><font size=2>[dm-devel] [PATCH 11/12] multipathd: proccess merged uevents</font>
<br>
<br>
<br><font size=2>Regards,</font>
<br><font size=2>Tang Junhui</font>
<br>
<br>
<br>
<br>
<br><font size=1 color=#5f5f5f face="sans-serif">发件人:
</font><font size=1 face="sans-serif">"Benjamin
Marzinski" <bmarzins@redhat.com></font>
<br><font size=1 color=#5f5f5f face="sans-serif">收件人:
</font><font size=1 face="sans-serif">tang.junhui@zte.com.cn,
</font>
<br><font size=1 color=#5f5f5f face="sans-serif">抄送:
</font><font size=1 face="sans-serif">mwilck@suse.com,
bart.vanassche@sandisk.com, dm-devel@redhat.com, dm-devel-bounces@redhat.com,
tang.wenjun3@zte.com.cn, zhang.kai16@zte.com.cn</font>
<br><font size=1 color=#5f5f5f face="sans-serif">日期:
</font><font size=1 face="sans-serif">2017/01/06
01:37</font>
<br><font size=1 color=#5f5f5f face="sans-serif">主题:
</font><font size=1 face="sans-serif">Re: Re: [dm-devel]
[PATCH 01/12] libmultipath: add wwid for "struct uevent" to record
wwid of uevent</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>On Thu, Jan 05, 2017 at 11:10:43AM +0800, tang.junhui@zte.com.cn
wrote:<br>
> Hello Ben,<br>
> <br>
> I know what you concern about. Maybe we can change the
"wwid" member<br>
> in "struct uevent" to another name such as
"merge_id" to only<br>
> identify which uevents can merge together. The value
of "merge_id" can<br>
> set by "ID_SERIAL" or "ID_UID" in
uevent. If we get the "merge_id"<br>
> from uevents, we can merge and process it as merging
way, otherwise<br>
> we process it as old way (processed one by one).<br>
<br>
<br>
> And we revert this patch:<br>
> "[dm-devel] [PATCH 12/12] libmultipath: use existing
wwid when wwid<br>
> has already been existed in uevent"<br>
> thus wwid of each path will get by methods of configuration
as old<br>
> ways.<br>
> <br>
> The premise is that, devices whose "merge_id"
is the same have the<br>
> same "wwid". I think this premise can be established.<br>
<br>
So, if you have device where the merge_id doesn't equal the wwid, you<br>
call domap on that device, as it it wasn't part of a merged set?<br>
<br>
That would work, but you would have to be careful to deal with the case<br>
where the last uevent in the set, the one that would be used to call<br>
domap for the whole merged set, suddenly gets processed all on it's own,<br>
because it had a different wwid from the rest of the set. In that case<br>
You would need to make sure that something called domap on the other<br>
members of the merged set. Now, I admit that this is a very unlikely<br>
thing to happen.<br>
<br>
I thought about proposing that we revert patch 12, but it does add<br>
complexity if you find that only some of your merged devices change<br>
wwid. Also, if all of them change to the same new wwid, you just skipped<br>
merging when you should be able to. But I'm not against reverting it, as<br>
long as we handle everything o.k. Reverting it would mean that we don't<br>
have to worry about removing a capability that multipath previously had.<br>
There's just a trade-off between supporting a capability (that we don't<br>
know if anyone uses), and maintaining less complex code.<br>
<br>
As long as it is possible to configure what uevent variable you check<br>
for by device type, I don't know of a situation where this will cause<br>
real world problems. It's possible that some users are changing<br>
uid_attribute for only certain scsi-based arrays, but I don't know of<br>
any cases. If somebody reading this knows of any cases, please speak up.<br>
But I would like to have the variable you check be configurable so that<br>
it's flexible with regards to new device types or the possiblity of<br>
differing uevent variable names either between distributions or over<br>
time. Does that sound reasonable?<br>
<br>
-Ben<br>
<br>
> <br>
> how do you think about the above proposal?<br>
> <br>
> > The other option would be to not actually
merge the uevents, but<br>
> simply<br>
> > run through the filtered but unmerged
list of uevents, and skip the<br>
> > domap stuff but remember the maps that
need pushing to device-mapper.<br>
> > Once you are done processing all the
uevents, except for updating the<br>
> > maps in device-mapper, you go back
and update all the maps that need<br>
> > updating. There's more code refactoring
in this approach, but it<br>
> keeps<br>
> > the uid being set in pathinfo, where
you have all the information<br>
> > necessary to set it using uid_attribute,
getuid, or specialized code<br>
> > like rbd uses.<br>
> I think it is ok, but a little complex. and if we can
get rid of the<br>
> "wwid" issue, we do not need to do so.<br>
> <br>
> Regards<br>
> Tang Junhui<br>
> <br>
> 发件人: "Benjamin
Marzinski" <bmarzins@redhat.com><br>
> 收件人: tang.junhui@zte.com.cn,<br>
> 抄送: tang.wenjun3@zte.com.cn,
zhang.kai16@zte.com.cn,<br>
> dm-devel@redhat.com, bart.vanassche@sandisk.com, mwilck@suse.com<br>
> 日期: 2017/01/05 02:23<br>
> 主题: Re: [dm-devel] [PATCH
01/12] libmultipath: add wwid for<br>
> "struct uevent" to record wwid of uevent<br>
> 发件人: dm-devel-bounces@redhat.com<br>
> <br>
> --------------------------------------------------------------------------<br>
> <br>
> On Wed, Jan 04, 2017 at 02:56:46PM +0800, tang.junhui@zte.com.cn
wrote:<br>
> > Hello Ben,<br>
> ><br>
> > > Right now, multipath users are
allowed configure devices to set the<br>
> wwid<br>
> > > based on any udev environment
variable (or even use a callout,<br>
> although<br>
> > > this is deprecated). With this
patch, that breaks.<br>
> > Does WWID obtained by different methods
change? If it changes, we<br>
> would<br>
> > better to modify code to keep it no
change.<br>
> <br>
> Yes. users can pick any udev environment variable (and
currently, any<br>
> callout program that they write) to use as the wwid.<br>
> <br>
> > > If the udev sets<br>
> > > ID_SERIAL for a device, that is
its wwid, right?<br>
> > Yes<br>
> ><br>
> > > Do you know if rbd<br>
> > > devices have ID_SERIAL set?<br>
> > WWID has different label in uevents
for different devices, I only<br>
> test for<br>
> > SCSI devices.<br>
> <br>
> Where is that check? I only see a check for whether ID_SERIAL
exists. If<br>
> it exists on things that are not scsi devices, you will
set the wwid for<br>
> these devices with it as well, even if it doesn't work
for them.<br>
> <br>
> > Now we do not support rbd divice for
uevents merging, these<br>
> > device process as old way, it has no
harm in logic. If we need to<br>
> merge<br>
> > rbd uevents for these devices, we can
add code to get wwid from<br>
> uevents<br>
> > and it can be supported easily.<br>
> <br>
> Look at get_rbd_uid(), and how it's called. rbd devices
don't even use<br>
> the getuid callout or uid_attribute. They use special
code called by<br>
> get_uid.<br>
> <br>
> Now you could add explicit checks so we only check ID_SERIAL
for scsi<br>
> devices, ID_UID for dasd devices, and never set the wwid
otherwise. You<br>
> could even make the attribute we check configurable by
device type with<br>
> an option like<br>
> <br>
> uid_attrs "sd:ID_SERIAL dasd:ID_UID"<br>
> <br>
> in the defaults section, that would set up mappings between
device types<br>
> and uevent attributes to check for the uid. But this
would be on per<br>
> device types, not per storage array, like it currently
is. uid_attribute<br>
> and getuid attribute would only ever be used for device
types that<br>
> weren't in the uid_attrs list.<br>
> <br>
> The other option would be to not actually merge the uevents,
but simply<br>
> run through the filtered but unmerged list of uevents,
and skip the<br>
> domap stuff but remember the maps that need pushing to
device-mapper.<br>
> Once you are done processing all the uevents, except
for updating the<br>
> maps in device-mapper, you go back and update all the
maps that need<br>
> updating. There's more code refactoring in this approach,
but it keeps<br>
> the uid being set in pathinfo, where you have all the
information<br>
> necessary to set it using uid_attribute, getuid, or specialized
code<br>
> like rbd uses.<br>
> <br>
> As long as we make sure that we are only checking specific
uevent<br>
> attributes for specific device types, I'm o.k. with your
way, but we are<br>
> losing flexibility that multipath has always had in regards
to setting<br>
> the wwid. I want to point that out so that anyone who
needs this knows<br>
> that it is changing.<br>
> <br>
> -Ben<br>
> <br>
> > Regards<br>
> > Tang Junhui<br>
> ><br>
> > ������:
"Benjamin Marzinski" <bmarzins@redhat.com><br>
> > �ռ���:
tang.junhui@zte.com.cn,<br>
> > ����:
christophe.varoqui@opensvc.com, hare@suse.de,<br>
> > mwilck@suse.com, bart.vanassche@sandisk.com,
dm-devel@redhat.com,<br>
> > zhang.kai16@zte.com.cn, tang.wenjun3@zte.com.cn<br>
> > ����:
2017/01/04 06:03<br>
> > ����:
Re: [PATCH 01/12] libmultipath: add wwid for "struct<br>
> uevent"<br>
> > to record wwid of uevent<br>
> ><br>
> > <br>
> --------------------------------------------------------------------------<br>
> ><br>
> > On Tue, Dec 27, 2016 at 04:03:18PM
+0800, tang.junhui@zte.com.cn<br>
> wrote:<br>
> > > From: tang.junhui <tang.junhui@zte.com.cn><br>
> > ><br>
> > > Add "char *wwid" to
point WWID of uevent. This member identifies<br>
> > > the LUN ID which the path belongs
to, and it is used for merging<br>
> > > uevents. WWID possibly did not
exist in uevent yet, so ->wwid<br>
> > > would be NULL, those uevents would
not be merged, but be proccessed<br>
> > > as old way.<br>
> ><br>
> > Right now, multipath users are allowed
configure devices to set the<br>
> wwid<br>
> > based on any udev environment variable
(or even use a callout,<br>
> although<br>
> > this is deprecated). With this patch,
that breaks. If the udev sets<br>
> > ID_SERIAL for a device, that is its
wwid, right? Do you know if rbd<br>
> > devices have ID_SERIAL set? If so,
this change will break them. Even<br>
> if<br>
> > this change doesn't break any devices
in their default<br>
> configurations,<br>
> > we would need to disallow changing
how the wwid is set for this patch<br>
> > to be safe.<br>
> ><br>
> > -Ben<br>
> ><br>
> > ><br>
> > > Change-Id: Ie6b076363b3735dc7de10184b27fa799b499af0e<br>
> > > Signed-off-by: tang.junhui <tang.junhui@zte.com.cn><br>
> > > ---<br>
> > > libmultipath/uevent.c |
2 ++<br>
> > > libmultipath/uevent.h |
1 +<br>
> > > 2 files changed, 3 insertions(+)<br>
> > ><br>
> > > diff --git a/libmultipath/uevent.c
b/libmultipath/uevent.c<br>
> > > index 7edcce1..ef1bafe 100644<br>
> > > --- a/libmultipath/uevent.c<br>
> > > +++ b/libmultipath/uevent.c<br>
> > > @@ -424,6 +424,8 @@ struct uevent
*uevent_from_udev_device(struct<br>
> > udev_device *dev)<br>
> > >
uev->devpath
=<br>
> > uev->envp[i] + 8;<br>
> > >
if (strcmp(name, "ACTION") == 0)<br>
> > >
uev->action
=<br>
> > uev->envp[i] + 7;<br>
> > > +
if (strcmp(name, "ID_SERIAL") ==<br>
> 0)<br>
> > > +
uev->wwid
=<br>
> > uev->envp[i] + 10;<br>
> > >
i++;<br>
> > >
if (i == HOTPLUG_NUM_ENVP - 1)<br>
> > >
break;<br>
> > > diff --git a/libmultipath/uevent.h
b/libmultipath/uevent.h<br>
> > > index 9d22dcd..7bfccef 100644<br>
> > > --- a/libmultipath/uevent.h<br>
> > > +++ b/libmultipath/uevent.h<br>
> > > @@ -22,6 +22,7 @@ struct uevent
{<br>
> > >
char *devpath;<br>
> > >
char *action;<br>
> > >
char *kernel;<br>
> > > +
char *wwid;<br>
> > >
unsigned long seqnum;<br>
> > >
char *envp[HOTPLUG_NUM_ENVP];<br>
> > > };<br>
> > > --<br>
> > > 2.8.1.windows.1<br>
> > ><br>
> <br>
> --<br>
> dm-devel mailing list<br>
> dm-devel@redhat.com<br>
> [1]</font></tt><a href="https://www.redhat.com/mailman/listinfo/dm-devel"><tt><font size=2>https://www.redhat.com/mailman/listinfo/dm-devel</font></tt></a><tt><font size=2><br>
> <br>
> References<br>
> <br>
> Visible links<br>
> 1. </font></tt><a href="https://www.redhat.com/mailman/listinfo/dm-devel"><tt><font size=2>https://www.redhat.com/mailman/listinfo/dm-devel</font></tt></a><tt><font size=2><br>
</font></tt>
<br>