On 26.10.22 17:42, Kui Liu wrote:
-----Original Message-----
From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
Date: Wednesday, 26 October 2022 at 9:32 PM
To: Kui Liu <kui....@acronis.com>, Konstantin Khorenko <khore...@virtuozzo.com>,
Alexey Kuznetsov <kuz...@acronis.com>
Cc: Devel <devel@openvz.org>
Subject: Re: [Devel] [PATCH RH9] dm-ploop: port the standby mode feature.
On 26.10.22 16:15, Kui Liu wrote:
>
>
> -----Original Message-----
> From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
> Date: Wednesday, 26 October 2022 at 5:14 PM
> To: Kui Liu <kui....@acronis.com>, Konstantin Khorenko
<khore...@virtuozzo.com>, Alexey Kuznetsov <kuz...@acronis.com>
> Cc: Devel <devel@openvz.org>
> Subject: Re: [Devel] [PATCH RH9] dm-ploop: port the standby mode feature.
>
> Hi,
>
> On 21.10.22 16:28, Kui Liu wrote:
> >
> >
> > -----Original Message-----
> > From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
> > Date: Friday, 21 October 2022 at 7:35 PM
> > To: Kui Liu <kui....@acronis.com>, Konstantin Khorenko
<khore...@virtuozzo.com>, Alexey Kuznetsov <kuz...@acronis.com>
> > Cc: Devel <devel@openvz.org>
> > Subject: Re: [Devel] [PATCH RH9] dm-ploop: port the standby mode
feature.
> >
> > On 21.10.22 14:28, Kui Liu wrote:
> > > > > Ok - for the user space there are two
options:
> > > > > 1. add an argument to enable at
creation time
> > > > > 2. command standby_enable /if using
the enable it is possible to
> > > > > implement a disable command too/.
> > > > >
> > > > > I prefer the second approach but let's
hear other opinions too.
> > > > > And yes we can possibly do both but is
there a use case ?
> > > > >
> > > > > [LIU]: Since the static key is a global
option, I also think the second approach is better.
> > > > > In case of nodes with vStorage + ploop +
iSCSI setup, we just need to enable the static
> > > > > key early in system init or iSCSI service
init. I don't think it is necessary to implement both.
> > > >
> > > > You are right. It will need at least one ploop
device created to issue a
> > > > device mapper command to enable standby. Would
a module parameter work?
> > > >
> > > > [LIU] Yes, I think it is better to make the static
key a module parameter.
> > >
> > > This rised a question - how and who will pass the
module parameter when
> > > required ? How we will handle the existin users ?
What do you think ?
> > >
> > > [LIU]: Like add a conf file ploop.conf in /etc/modprobe.d
or lib/modeprobe.d? This conf file can be shipped with the release image, or installed
later. Are you referring to VZ7 as existing users? I saw you also made the changes to
VZ7, but I'm not sure whether it is necessary to backport these changes to VZ7.
> >
> > Yes, VZ7 users - there are issues related to it so trying to
fix them.
> > the modprobe.d would do but do you have a package that can
install the
> > conf tied to the iSCSI target, so it gets installed only if
the iSCSI is
> > used ?
> >
> > [LIU]: Yes, we do build the our own SCST package, so we can
install the conf from there.
>
> After discussion turns out module_param is not an option - package
is
> always installed so it will enable the key always which is not what
we want.
>
> To solve this we can
> struct static_key ploop_standby_check = STATIC_KEY_INIT_FALSE;
> +EXPORT_SYMBOL(ploop_standby_check)
>
> and the module can enable it at use or init time.
>
> static int scst_vstor_pr_init(struct scst_device *dev, const char
> *cl_dev_id)
>
> looks like the place to enable the static key and set the _en flag
on
> the queue.
>
> _en flag is a MUST static key is a SHOULD - where do you think is
the
> best place to implement them in the scst code?
>
> [LIU]: This would introduce explicit dependency between SCST and ploop
module, which I feel is inappropriate .
> Can we instead export the static key via a sysfs file, say
/sys/kernel/ploop/ploop_standby_check?
We can export it via sysfs or proc but who and how will enable it thru
there ?
[LIU] Well, I think we can enable it in Pre-/ On- / Post- start of
vstorage-target-manager.service.
The point is once it is exported to userspace, when and who to enable it
shouldn't be a big issue.
Since QUEUE_FLAG_STANDBY_EN will make the scst module depend on the
kernel. It would be easier to leave setting in the kernel. And not make
it create userspace dependencies which may cause trouble when upgrading.
The key can be moved in core to avoid depending on another module.
--
Regards,
Alexander Atanasov
--- scst/src/scst_vstor.c.orig 2022-10-26 13:55:26.544317020 +0300
+++ scst/src/scst_vstor.c 2022-10-27 11:15:07.568776117 +0300
@@ -886,7 +886,14 @@ static int scst_vstor_pr_init(struct scs
dev->pr_vstor = pr_vstor;
pr_vstor->pr_dirty = 1;
res = 0;
-
+#ifdef QUEUE_FLAG_STANDBY_EN
+ if (dev->scsi_dev) {
+ extern struct static_key ploop_standby_check;
+ queue_flag_set(QUEUE_FLAG_STANDBY_EN,
dev->scsi_dev->request_queue);
+ static_key_slow_inc(&ploop_standby_check);
+ PRINT_INFO("Enabled:ploop standby check");
+ }
+#endif
out:
return res;
}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
