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?
 --
Regards,
Alexander Atanasov

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to