Re: [PATCH] storage: Linstor support

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(a)linbit.com&gt; > --- 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.