Re: [PATCH v3 4/5] utils: Use overrides in virFileIsSharedFS()
On Thu, May 09, 2024 at 06:54:11 -0700, Andrea Bolognani wrote:
On Thu, May 09, 2024 at 02:28:15PM GMT, Peter Krempa wrote:
> On Thu, May 02, 2024 at 19:39:41 +0200, Andrea Bolognani wrote:
> > +static bool
> > +virFileIsSharedFSOverride(const char *path,
> > + char *const *overrides)
> > +{
> > + g_autofree char *dirpath = NULL;
> > + char *p = NULL;
> > +
> > + if (!path || path[0] != '/' || !overrides)
> > + return false;
>
> Per my comment on canonicalizing paths only when they're about to be
> used.
Gotcha.
> I think you can also modify the algorithm to avoid the truncate&compare
> operations to:
>
>
> foreach override in overrides:
>
> pc = canonicalize(path);
> po = canonicalize(override);
>
> if (STRPREFIX(pc, po))
> return true;
I'll give it a try.
> Checking the full prefix on canonicalized paths is IIUC equivalent to
> what you do below. (Okay perhaps except the case when user declares a
> full to a single file as an exported override).
Okay, so firstly I wrote something else than I've thought.
The algorithm above has a bug if you declare an exported file such
as:
/path/to/ble
which is a file not a directory, and have VM with disk pointing to
/path/to/blesomething
the above code would mark it as shared based on the override. You need
to use the fixed version I've suggested in a reply.
That isn't supposed to work anyway... If the current code allows
it
then it will need to be fixed.
By going with the meaning that users can mark individual files as
exported, the original impl you've posted does support that:
+ if (g_strv_contains((const char *const *) overrides, path))
+ return true;
+