Re: [PATCH] storage: Linstor support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 18, 2021 at 02:12:04PM +0100, Peter Krempa wrote:
> On Fri, Jan 15, 2021 at 08:16:47 +0100, Rene Peinthor wrote:
> > Implement a LINSTOR backend storage driver.
> > The Linstor client needs to be installed and it needs to be configured
> > on the nodes used by the controller.
> > 
> > It supports most pool/vol commands, except for pool-build/pool-delete
> > and provides a block device in RAW file mode.
> > Linstor supports more than just DRBD so it would also be possible to have
> > it provide LVM, ZFS or NVME volumes, but the common case will be to provide
> > DRBD volumes in a cluster.
> > 
> > Sample pool XML:
> > <pool type='linstor'>
> >   <name>linstor</name>
> >   <source>
> >     <host name='ubuntu-focal-60'/>
> >     <name>libvirtgrp</name>
> >   </source>
> > </pool>
> > 
> > <pool/source/name> element must point to an already created LINSTOR
> > resource-group, which is used to spawn resources/volumes.
> > <pool/source/host@name> attribute should be the local linstor node name,
> > if missing it will try to get the hosts uname and use that instead.
> > 
> > Result volume XML sample:
> > <volume type='block'>
> >   <name>alpine12</name>
> >   <key>libvirtgrp/alpine12</key>
> >   <capacity unit='bytes'>5368709120</capacity>
> >   <allocation unit='bytes'>5540028416</allocation>
> >   <target>
> >     <path>/dev/drbd1000</path>
> >     <format type='raw'/>
> >   </target>
> > </volume>
> > 
> > Signed-off-by: Rene Peinthor <rene.peinthor@xxxxxxxxxx>
> > ---
> 
> Firstly I'd like you to split the patch into more granular commits as
> it's customary in our project ...
> 
> >  docs/schemas/storagepool.rng                  |  27 +
> >  docs/storage.html.in                          |  39 +
> >  include/libvirt/libvirt-storage.h             |   1 +
> >  meson.build                                   |   6 +
> >  meson_options.txt                             |   1 +
> >  po/POTFILES.in                                |   1 +
> >  src/conf/domain_conf.c                        |   1 +
> >  src/conf/storage_conf.c                       |  14 +-
> >  src/conf/storage_conf.h                       |   1 +
> >  src/conf/virstorageobj.c                      |   4 +-a
> 
> Conf changes are usually separate
> 
> >  src/storage/meson.build                       |  25 +
> >  src/storage/storage_backend.c                 |   6 +
> >  src/storage/storage_backend_linstor.c         | 803 ++++++++++++++++++
> >  src/storage/storage_backend_linstor.h         |  23 +
> >  src/storage/storage_backend_linstor_priv.h    |  53 ++
> >  src/storage/storage_driver.c                  |   1 +
> 
> Implementation should also be separate
> 
> >  src/test/test_driver.c                        |   1 +
> >  tests/linstorjsondata/broken.json             |   1 +
> >  tests/linstorjsondata/resource-group.json     |   1 +
> >  .../linstorjsondata/resource-list-test2.json  | 332 ++++++++
> >  .../storage-pools-ssdpool.json                |  72 ++
> >  tests/linstorjsondata/storage-pools.json      | 192 +++++
> >  tests/linstorjsondata/volume-def-list.json    | 158 ++++
> >  .../volume-definition-test2.json              |   1 +
> >  tests/meson.build                             |   6 +
> >  tests/storagebackendlinstortest.c             | 371 ++++++++
> >  .../storagepoolcapsschemadata/poolcaps-fs.xml |   7 +
> >  .../poolcaps-full.xml                         |   7 +
> >  tests/storagepoolxml2argvtest.c               |   1 +
> >  tests/storagepoolxml2xmlin/pool-linstor.xml   |   8 +
> >  tests/storagevolxml2xmlin/vol-linstor.xml     |  10 +
> 
> Placing tests depends. Usually XML related tests go with the commits
> implementing the parser/formatter or follow that commit.
> 
> Other tests should be added separately.
> 
> >  tools/virsh-pool.c                            |   3 +
> 
> This is adding the support for the new type so it goes where the type is
> declared
> 
> >  32 files changed, 2175 insertions(+), 2 deletions(-)
> 
> [...]
> 
> > diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng
> > index bd24b8b8d0..9b163e611d 100644
> > --- a/docs/schemas/storagepool.rng
> > +++ b/docs/schemas/storagepool.rng
> > @@ -26,6 +26,7 @@
> >          <ref name="poolgluster"/>
> >          <ref name="poolzfs"/>
> >          <ref name="poolvstorage"/>
> > +        <ref name="poollinstor"/>
> >        </choice>
> >      </element>
> >    </define>
> > @@ -224,6 +225,21 @@
> >      </interleave>
> >    </define>
> >  
> > +  <define name="poollinstor">
> > +    <attribute name="type">
> > +      <value>linstor</value>
> > +    </attribute>
> > +    <interleave>
> > +      <ref name="commonMetadataNameOptional"/>
> > +      <ref name="sizing"/>
> > +      <ref name="features"/>
> > +      <ref name="sourcelinstor"/>
> > +      <optional>
> > +        <ref name="target"/>
> > +      </optional>
> > +    </interleave>
> > +  </define>
> > +
> >    <define name="sourceinfovendor">
> >      <interleave>
> >        <optional>
> > @@ -463,6 +479,17 @@
> >      </element>
> >    </define>
> >  
> > +  <define name="sourcelinstor">
> > +    <element name="source">
> > +      <interleave>
> > +        <ref name="sourceinfoname"/>
> > +        <optional>
> > +          <ref name="sourceinfohost"/>
> > +        </optional>
> > +      </interleave>
> > +    </element>
> > +  </define>
> > +
> >    <define name="sourcefmtfs">
> >      <optional>
> >        <element name="format">
> > diff --git a/docs/storage.html.in b/docs/storage.html.in
> > index b2cf343933..9130fbd180 100644
> > --- a/docs/storage.html.in
> > +++ b/docs/storage.html.in
> > @@ -829,5 +829,44 @@
> >  
> >      <h3>Valid volume format types</h3>
> >      <p>The valid volume types are the same as for the directory pool.</p>
> > +
> > +
> > +    <h2><a id="StorageBackendLINSTOR">LINSTOR pool</a></h2>
> > +    <p>
> > +      This provides a pool using the LINSTOR software-defined-storage.
> > +      LINSTOR can provide block storage devices based on DRBD or basic
> > +      LVM/ZFS volumes.
> > +    </p>
> > +
> > +    <p>
> > +      To use LINSTOR in libvirt, setup a working LINSTOR cluster, documentation
> > +      for that is in the LINSTOR Users-guide.
> 
> Could you link to it?
> 
> Also I'd like to ask you to provide a way to setup this storage on our
> CI system so that we can compile-test the new driver.

It doesn't seem like there is anything required in this respect. The
code isn't linking to any linstor specific libraries from what I see
in the meson.build.  So compile testing should be easy unless Im
missing something.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux