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

[Pavel Hrdina]

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 at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210804/850d2ed6/attachment-0001.sig>