[Devel] [PATCH RH9] dm-ploop: port the standby mode feature.

[Kui Liu]

-----Original Message-----
From: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
Date: Wednesday, 26 October 2022 at 9:32 PM
To: Kui Liu <Kui.Liu at acronis.com>, Konstantin Khorenko <khorenko at virtuozzo.com>, Alexey Kuznetsov <kuznet at acronis.com>
Cc: Devel <devel at 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 at virtuozzo.com>
    > Date: Wednesday, 26 October 2022 at 5:14 PM
    > To: Kui Liu <Kui.Liu at acronis.com>, Konstantin Khorenko <khorenko at virtuozzo.com>, Alexey Kuznetsov <kuznet at acronis.com>
    > Cc: Devel <devel at 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 at virtuozzo.com>
    >      > Date: Friday, 21 October 2022 at 7:35 PM
    >      > To: Kui Liu <Kui.Liu at acronis.com>, Konstantin Khorenko <khorenko at virtuozzo.com>, Alexey Kuznetsov <kuznet at acronis.com>
    >      > Cc: Devel <devel at 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. 

    -- 
    Regards,
    Alexander Atanasov