8:56 a.m.
changes in v2:
- refactor of resource parsing and formatting code
- use <fibrechannel vmid=''/> element
- use stat() directly
- report only single system error
Pavel Hrdina (5):
vircgroup: introduce virCgroupGetInode function
conf: refactor virDomainResourceDefParse
conf: refactor virDomainResourceDefFormat
conf: introduce support for Fibre Channel VMID
qemu: implement support for Fibre Channel VMID
docs/formatdomain.rst | 21 ++++++++++
docs/schemas/domaincommon.rng | 21 ++++++++--
src/conf/domain_conf.c | 42 +++++++++++--------
src/conf/domain_conf.h | 1 +
src/conf/domain_validate.c | 19 +++++++++
src/libvirt_private.syms | 1 +
src/qemu/qemu_cgroup.c | 27 ++++++++++++
src/util/vircgroup.c | 31 ++++++++++++++
src/util/vircgroup.h | 2 +
.../fibrechannel-vmid.xml | 21 ++++++++++
tests/genericxml2xmltest.c | 2 +
11 files changed, 167 insertions(+), 21 deletions(-)
create mode 100644 tests/genericxml2xmlindata/fibrechannel-vmid.xml
--
2.31.1
8:56 a.m.
New subject: [libvirt PATCH v2 1/5] vircgroup: introduce virCgroupGetInode function
For new feature Fibre Channel VMID we will need to get inode of the
VM root cgroup as it is used in the new kernel API together with VMID.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/vircgroup.c | 31 +++++++++++++++++++++++++++++++
src/util/vircgroup.h | 2 ++
3 files changed, 34 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index aed40977b3..c2321aba70 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1920,6 +1920,7 @@ virCgroupGetCpuShares;
virCgroupGetDevicePermsString;
virCgroupGetDomainTotalCpuStats;
virCgroupGetFreezerState;
+virCgroupGetInode;
virCgroupGetMemoryHardLimit;
virCgroupGetMemorySoftLimit;
virCgroupGetMemoryStat;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 1b3b28342e..4c9445340e 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -3973,3 +3973,34 @@ virCgroupGetCpuPeriodQuota(virCgroup *cgroup, unsigned long long
*period,
return 0;
}
+
+
+/**
+ * virCgroupGetInode:
+ *
+ * @cgroup: the cgroup to get inode for
+ *
+ * Get the @cgroup inode and return its value to the caller.
+ *
+ * Returns inode on success, -1 on error with error message reported.
+ */
+int
+virCgroupGetInode(virCgroup *cgroup)
+{
+ struct stat st;
+ int controller = virCgroupGetAnyController(cgroup);
+ g_autofree char *path = NULL;
+
+ if (controller < 0)
+ return -1;
+
+ if (virCgroupPathOfController(cgroup, controller, "", &path) < 0)
+ return -1;
+
+ if (stat(path, &st) < 0) {
+ virReportSystemError(errno, _("failed to get stat for '%s'"),
path);
+ return -1;
+ }
+
+ return st.st_ino;
+}
diff --git a/src/util/vircgroup.h b/src/util/vircgroup.h
index 8002cac570..690f09465c 100644
--- a/src/util/vircgroup.h
+++ b/src/util/vircgroup.h
@@ -283,3 +283,5 @@ int virCgroupSetOwner(virCgroup *cgroup,
int virCgroupHasEmptyTasks(virCgroup *cgroup, int controller);
bool virCgroupControllerAvailable(int controller);
+
+int virCgroupGetInode(virCgroup *cgroup);
--
2.31.1
8:56 a.m.
New subject: [libvirt PATCH v2 2/5] conf: refactor virDomainResourceDefParse
There is no need to error out for empty <partition></partition> element
as we can just simply ignore it. This allows to simplify the function
and prepare it for new sub-elements of <resource>.
It makes the <partition> element optional so we need to reflect the
change in schema as well.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
docs/schemas/domaincommon.rng | 8 +++++---
src/conf/domain_conf.c | 18 +++++++-----------
2 files changed, 12 insertions(+), 14 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2442078969..9b669d9de5 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1172,9 +1172,11 @@
<define name="respartition">
<element name="resource">
- <element name="partition">
- <ref name="absFilePath"/>
- </element>
+ <optional>
+ <element name="partition">
+ <ref name="absFilePath"/>
+ </element>
+ </optional>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 09da4ab952..de38dd7110 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17292,23 +17292,19 @@ virDomainResourceDefParse(xmlNodePtr node,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
virDomainResourceDef *def = NULL;
+ char *partition = NULL;
ctxt->node = node;
+ partition = virXPathString("string(./partition)", ctxt);
+
+ if (!partition)
+ return NULL;
+
def = g_new0(virDomainResourceDef, 1);
-
- /* Find out what type of virtualization to use */
- if (!(def->partition = virXPathString("string(./partition)", ctxt))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("missing resource partition
attribute"));
- goto error;
- }
+ def->partition = partition;
return def;
-
- error:
- virDomainResourceDefFree(def);
- return NULL;
}
--
2.31.1
8:56 a.m.
New subject: [libvirt PATCH v2 3/5] conf: refactor virDomainResourceDefFormat
Prepare the function for additional sub-elements where all of the
sub-elements are optional.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/conf/domain_conf.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index de38dd7110..d33d708189 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -26794,11 +26794,15 @@ static void
virDomainResourceDefFormat(virBuffer *buf,
virDomainResourceDef *def)
{
- virBufferAddLit(buf, "<resource>\n");
- virBufferAdjustIndent(buf, 2);
- virBufferEscapeString(buf, "<partition>%s</partition>\n",
def->partition);
- virBufferAdjustIndent(buf, -2);
- virBufferAddLit(buf, "</resource>\n");
+ g_auto(virBuffer) childBuf = VIR_BUFFER_INIT_CHILD(buf);
+
+ if (!def)
+ return;
+
+ if (def->partition)
+ virBufferEscapeString(&childBuf,
"<partition>%s</partition>\n", def->partition);
+
+ virXMLFormatElement(buf, "resource", NULL, &childBuf);
}
@@ -27951,8 +27955,7 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def,
if (virDomainNumatuneFormatXML(buf, def->numa) < 0)
return -1;
- if (def->resource)
- virDomainResourceDefFormat(buf, def->resource);
+ virDomainResourceDefFormat(buf, def->resource);
for (i = 0; i < def->nsysinfo; i++) {
if (virSysinfoFormat(buf, def->sysinfo[i]) < 0)
--
2.31.1
8:56 a.m.
New subject: [libvirt PATCH v2 4/5] conf: introduce support for Fibre Channel VMID
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
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-vmid.xml | 21 +++++++++++++++++++
tests/genericxml2xmltest.c | 2 ++
7 files changed, 85 insertions(+), 1 deletion(-)
create mode 100644 tests/genericxml2xmlindata/fibrechannel-vmid.xml
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index 61ccd8895a..f9ea2de59c 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
+VMID. Also it can collect telemetry data at per-VM level which can be used
+to enhance the IO performance of the VM. This can be configured by using
+the ``vmid`` attribute of ``fibrechannel`` element. The attribute contains
+single string (max 128 bytes).
+
+::
+
+ ...
+ <resource>
+ <fibrechannel vmid='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..1087269fc7 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="vmid">
+ <data type="string">
+ <param name="pattern">.{1,128}</param>
+ </data>
+ </attribute>
+ </element>
+ </define>
+
<!--
The Identifiers can be:
- an optional id attribute with a number on the domain element
@@ -1177,6 +1187,9 @@
<ref name="absFilePath"/>
</element>
</optional>
+ <optional>
+ <ref name="fibrechannel"/>
+ </optional>
</element>
</define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d33d708189..85eb5205df 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3446,6 +3446,7 @@ virDomainResourceDefFree(virDomainResourceDef *resource)
return;
g_free(resource->partition);
+ g_free(resource->vmid);
g_free(resource);
}
@@ -17293,16 +17294,19 @@ virDomainResourceDefParse(xmlNodePtr node,
VIR_XPATH_NODE_AUTORESTORE(ctxt)
virDomainResourceDef *def = NULL;
char *partition = NULL;
+ char *vmid = NULL;
ctxt->node = node;
partition = virXPathString("string(./partition)", ctxt);
+ vmid = virXPathString("string(./fibrechannel/@vmid)", ctxt);
- if (!partition)
+ if (!partition && !vmid)
return NULL;
def = g_new0(virDomainResourceDef, 1);
def->partition = partition;
+ def->vmid = vmid;
return def;
}
@@ -26802,6 +26806,9 @@ virDomainResourceDefFormat(virBuffer *buf,
if (def->partition)
virBufferEscapeString(&childBuf,
"<partition>%s</partition>\n", def->partition);
+ if (def->vmid)
+ virBufferEscapeString(&childBuf, "<fibrechannel
vmid='%s'/>\n", def->vmid);
+
virXMLFormatElement(buf, "resource", NULL, &childBuf);
}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 9f32bcf9cf..c978e5b4dc 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2538,6 +2538,7 @@ void virBlkioDeviceArrayClear(virBlkioDevice *deviceWeights,
struct _virDomainResourceDef {
char *partition;
+ char *vmid;
};
struct _virDomainHugePage {
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
index a9e4519b1a..7f27ec3fd9 100644
--- a/src/conf/domain_validate.c
+++ b/src/conf/domain_validate.c
@@ -55,6 +55,22 @@ virDomainDefBootValidate(const virDomainDef *def)
}
+static int
+virDomainDefResourceValidate(const virDomainDef *def)
+{
+ if (!def->resource)
+ return 0;
+
+ if (def->resource->vmid && strlen(def->resource->vmid) > 128)
{
+ virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("Fibre Channel VMID cannot be longer then 128
characters"));
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
virDomainDefVideoValidate(const virDomainDef *def)
{
@@ -1538,6 +1554,9 @@ static int
virDomainDefValidateInternal(const virDomainDef *def,
virDomainXMLOption *xmlopt)
{
+ if (virDomainDefResourceValidate(def) < 0)
+ return -1;
+
if (virDomainDefDuplicateDiskInfoValidate(def) < 0)
return -1;
diff --git a/tests/genericxml2xmlindata/fibrechannel-vmid.xml
b/tests/genericxml2xmlindata/fibrechannel-vmid.xml
new file mode 100644
index 0000000000..fef2c86e3c
--- /dev/null
+++ b/tests/genericxml2xmlindata/fibrechannel-vmid.xml
@@ -0,0 +1,21 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <vcpu placement='static'>4</vcpu>
+ <resource>
+ <fibrechannel vmid='someapp:c7a5fdbd-edaf-9455-926a-d65c16db1809'/>
+ </resource>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu-system-x86_64</emulator>
+ </devices>
+</domain>
diff --git a/tests/genericxml2xmltest.c b/tests/genericxml2xmltest.c
index ef51ed91a6..62789fb7eb 100644
--- a/tests/genericxml2xmltest.c
+++ b/tests/genericxml2xmltest.c
@@ -225,6 +225,8 @@ mymain(void)
DO_TEST_DIFFERENT("cputune");
DO_TEST("device-backenddomain");
+ DO_TEST("fibrechannel-vmid");
+
#define DO_TEST_BACKUP_FULL(name, intrnl) \
do { \
const struct testCompareBackupXMLData data = { .testname = name, \
--
2.31.1
8:56 a.m.
New subject: [libvirt PATCH v2 5/5] qemu: implement support for Fibre Channel VMID
Based on kernel commit messages the interface is
/sys/class/fc/fc_udev_device/appid_store
where we need to write the following string "$INODE:$VMID".
$INODE is the VM root cgroup inode in hexadecimal and $VMID is user
provided string that will be attached to each FC frame for the VM
within the cgroup identified by inode and has limit 128 bytes.
Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com>
---
src/qemu/qemu_cgroup.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f2d99abcfa..cd5d4341f7 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -904,6 +904,30 @@ qemuSetupCpuCgroup(virDomainObj *vm)
}
+static int
+qemuSetupCgroupVMID(virDomainObj *vm)
+{
+ qemuDomainObjPrivate *priv = vm->privateData;
+ int inode = virCgroupGetInode(priv->cgroup);
+ const char *path = "/sys/class/fc/fc_udev_device/appid_store";
+ g_autofree char *appid = NULL;
+ virDomainResourceDef *resource = vm->def->resource;
+
+ if (!resource || !resource->vmid)
+ return 0;
+
+ appid = g_strdup_printf("%X:%s", inode, resource->vmid);
+
+ if (virFileWriteStr(path, appid, 0) < 0) {
+ virReportSystemError(errno,
+ _("Unable to write '%s' to '%s'"),
appid, path);
+ return -1;
+ }
+
+ return 0;
+}
+
+
static int
qemuInitCgroup(virDomainObj *vm,
size_t nnicindexes,
@@ -1096,6 +1120,9 @@ qemuSetupCgroup(virDomainObj *vm,
if (qemuSetupCpusetCgroup(vm) < 0)
return -1;
+ if (qemuSetupCgroupVMID(vm) < 0)
+ return -1;
+
return 0;
}
--
2.31.1
1234
days inactive
1234
days old
5 comments
1 participants
participants (1)
-
Pavel Hrdina