[lvm-devel] [LVM2 RFCv1 4/5] lib: locking: Parse PV list for IDM locking
Leo Yan
leo.yan at linaro.org
Thu Apr 29 03:12:09 UTC 2021
On Wed, Apr 28, 2021 at 02:39:27PM -0500, David Teigland wrote:
> On Sun, Apr 25, 2021 at 10:22:40AM +0800, Leo Yan wrote:
> > +static void _lockd_retrive_lv_pv_list(struct volume_group *vg,
> > + const char *lv_name,
> > + struct lvmlockd_pvs *lock_pvs)
> > +{
>
> It looks like this wants a list of PVs (names) used by the LV. Try
> iterating through all PVs in the VG and using the existing lv_is_on_pv()
> function to check if the LV is using that PV, e.g.
>
> for each pv in vg->pvs,
> if (lv_is_on_pv(lv, pv))
> copy the pv name;
Using lv_is_on_pv() is much better. I read a bit for the code, except
it checks the LV itself is on the PV or not, it also checks sub LVs
(like metadata, pool, etc). Will fix it.
> You could pass the lv struct through to here, or use find_lv(vg, lv_name)
> to get it again here.
>
> > @@ -251,7 +485,16 @@ static int _lockd_request(struct cmd_context *cmd,
> > if (vg_name && lv_name) {
> > - reply = _lockd_send(req_name,
> > + /*
> > + * For LV operation, the PV list must be passed for idm,
> > + * otherwise, IDM lock manager has no idea to send locking
> > + * request to which drives, so return failure.
> > + */
> > + if (!lock_pvs)
> > + return 1;
> > +
> > + reply = _lockd_send_with_pvs(req_name,
>
> Requires other lock managers to include lock_pvs?
No, will change code to only pass "lock_pvs" for IDM; for other
locking schemes, will assign NULL pointer to "lock_pvs".
> > + /*
> > + * Create the VG's PV list when start the VG, the PV list
> > + * is passed to lvmlockd, and the the PVs path will be used
> > + * to send SCSI commands for idm locking scheme.
> > + */
>
> if vg->lock_type is IDM before creating pv_list?
Yes, IIUC, when start the VG, "vg->lock_type" has been assigned to the
locking scheme which it's using; for IDM locking scheme, "vg->lock_type"
is "idm" before creating pv_list.
> > + _lockd_retrive_vg_pv_list(vg, &lock_pvs);
> > +
> > + reply = _lockd_send_with_pvs("start_vg",
> > + &lock_pvs,
>
> and probably NULL instead of &lock_pvs for non-IDM.
Will do.
Will follow up the comments in next patch set, thanks for reviewing!
Leo
More information about the lvm-devel
mailing list