Re: [Libguestfs] [nbdkit PATCH v2] plugin: add and use nbdkit_realpath
On 02/21/2018 11:13 AM, Pino Toscano wrote:
On Wednesday, 14 February 2018 18:06:10 CET Eric Blake wrote:
> On 02/14/2018 10:53 AM, Pino Toscano wrote:
>> Introduce a new helper function to resolve a path name, calling
>> nbdkit_error on failure: other than doing what nbdkit_absolute_path
>> does, it also checks that the file exists (and thus avoids errors later
>> on). To help distinguish it from nbdkit_absolute_path, improve the
>> documentation of the latter.
>>
>> Apply it where an existing path is required, both in nbdkit itself and
>> in plugins.
>>
>> Related to: https://bugzilla.redhat.com/show_bug.cgi?id=1527334
>> ---
>> +char *
>> +nbdkit_realpath (const char *path)
>> +{
>> + char *ret;
>> +
>> + if (path == NULL || *path == '\0') {
>> + nbdkit_error ("cannot resolve a null or empty path");
>> + return NULL;
>> + }
>> +
>> + ret = realpath (path, NULL);
>
> Wait. Does this even work?
It works in the same way as nbdkit_absolute_path() did: when calling it
on a relative path, nbdkit_absolute_path() will prepend $PWD to it,
while the new nbdkit_realpath() will do something similar.
My question is whether the $PWD it is prepending is guaranteed to be
correct. If we call this too late, then it is not; it would be trivial
to rewrite it so that we manually prepend something cached from when the
program started, so that this is then conceptually correct no matter
when we change directories later.
At least the usages that I changed are called before start_serving()
(and thus before fork_into_background()), so I think there should be no
issue wrt paths.
Oh. Hmm. Interesting observation, that .config and .config_complete
finish PRIOR to start_serving() (and makes sense, too, as if we're going
to diagnose any errors, printing to stderr is a LOT easier at the point
where we haven't forked and redirected stderr). But maybe that means
that we should document that these two functions are available for
plugins/filters, but ONLY for use during .config/.config_complete; that
any time later they are unsafe. OR, we can fix them to ALWAYS work,
even if they are invoked somewhere outside of .config.
Did I miss anything?
No, but I missed that .config runs before forking.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org