If the local admin has explicitly declared that a certain
filesystem is to be considered shared, we should treat it as
such.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
 src/util/virfile.c | 42 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 3268866f8b..45815919d6 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -3801,9 +3801,49 @@ virFileGetDefaultHugepage(virHugeTLBFS *fs,
     return NULL;
 }
 
+static bool
+virFileIsSharedFSOverride(const char *path,
+                          char *const *overrides)
+{
+    g_autofree char *dirpath = NULL;
+    char *p = NULL;
+
+    if (!path || path[0] != '/' || !overrides)
+        return false;
+
+    if (g_strv_contains((const char *const *) overrides, path))
+        return true;
+
+    dirpath = g_strdup(path);
+
+    /* Continue until we've scanned the entire path */
+    while (p != dirpath) {
+
+        /* Find the last slash */
+        if ((p = strrchr(dirpath, '/')) == NULL)
+            break;
+
+        /* Truncate the path by overwriting the slash that we've just
+         * found with a null byte. If it is the very first slash in
+         * the path, we need to handle things slightly differently */
+        if (p == dirpath)
+            *(p+1) = '\0';
+        else
+            *p = '\0';
+
+        if (g_strv_contains((const char *const *) overrides, dirpath))
+            return true;
+    }
+
+    return false;
+}
+
 int virFileIsSharedFS(const char *path,
-                      char *const *overrides G_GNUC_UNUSED)
+                      char *const *overrides)
 {
+    if (virFileIsSharedFSOverride(path, overrides))
+        return 1;
+
     return virFileIsSharedFSType(path,
                                  VIR_FILE_SHFS_NFS |
                                  VIR_FILE_SHFS_GFS2 |
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On 4/17/24 09:29, Andrea Bolognani wrote:
> If the local admin has explicitly declared that a certain
> filesystem is to be considered shared, we should treat it as
> such.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>

> ---
>   src/util/virfile.c | 42 +++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index 3268866f8b..45815919d6 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -3801,9 +3801,49 @@ virFileGetDefaultHugepage(virHugeTLBFS *fs,
>       return NULL;
>   }
>   
> +static bool
> +virFileIsSharedFSOverride(const char *path,
> +                          char *const *overrides)
> +{
> +    g_autofree char *dirpath = NULL;
> +    char *p = NULL;
> +
> +    if (!path || path[0] != '/' || !overrides)
> +        return false;
> +
> +    if (g_strv_contains((const char *const *) overrides, path))
> +        return true;
> +
> +    dirpath = g_strdup(path);
> +
> +    /* Continue until we've scanned the entire path */
> +    while (p != dirpath) {
> +
> +        /* Find the last slash */
> +        if ((p = strrchr(dirpath, '/')) == NULL)
> +            break;
> +
> +        /* Truncate the path by overwriting the slash that we've just
> +         * found with a null byte. If it is the very first slash in
> +         * the path, we need to handle things slightly differently */
> +        if (p == dirpath)
> +            *(p+1) = '\0';
> +        else
> +            *p = '\0';

When admins declare a path as shared they must omit the trailing '/' 
then. It may be wroth mentioning this in 2/5.

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>


> +
> +        if (g_strv_contains((const char *const *) overrides, dirpath))
> +            return true;
> +    }
> +
> +    return false;
> +}
> +
>   int virFileIsSharedFS(const char *path,
> -                      char *const *overrides G_GNUC_UNUSED)
> +                      char *const *overrides)
>   {
> +    if (virFileIsSharedFSOverride(path, overrides))
> +        return 1;
> +
>       return virFileIsSharedFSType(path,
>                                    VIR_FILE_SHFS_NFS |
>                                    VIR_FILE_SHFS_GFS2 |
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On Thu, Apr 18, 2024 at 01:17:34PM GMT, Stefan Berger wrote:
> On 4/17/24 09:29, 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;
> > +
> > +    if (g_strv_contains((const char *const *) overrides, path))
> > +        return true;
> > +
> > +    dirpath = g_strdup(path);
> > +
> > +    /* Continue until we've scanned the entire path */
> > +    while (p != dirpath) {
> > +
> > +        /* Find the last slash */
> > +        if ((p = strrchr(dirpath, '/')) == NULL)
> > +            break;
> > +
> > +        /* Truncate the path by overwriting the slash that we've just
> > +         * found with a null byte. If it is the very first slash in
> > +         * the path, we need to handle things slightly differently */
> > +        if (p == dirpath)
> > +            *(p+1) = '\0';
> > +        else
> > +            *p = '\0';
>
> When admins declare a path as shared they must omit the trailing '/' then.
> It may be wroth mentioning this in 2/5.

Very good catch!

It's fairly easy, and certainly much more user-friendly, to remove
trailing slashes when parsing the configuration file instead. I'll
have that in the next respin.

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org

On Thu, May 02, 2024 at 12:16:32PM GMT, Andrea Bolognani wrote:
> On Thu, Apr 18, 2024 at 01:17:34PM GMT, Stefan Berger wrote:
> > On 4/17/24 09:29, 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;
> > > +
> > > +    if (g_strv_contains((const char *const *) overrides, path))
> > > +        return true;
> > > +
> > > +    dirpath = g_strdup(path);
> > > +
> > > +    /* Continue until we've scanned the entire path */
> > > +    while (p != dirpath) {
> > > +
> > > +        /* Find the last slash */
> > > +        if ((p = strrchr(dirpath, '/')) == NULL)
> > > +            break;
> > > +
> > > +        /* Truncate the path by overwriting the slash that we've just
> > > +         * found with a null byte. If it is the very first slash in
> > > +         * the path, we need to handle things slightly differently */
> > > +        if (p == dirpath)
> > > +            *(p+1) = '\0';
> > > +        else
> > > +            *p = '\0';
> >
> > When admins declare a path as shared they must omit the trailing '/' then.
> > It may be wroth mentioning this in 2/5.
>
> Very good catch!
>
> It's fairly easy, and certainly much more user-friendly, to remove
> trailing slashes when parsing the configuration file instead. I'll
> have that in the next respin.

v3 here:

  https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PISBZCI5MAQQWPN7NMMEGV4VPLJKGEFJ/

I have not CC'd you this time around to avoid unnecessary noise in
your inbox.

-- 
Andrea Bolognani / Red Hat / Virtualization
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org