-----Original Message-----
From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
Date: Friday, 21 October 2022 at 6:08 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 19.10.22 11:25, Kui Liu wrote:
>
>
> -----Original Message-----
> From: Alexander Atanasov <alexander.atana...@virtuozzo.com>
> Date: Wednesday, 19 October 2022 at 4:02 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 19.10.22 9:58, Kui Liu wrote:
> >
> >
> > > >
> > > > + if
(blk_queue_standby_en(ploop_blk_queue(ploop))) {
> > > > +
blk_queue_flag_clear(QUEUE_FLAG_STANDBY, ploop_blk_queue(ploop));
> > > > + if
(!static_key_enabled(&ploop_standby_check)) {
> > > > +
static_key_enable(&ploop_standby_check);
> > > > + pr_info("ploop: standby
mode check enabled\n");
> > > > + }
> > > > + }
> > > > +
> > > >
> > > > [LIU]: This may not work, because normally a ploop
device is created first before being attached to SCST, which means 'ploop_ctr'
is called before QUEUE_FLAG_STANDBY_EN can be set. We need to put "
static_key_enable(&ploop_standby_check))" somewhere else.
> > >
> > > I am not sure i undestand that correctly but dm-ploop
maps the ploop
> > > device over existing device. It first opens the files
that are on the
> > > existing device then passes the fds to dm-ploop. To do
this the target
> > > must be up and running. device mapper setups the table
before calling
> > > the ploop ctr. You already checked the queue lifecycle.
> > > Is there any other precodition for the target to set
the EN flag?
> > >
> > >
> > > There is one more alternative to pass the stanby
enable flag enable from
> > > userspace .
> > >
> > > [LIU]: well, you are right about the dm-ploop setup
process. However SCST can only set the EN flag after the ploop device is
assigned to
> > > SCST, before which the table could have been loaded. So if
setup sequence is 1) create ploop device -> 2) load table -> 3) assign ploop
> > > device to SCST, then the outer if condition won't be true,
hence not triggering static_key_enable().
> > > My concern is this should not be the only place where the
static key can be enabled. Now that there is one more alternative in user space
> > > , above code is OK.
> >
> > 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.
--
Regards,
Alexander Atanasov
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
