<font size=2>Hello Ben,</font>
<br>
<br><font size=2>I know what you concern about</font><font size=2 face="Times New Roman">.</font><font size=2>
Maybe we can change the "wwid" member</font>
<br><font size=2>in "struct uevent" to another name such as "merge_id"
to only </font>
<br><font size=2>identify which uevents can merge together. The value of
"merge_id" can</font>
<br><font size=2>set by "ID_SERIAL" or "ID_UID" in
uevent. If we get the "merge_id"</font>
<br><font size=2>from uevents, we can merge and process it as merging way,
otherwise</font>
<br><font size=2>we process it as old way (processed one by one).</font>
<br>
<br><font size=2>And we revert this patch:</font>
<br><font size=2>"[dm-devel] [PATCH 12/12] libmultipath: use existing
wwid when wwid</font>
<br><font size=2>has already been existed in uevent"</font>
<br><font size=2>thus wwid of each path will get by methods of configuration
as old</font>
<br><font size=2>ways.</font>
<br>
<br><font size=2>The premise is that, devices whose "merge_id"
is the same have the</font>
<br><font size=2>same "wwid". I think this premise can be established.</font>
<br>
<br><font size=2>how do you think about the above proposal?</font>
<br>
<br><font size=2>> 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.</font>
<br><font size=2>I think it is ok, but a little complex. and if we can
get rid of the </font>
<br><font size=2>"wwid" issue, we do not need to do so.</font>
<br>
<br><font size=2>Regards</font>
<div><font size=2>Tang Junhui</font>
<br>
<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">tang.wenjun3@zte.com.cn,
zhang.kai16@zte.com.cn, dm-devel@redhat.com, bart.vanassche@sandisk.com,
mwilck@suse.com</font>
<br><font size=1 color=#5f5f5f face="sans-serif">日期:
</font><font size=1 face="sans-serif">2017/01/05
02:23</font>
<br><font size=1 color=#5f5f5f face="sans-serif">主题:
</font><font size=1 face="sans-serif">Re: [dm-devel]
[PATCH 01/12] libmultipath: add wwid for "struct uevent" to record
wwid of uevent</font>
<br><font size=1 color=#5f5f5f face="sans-serif">发件人:
</font><font size=1 face="sans-serif">dm-devel-bounces@redhat.com</font>
<br>
<hr noshade>
<br>
<br>
<br><tt><font size=2>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 wwid<br>
> > based on any udev environment variable (or even
use a callout, although<br>
> > this is deprecated). With this patch, that breaks.<br>
> Does WWID obtained by different methods change? If it
changes, we 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 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 merge<br>
> rbd uevents for these devices, we can add code to get
wwid from 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 uevent"<br>
> to record wwid of uevent<br>
> <br>
> --------------------------------------------------------------------------<br>
> <br>
> On Tue, Dec 27, 2016 at 04:03:18PM +0800, tang.junhui@zte.com.cn
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 wwid<br>
> based on any udev environment variable (or even use a
callout, 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 if<br>
> this change doesn't break any devices in their default
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") == 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>
</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>
<br></div>