From: Radoslaw Smigielski <[email protected]>
LXC domains using a file-backed filesystem with the loop driver fail to
start when the backing file path is longer than LO_NAME_SIZE (64 bytes,
63 characters plus NUL). When file path was truncated by virStrcpy(),
it caused virFileLoopDeviceAssociate() to fail and reported a misleading
virReportSystemError(errno, ...), so users saw below error:
"Unable to set backing file ...: Success"
or ENOENT even when the file exists.
Example of XML that failed before this change:
<filesystem type='file' accessmode='passthrough'>
<driver type='loop' format='raw'/>
<source
file='/root/demoaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa.raw'/>
<target dir='/'/>
</filesystem>
This is how losetup output looks like with the new unique identifier visible
in last REF column:
[root@rvm08m-1 radek]# losetup -l -O +REF
NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE
DIO LOG-SEC REF
/dev/loop1 0 0 1 0
/var/tmp/looooooooooooo[...]ooooooooooooooooong.raw 0 512
libvirt-69d62093-b691-46a4-81ab-39db6cead5a8-fs2
/dev/loop0 0 0 1 0 /var/tmp/short.raw
0 512 libvirt-69d62093-b691-46a4-81ab-39db6cead5a8-fs1
Kernel only stores a short name in LOOP_GET_STATUS64.lo_file_name.
While the backing file of a block device is still opened by full path,
and not by lo_file_name. So the Linux kernel only uses lo_file_name
as an informational metadata identifier.
To avoid collisions when long paths are truncated a new unique identifier
is generated in format: "libvirt-$UUID-$DEVALIAS" and assigned to loName.
Additionally, the fatal error on string truncation has been removed
and replaced with a warning message.
Fixes: https://gitlab.com/libvirt/libvirt/-/work_items/63
Signed-off-by: Radoslaw Smigielski <[email protected]>
---
src/lxc/lxc_controller.c | 27 ++++++++++++++++++++-------
src/util/virfile.c | 19 ++++++++++++++-----
src/util/virfile.h | 1 +
3 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index f7776534e8a5..ca8720774c1f 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -53,6 +53,7 @@
#include "virnetdevveth.h"
#include "viralloc.h"
#include "virfile.h"
+#include "viruuid.h"
#include "virgdbus.h"
#include "virpidfile.h"
#include "virhostcpu.h"
@@ -452,12 +453,18 @@ static int
virLXCControllerValidateConsoles(virLXCController *ctrl)
}
-static int virLXCControllerSetupLoopDeviceFS(virDomainFSDef *fs)
+static int virLXCControllerSetupLoopDeviceFS(virDomainDef *def,
+ virDomainFSDef *fs)
{
int lofd;
- char *loname = NULL;
+ g_autofree char *loname = NULL;
+ g_autofree char *loDeviceName = NULL;
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(def->uuid, uuidstr);
+ loDeviceName = g_strdup_printf("libvirt-%s-%s", uuidstr, fs->info.alias);
- if ((lofd = virFileLoopDeviceAssociate(fs->src->path, &loname)) < 0)
+ if ((lofd = virFileLoopDeviceAssociate(fs->src->path, loDeviceName,
&loname)) < 0)
return -1;
VIR_DEBUG("Changing fs %s to use type=block for dev %s",
@@ -474,13 +481,19 @@ static int
virLXCControllerSetupLoopDeviceFS(virDomainFSDef *fs)
}
-static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDef *disk)
+static int virLXCControllerSetupLoopDeviceDisk(virDomainDef *def,
+ virDomainDiskDef *disk)
{
int lofd;
g_autofree char *loname = NULL;
+ g_autofree char *loDeviceName = NULL;
const char *src = virDomainDiskGetSource(disk);
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+
+ virUUIDFormat(def->uuid, uuidstr);
+ loDeviceName = g_strdup_printf("libvirt-%s-%s", uuidstr, disk->info.alias);
- if ((lofd = virFileLoopDeviceAssociate(src, &loname)) < 0)
+ if ((lofd = virFileLoopDeviceAssociate(src, loDeviceName, &loname)) < 0)
return -1;
VIR_DEBUG("Changing disk %s to use type=block for dev %s",
@@ -630,7 +643,7 @@ static int
virLXCControllerSetupLoopDevices(virLXCController *ctrl)
return -1;
}
- fd = virLXCControllerSetupLoopDeviceFS(fs);
+ fd = virLXCControllerSetupLoopDeviceFS(ctrl->def, fs);
if (fd < 0)
return -1;
@@ -685,7 +698,7 @@ static int
virLXCControllerSetupLoopDevices(virLXCController *ctrl)
* don't want to go into the auto-probing
* business for security reasons
*/
- fd = virLXCControllerSetupLoopDeviceDisk(disk);
+ fd = virLXCControllerSetupLoopDeviceDisk(ctrl->def, disk);
if (fd < 0)
return -1;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index a0c6cb804862..6eb9ea1c893a 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -982,6 +982,7 @@ static int virFileLoopDeviceOpen(char **dev_name)
}
int virFileLoopDeviceAssociate(const char *file,
+ const char *loName,
char **dev)
{
int lofd = -1;
@@ -989,6 +990,8 @@ int virFileLoopDeviceAssociate(const char *file,
struct loop_info64 lo = { 0 };
g_autofree char *loname = NULL;
int ret = -1;
+ /* use provided loName if available, otherwise fallback to the file path.
*/
+ const char *name_to_use = loName ? loName : file;
if ((lofd = virFileLoopDeviceOpen(&loname)) < 0)
return -1;
@@ -996,10 +999,16 @@ int virFileLoopDeviceAssociate(const char *file,
lo.lo_flags = LO_FLAGS_AUTOCLEAR;
/* Set backing file name for LOOP_GET_STATUS64 queries */
- 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 (virStrcpy((char *) lo.lo_file_name, name_to_use, LO_NAME_SIZE) < 0) {
+ if (loName) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Generated loop device name '%1$s' exceeds
maximum limit"),
+ loName);
+ goto cleanup;
+ }
+ /* If the raw file path is what truncated, it's safe to just log a
debug
+ * message since the kernel doesn't use lo_file_name for actual I/O. */
+ VIR_DEBUG("Loop backing file path '%s' truncated in loop metadata",
file);
}
if ((fsfd = open(file, O_RDWR)) < 0) {
@@ -1036,7 +1045,6 @@ int virFileLoopDeviceAssociate(const char *file,
return lofd;
}
-
# define SYSFS_BLOCK_DIR "/sys/block"
# define NBD_DRIVER "nbd"
@@ -1160,6 +1168,7 @@ int virFileNBDDeviceAssociate(const char *file,
#else /* __linux__ */
int virFileLoopDeviceAssociate(const char *file,
+ const char *loName G_GNUC_UNUSED,
char **dev G_GNUC_UNUSED)
{
virReportSystemError(ENOSYS,
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 8c9ad5989818..b5eb4e727548 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -151,6 +151,7 @@ int virFileUpdatePerm(const char *path,
mode_t mode_add);
int virFileLoopDeviceAssociate(const char *file,
+ const char *loName,
char **dev);
int virFileNBDDeviceAssociate(const char *file,
--
2.54.0
