[prev in list] [next in list] [prev in thread] [next in thread]
List: openvz-devel
Subject: Re: [Devel] [PATCH RH9] dm-ploop: port the standby mode feature.
From: Kui Liu <Kui.Liu () acronis ! com>
Date: 2022-10-27 8:58:14
Message-ID: E2A4A7ED-5159-440F-A4A0-22E228165642 () acronis ! com
[Download RAW message or body]
-----Original Message-----
From: Alexander Atanasov <alexander.atanasov@virtuozzo.com>
Date: Thursday, 27 October 2022 at 4:28 PM
To: Kui Liu <Kui.Liu@acronis.com>, Konstantin Khorenko <khorenko@virtuozzo.com>, \
Alexey Kuznetsov <kuznet@acronis.com>, "Denis V. Lunev" \
<den@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.atanasov@virtuozzo.com>
> Date: Wednesday, 26 October 2022 at 9:32 PM
> To: Kui Liu <Kui.Liu@acronis.com>, Konstantin Khorenko \
<khorenko@virtuozzo.com>, Alexey Kuznetsov <kuznet@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.atanasov@virtuozzo.com>
> > Date: Wednesday, 26 October 2022 at 5:14 PM
> > To: Kui Liu <Kui.Liu@acronis.com>, Konstantin Khorenko \
<khorenko@virtuozzo.com>, Alexey Kuznetsov <kuznet@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.atanasov@virtuozzo.com>
> > > Date: Friday, 21 October 2022 at 7:35 PM
> > > To: Kui Liu <Kui.Liu@acronis.com>, Konstantin Khorenko \
<khorenko@virtuozzo.com>, Alexey Kuznetsov <kuznet@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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic