[PATCH] lxc: truncate LOOP_GET_STATUS64.lo_file_name for long loop backing paths

Radoslaw Smigielski via Devel posted 1 patch 1 day, 3 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20260529110210.507995-1-rsmigiel@redhat.com
src/util/virfile.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
[PATCH] lxc: truncate LOOP_GET_STATUS64.lo_file_name for long loop backing paths
Posted by Radoslaw Smigielski via Devel 1 day, 3 hours ago
LXC domains with long file-backed filesystem path fail to start when
the backing image path is longer than LO_NAME_SIZE (64 bytes, 63 characters plus NUL).
When long file path is passed, virFileLoopDeviceAssociate() -> virStrcpy() fails
and user gets missleading error and domain fails to start.

Example:

    <filesystem type='file' accessmode='passthrough'>
      <driver type='loop' format='raw'/>
      <source file='/root/demoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.raw'/>
      <target dir='/'/>
    </filesystem>

To match losetup behavior we copy the path with virStrcpy() and allow truncation
of lo_file_name only if needed, while still calling open() on the unchanged path.
Finally log VIR_WARN when the path is expected to be truncated. But still report
VIR_ERR_INTERNAL_ERROR for all other virStrcpy() failures.

Fixes: https://gitlab.com/libvirt/libvirt/-/work_items/63

Signed-off-by: Radoslaw Smigielski <rsmigiel@redhat.com>
---
 src/util/virfile.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index a0c6cb804862..ae33deb8d223 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -995,11 +995,18 @@ int virFileLoopDeviceAssociate(const char *file,
 
     lo.lo_flags = LO_FLAGS_AUTOCLEAR;
 
-    /* Set backing file name for LOOP_GET_STATUS64 queries */
+    /* lo_file_name is loop device name, max length is LO_NAME_SIZE bytes.
+     * Truncate loop device name if file path is longer than LO_NAME_SIZE,
+     * and still use the full path to open backing file. */
     if (virStrcpy((char *) lo.lo_file_name, file, LO_NAME_SIZE) < 0) {
-        virReportSystemError(errno,
-                             _("Unable to set backing file %1$s"), file);
-        goto cleanup;
+        if (strlen(file) >= LO_NAME_SIZE) {
+            VIR_WARN("Loop backing device name %s truncated to %d bytes.",
+                     file, LO_NAME_SIZE);
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Unable to set loop lo_file_name for %1$s"), file);
+            goto cleanup;
+        }
     }
 
     if ((fsfd = open(file, O_RDWR)) < 0) {
-- 
2.54.0
Re: [PATCH] lxc: truncate LOOP_GET_STATUS64.lo_file_name for long loop backing paths
Posted by Peter Krempa via Devel 1 day, 2 hours ago
Please setup your git so that it formats the extra "From: " line which
is needed as the mailing list mangles sender for domains having DMARC
setup:

 https://www.libvirt.org/submitting-patches.html#git-configuration

On Fri, May 29, 2026 at 13:00:22 +0200, Radoslaw Smigielski via Devel wrote:
> LXC domains with long file-backed filesystem path fail to start when
> the backing image path is longer than LO_NAME_SIZE (64 bytes, 63 characters plus NUL).
> When long file path is passed, virFileLoopDeviceAssociate() -> virStrcpy() fails
> and user gets missleading error and domain fails to start.
> 
> Example:
> 
>     <filesystem type='file' accessmode='passthrough'>
>       <driver type='loop' format='raw'/>
>       <source file='/root/demoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.raw'/>
>       <target dir='/'/>
>     </filesystem>
> 
> To match losetup behavior we copy the path with virStrcpy() and allow truncation
> of lo_file_name only if needed, while still calling open() on the unchanged path.
> Finally log VIR_WARN when the path is expected to be truncated. But still report
> VIR_ERR_INTERNAL_ERROR for all other virStrcpy() failures.

Umm, there are no other virStrcpy failures:

/**
 * virStrcpy:
 *
 * @dest: destination buffer
 * @src: source buffer
 * @destbytes: number of bytes the destination can accommodate
 *
 * Copies @src to @dest. @dest is guaranteed to be 'nul' terminated if
 * destbytes is 1 or more.
 *
 * Returns: 0 on success, -1 if @src doesn't fit into @dest and was truncated.

so what you have there is effectively dead code.

Another issue is that if you'll have:

     <filesystem type='file' accessmode='passthrough'>
       <driver type='loop' format='raw'/>
       <source file='/root/someveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeerylongpath/a.raw'/>
       <target dir='/'/>
     </filesystem>
     <filesystem type='file' accessmode='passthrough'>
       <driver type='loop' format='raw'/>
       <source file='/root/someveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeerylongpath/b.raw'/>
       <target dir='/'/>
     </filesystem>

They'll be truncated to the same string.

> 
> Fixes: https://gitlab.com/libvirt/libvirt/-/work_items/63
> 
> Signed-off-by: Radoslaw Smigielski <rsmigiel@redhat.com>
> ---
>  src/util/virfile.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/src/util/virfile.c b/src/util/virfile.c
> index a0c6cb804862..ae33deb8d223 100644
> --- a/src/util/virfile.c
> +++ b/src/util/virfile.c
> @@ -995,11 +995,18 @@ int virFileLoopDeviceAssociate(const char *file,
>  
>      lo.lo_flags = LO_FLAGS_AUTOCLEAR;
>  
> -    /* Set backing file name for LOOP_GET_STATUS64 queries */
> +    /* lo_file_name is loop device name, max length is LO_NAME_SIZE bytes.
> +     * Truncate loop device name if file path is longer than LO_NAME_SIZE,
> +     * and still use the full path to open backing file. */
>      if (virStrcpy((char *) lo.lo_file_name, file, LO_NAME_SIZE) < 0) {
> -        virReportSystemError(errno,
> -                             _("Unable to set backing file %1$s"), file);
> -        goto cleanup;
> +        if (strlen(file) >= LO_NAME_SIZE) {
> +            VIR_WARN("Loop backing device name %s truncated to %d bytes.",
> +                     file, LO_NAME_SIZE);

So these go only into the log. The question is if it here makes sense.

In case when the user has just one truncated path which will thus never
cause another error it will be forever ignored.

Then if there are multiple paths, if they collide but work, why bother
with warning? and if they don't work then reporting the error upfront is
better.

> +        } else {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                           _("Unable to set loop lo_file_name for %1$s"), file);
> +            goto cleanup;

This is dead code.

> +        }
>      }
>  
>      if ((fsfd = open(file, O_RDWR)) < 0) {
> -- 
> 2.54.0
>
Re: [PATCH] lxc: truncate LOOP_GET_STATUS64.lo_file_name for long loop backing paths
Posted by Radoslaw Smigielski via Devel 23 hours ago
Hi Peter,
  Sure will fix my git config, sorry for that.

On Fri, 29 May 2026 at 13:47, Peter Krempa <pkrempa@redhat.com> wrote:
> On Fri, May 29, 2026 at 13:00:22 +0200, Radoslaw Smigielski via Devel
wrote:
> > LXC domains with long file-backed filesystem path fail to start when
> > the backing image path is longer than LO_NAME_SIZE (64 bytes, 63
characters plus NUL).
> > When long file path is passed, virFileLoopDeviceAssociate() ->
virStrcpy() fails
> > and user gets missleading error and domain fails to start.
> >
> > Example:
> >
> >     <filesystem type='file' accessmode='passthrough'>
> >       <driver type='loop' format='raw'/>
> >       <source
file='/root/demoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.raw'/>
> >       <target dir='/'/>
> >     </filesystem>
> >
> > To match losetup behavior we copy the path with virStrcpy() and allow
truncation
> > of lo_file_name only if needed, while still calling open() on the
unchanged path.
> > Finally log VIR_WARN when the path is expected to be truncated. But
still report
> > VIR_ERR_INTERNAL_ERROR for all other virStrcpy() failures.
>
> Umm, there are no other virStrcpy failures:
>
> /**
>  * virStrcpy:
>  *
>  * @dest: destination buffer
>  * @src: source buffer
>  * @destbytes: number of bytes the destination can accommodate
>  *
>  * Copies @src to @dest. @dest is guaranteed to be 'nul' terminated if
>  * destbytes is 1 or more.
>  *
>  * Returns: 0 on success, -1 if @src doesn't fit into @dest and was
truncated.
>
> so what you have there is effectively dead code.

Indeed, this would need to be removed.



> Another issue is that if you'll have:
>
>      <filesystem type='file' accessmode='passthrough'>
>        <driver type='loop' format='raw'/>
>        <source
file='/root/someveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeerylongpath/a.raw'/>
>        <target dir='/'/>
>      </filesystem>
>      <filesystem type='file' accessmode='passthrough'>
>        <driver type='loop' format='raw'/>
>        <source
file='/root/someveeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeerylongpath/b.raw'/>
>        <target dir='/'/>
>      </filesystem>
>
> They'll be truncated to the same string.

So I tried to follow the same logic like losetup from util-linux uses to
handle long paths:
  - takes the first 63 bytes of the absolute path
  - no intelligent path shortening (like keeping the filename and
truncating middle)
  - adds an asterisk at position 62 to indicate the name was truncated

    lo->lo_file_name[LO_NAME_SIZE - 2] = '*';  // Position 62 gets '*'
    lo->lo_file_name[LO_NAME_SIZE - 1] = '\0'; // Position 63 is null
terminator

Above indeed can result in non-unique or unhelpful loop device names in the
kernel's loop_info64 structure.
Question if we should mimic the same logic or imlement someting smarter.
Addint "*" before '\0' to indicate truncation would make it compatibile
with losetup behavior.



----------------------
< Tℏanks | Radek >