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

[Alexander Atanasov]

On 19.10.22 11:25, Kui Liu wrote:
> 
> 
> -----Original Message-----
> From: Alexander Atanasov <alexander.atanasov at virtuozzo.com>
> Date: Wednesday, 19 October 2022 at 4:02 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 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 ?


-- 
Regards,
Alexander Atanasov