[PATCH v2 0/1] lxc: fix problem with truncation of long path used for file backing block device
This is a contunuation of thread: "lxc: truncate LOOP_GET_STATUS64.lo_file_name for long loop backing paths" https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/NSOQD... with updated and more relevant subject. This patch fixes problem reported as https://gitlab.com/libvirt/libvirt/-/work_items/63 but before it could be posted, it required fixing or rather implementing aliase for block devices and file systems on LXC domains, commit 065337dd2c1b120be92a4960d0566eb745e8e350 and the patch posted here: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/RWKBB... Now, with the working aliases for for block devices and file systems, this change implement the this naming pattern "libvirt-$UUID-$DEVALIAS" for LOOP_GET_STATUS64.lo_file_name which is shown in output of the losetup. Example, for domain with these file systems attached, where the second one is backed by path with long file name: <filesystem type='file' accessmode='passthrough'> <driver type='loop' format='raw'/> <source file='/var/tmp/short.raw'/> <target dir='/short'/> <alias name='fs1'/> </filesystem> <filesystem type='file' accessmode='passthrough'> <driver type='loop' format='raw'/> <source file='/var/tmp/looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong.raw'/> <target dir='/long'/> <alias name='fs2'/> </filesystem> The losetup output looks like below: [root@rvm08m-3 ]# losetup -l -O +REF NAME SIZELIMIT OFFSET AUTOCLEAR RO BACK-FILE DIO LOG-SEC REF /dev/loop1 0 0 1 0 /var/tmp/looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooong.raw 0 512 libvirt-942c6580-8c49-4b8d-8e79-bba79e9709fd-fs2 /dev/loop0 0 0 1 0 /var/tmp/short.raw 0 512 libvirt-942c6580-8c49-4b8d-8e79-bba79e9709fd-fs1 The last column is where the LOOP_GET_STATUS64.lo_file_name can be seen. This patch does not add support for custom, user specified aliases because: 1. At the start of the domain, user aliases are silently dropped during XML parsing and are auto-generated as fs0, fs1, ... VIR_DOMAIN_DEF_FEATURE_USER_ALIAS is not set for LXC domains. 2. Attaching, hot-plug of a file system to live LXC domain is not supported: "error: unsupported configuration: device type 'filesystem' cannot be attached"
From: Radoslaw Smigielski <rsmigiel@redhat.com> 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 <rsmigiel@redhat.com> --- 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
participants (1)
-
rsmigiel＠redhat.com