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