On Tue, Nov 24, 2020 at 12:59 AM Peter Krempa <pkrempa@xxxxxxxxxx> wrote:
On Mon, Nov 23, 2020 at 17:17:15 -0600, Dustan B Helm wrote:
> On Mon, Nov 23, 2020 at 2:33 AM Peter Krempa <pkrempa@xxxxxxxxxx> wrote:
>
> > On Sat, Nov 21, 2020 at 11:20:57 -0600, Dustan B Helm wrote:
> > > We plan to support NFS protocol according to the example XML from Issue
> > 90
> > > <http://gitlab.com/libvirt/libvirt/-/issues/90>. Since there is already
> > > support for network disks of different protocol types and host
> > information,
> > > we think that the only new XML information we will add is an <nfs>
> > element
> > > which will be a subelement of <source>, with attributes “user” and
> > “group”
> > > (both strings). This element will only be generated if the source
> > protocol
> > > is “nfs” and we assume that both “user” and “group” will be required.
> > >
> > > Here is the XML example given in the issue for reference:
> > >
> > > <disk type='network' device='disk'>
> > >
> > > <driver name='qemu' type='raw'/>
> > >
> > > <source protocol='nfs' name='PATH'>
> > >
> > > <host name='example.com' port='2049'/
> > >
> > > <nfs user='USER' group='GROUP'/>
> > >
> > > </source>
> > >
> > > <target dev='vda' bus='virtio'/>
> > >
> > > </disk>
> >
> > Sounds reasonable to me. We tend to name elements equivalent to <nfs>
> > you propose by their purpose (such as <auth> <initiator> <cookies> for
> > other protocols) but in this case I don't have a better suggestion so
> > going with <nfs> is reasonable.
> >
> > Since you are proposing 'user' and 'group' to be strings while qemu
> > accepts only numeric UID/GID, please use the same conversion code we
> > have for the <inituser> and <initgroup> values in regards to forcing
> > numeric value to skip being interpreded:
> >
> > https://www.libvirt.org/formatdomain.html#container-boot
> >
> > The linked documentation
> <https://www.libvirt.org/formatdomain.html#container-boot> suggests that
> providing a “+” prefix will force attribute data to be interpreted
> numerically. Since QEMU requires that the group and user attributes be
> numeric id values, would we want to simply insert a “+” prefix to these
> attributes when they are provided explicitly (instead of using the
> hypervisor default values, which we assume will just be numeric data)?
No, not at all. QEMU requires you to provide an integer according to the
schema. That means that for any '+'-prefixed value, you strip the + and
convert it to int. For a username you look it up in the system to get
the uid and use that. We have helpers for this, but I can't recall the
name. You'll have to do some digging.
We found methods to convert user and group strings into integer IDs in src/util/virutil.c (the getUserID and getGroupID methods). We plan on checking if the parameter provided has a + at the front of the string, in which case we will call these methods to convert the user or group name into an ID. However, we aren’t sure where the _virStorageSource data is actually being initialized. We found the qemuBlockStorageSourceGetBackendProps method in src/qemu/qemu_block.c, for which we would need to provide an NFS props method. At this point we need the _virStorageSource to be initialized and contain the nfs_user and nfs_group parameters, but we can’t find where to actually set these. You had mentioned some kind of parser in domain_conf.c, but since domain_conf is an extensive file, we had some trouble narrowing down our concerns to a specific parsing/formatting mechanism. What method(s) would we change here? Would that be satisfactory to set our _virStorageSource for use in the JSON initialization?
