-----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. -- Regards, Alexander Atanasov _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel