[libvirt PATCH 3/3] qemu: implement support for Fibre Channel VMID

Pavel Hrdina posted 3 patches 4 years, 6 months ago
There is a newer version of this series
[libvirt PATCH 3/3] qemu: implement support for Fibre Channel VMID
Posted by Pavel Hrdina 4 years, 6 months ago
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@redhat.com>
---
 src/qemu/qemu_cgroup.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index f2d99abcfa..df021b5985 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -904,6 +904,35 @@ 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;
+
+    if (!vm->def->vmid)
+        return 0;
+
+    appid = g_strdup_printf("%X:%s", inode, vm->def->vmid);
+
+    if (virFileWriteStr(path, appid, 0) < 0) {
+        if (errno == ENOENT) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("FC VMID not available on this host"));
+            return -1;
+        }
+
+        virReportSystemError(errno,
+                             _("Unable to write '%s' to '%s'"), appid, path);
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 qemuInitCgroup(virDomainObj *vm,
                size_t nnicindexes,
@@ -1096,6 +1125,9 @@ qemuSetupCgroup(virDomainObj *vm,
     if (qemuSetupCpusetCgroup(vm) < 0)
         return -1;
 
+    if (qemuSetupCgroupVMID(vm) < 0)
+        return -1;
+
     return 0;
 }
 
-- 
2.31.1

Re: [libvirt PATCH 3/3] qemu: implement support for Fibre Channel VMID
Posted by Michal Prívozník 4 years, 6 months ago
On 8/3/21 4:29 PM, Pavel Hrdina wrote:
> 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@redhat.com>
> ---
>  src/qemu/qemu_cgroup.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index f2d99abcfa..df021b5985 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -904,6 +904,35 @@ 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;
> +
> +    if (!vm->def->vmid)
> +        return 0;
> +
> +    appid = g_strdup_printf("%X:%s", inode, vm->def->vmid);
> +
> +    if (virFileWriteStr(path, appid, 0) < 0) {
> +        if (errno == ENOENT) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("FC VMID not available on this host"));
> +            return -1;
> +        }
> +
> +        virReportSystemError(errno,
> +                             _("Unable to write '%s' to '%s'"), appid, path);

Alternatively, use the following pattern:

if (errno == ENOENT) {
  virReportError();
} else {
  virReportSystemError();
}

return -1;


Or use just virReportSystemError(); I believe "Unable to write to
/sys/class/...: No such file or directory" is pretty comprehensive.

Michal

Re: [libvirt PATCH 3/3] qemu: implement support for Fibre Channel VMID
Posted by Pavel Hrdina 4 years, 6 months ago
On Wed, Aug 04, 2021 at 10:06:21AM +0200, Michal Prívozník wrote:
> On 8/3/21 4:29 PM, Pavel Hrdina wrote:
> > 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@redhat.com>
> > ---
> >  src/qemu/qemu_cgroup.c | 32 ++++++++++++++++++++++++++++++++
> >  1 file changed, 32 insertions(+)
> > 
> > diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> > index f2d99abcfa..df021b5985 100644
> > --- a/src/qemu/qemu_cgroup.c
> > +++ b/src/qemu/qemu_cgroup.c
> > @@ -904,6 +904,35 @@ 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;
> > +
> > +    if (!vm->def->vmid)
> > +        return 0;
> > +
> > +    appid = g_strdup_printf("%X:%s", inode, vm->def->vmid);
> > +
> > +    if (virFileWriteStr(path, appid, 0) < 0) {
> > +        if (errno == ENOENT) {
> > +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> > +                           _("FC VMID not available on this host"));
> > +            return -1;
> > +        }
> > +
> > +        virReportSystemError(errno,
> > +                             _("Unable to write '%s' to '%s'"), appid, path);
> 
> Alternatively, use the following pattern:
> 
> if (errno == ENOENT) {
>   virReportError();
> } else {
>   virReportSystemError();
> }
> 
> return -1;
> 
> 
> Or use just virReportSystemError(); I believe "Unable to write to
> /sys/class/...: No such file or directory" is pretty comprehensive.

I wanted to have nice explanatory error messages where missing presence
of the file means that the kernel is old and doesn't have the feature
implemented at all.

I wanted to do the same for case where the feature is not enabled in
kernel config but that is basically impossible. The file will still be
present but any write to it will return EINVAL. However, the same
happens for actual invalid string format or if invalid inode and so on.

I guess I'll drop this attempt too and use single error message as
you've suggested.

Pavel