On Tue, Nov 24, 2020 at 12:59 AM Peter Krempa <pkre...@redhat.com> 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 <pkre...@redhat.com> 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?