-----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

Reply via email to