-----Original Message-----
From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
Date: Thursday, 27 October 2022 at 4:28 PM
To: Kui Liu <kui....@acronis.com>, Konstantin Khorenko
<khore...@virtuozzo.com>, Alexey Kuznetsov <kuz...@acronis.com>, "Denis V.
Lunev" <d...@virtuozzo.com>
Cc: Devel <devel@openvz.org>
Subject: Re: [Devel] [PATCH RH9] dm-ploop: port the standby mode feature.
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;
}
[LIU]: OK, then move the key in core, I'll modify SCST code to enable it,
though not in scst_vstor_pr_init().
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
