Re: [libvirt PATCH v3 4/5] conf: introduce support for Fibre Channel VMID

On Tue, Aug 17, 2021 at 11:26:41AM +0200, Pavel Hrdina wrote: > Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > --- > docs/formatdomain.rst | 21 +++++++++++++++++++ > docs/schemas/domaincommon.rng | 13 ++++++++++++ > src/conf/domain_conf.c | 9 +++++++- > src/conf/domain_conf.h | 1 + > src/conf/domain_validate.c | 19 +++++++++++++++++ > .../fibrechannel-appid.xml | 21 +++++++++++++++++++ > tests/genericxml2xmltest.c | 2 ++ > 7 files changed, 85 insertions(+), 1 deletion(-) > create mode 100644 tests/genericxml2xmlindata/fibrechannel-appid.xml > > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst > index 61ccd8895a..881a75df87 100644 > --- a/docs/formatdomain.rst > +++ b/docs/formatdomain.rst > @@ -1221,6 +1221,27 @@ Resource partitions are currently supported by the QEMU and LXC drivers, which > map partition paths to cgroups directories, in all mounted controllers. > :since:`Since 1.0.5` > > +Fibre Channel VMID > +------------------- > + > +The FC SAN can provide various QOS levels, access control depending on the "QoS levels and access control" > +VMID. Also it can collect telemetry data at per-VM level which can be used "It can also collect" reads better > +to enhance the IO performance of the VM. This can be configured by using > +the ``appid`` attribute of ``fibrechannel`` element. The attribute contains > +single string (max 128 bytes) and it is used by kernel to create VMID. > + > +:: > + > + ... > + <resource> > + <fibrechannel appid='userProvidedID'/> > + </resource> > + ... > + > +Using this feature requires Fibre Channel capable HW, kernel compiled with > +option ``CONFIG_BLK_CGROUP_FC_APPID`` and ``nvme_fc`` kernel module loaded. > +:since:`Since 7.7.0` > + > :anchor:`<a id="elementsCPU"/>` > > CPU model and topology > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng > index 9b669d9de5..b32fb8c04c 100644 > --- a/docs/schemas/domaincommon.rng > +++ b/docs/schemas/domaincommon.rng > @@ -576,6 +576,16 @@ > </element> > </define> > > + <define name="fibrechannel"> > + <element name="fibrechannel"> > + <attribute name="appid"> > + <data type="string"> > + <param name="pattern">.{1,128}</param> I can imagine this biting us in the future, wouldn't it be safer to limit the character set of the string? From the kernel code it looks like it handles any bytes and pads them to 128 unconditionally, but I still feel uneasy about not limiting it to at least printable characters (like we do with e.g. disk vendor and product strings).