When user-namespace is enabled we are not allowed
to mount block/NBD devices.
Instead, mount /dev/nbdX to /run/libvirt/lxc/<domain>.root
and set:
fs->src->path = /run/libvirt/lxc/<domain>.root
fs->type = VIR_DOMAIN_FS_TYPE_MOUNT
---
src/lxc/lxc_container.c | 53 ------------------------------------------------
src/lxc/lxc_controller.c | 49 +++++++++++++++++++++++++++++---------------
2 files changed, 33 insertions(+), 69 deletions(-)
diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index 3b8cb966e..420bb20ab 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -658,55 +658,6 @@ static int lxcContainerResolveSymlinks(virDomainFSDefPtr fs, bool gentle)
return 0;
}
-static int lxcContainerPrepareRoot(virDomainDefPtr def,
- virDomainFSDefPtr root,
- const char *sec_mount_options)
-{
- char *dst;
- char *tmp;
-
- VIR_DEBUG("Prepare root %d", root->type);
-
- if (root->type == VIR_DOMAIN_FS_TYPE_MOUNT)
- return 0;
-
- if (root->type == VIR_DOMAIN_FS_TYPE_FILE) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Unexpected root filesystem without loop device"));
- return -1;
- }
-
- if (root->type != VIR_DOMAIN_FS_TYPE_BLOCK) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unsupported root filesystem type %s"),
- virDomainFSTypeToString(root->type));
- return -1;
- }
-
- if (lxcContainerResolveSymlinks(root, false) < 0)
- return -1;
-
- if (virAsprintf(&dst, "%s/%s.root",
- LXC_STATE_DIR, def->name) < 0)
- return -1;
-
- tmp = root->dst;
- root->dst = dst;
-
- if (lxcContainerMountFSBlock(root, "", sec_mount_options) < 0) {
- root->dst = tmp;
- VIR_FREE(dst);
- return -1;
- }
-
- root->dst = tmp;
- root->type = VIR_DOMAIN_FS_TYPE_MOUNT;
- VIR_FREE(root->src->path);
- root->src->path = dst;
-
- return 0;
-}
-
static int lxcContainerPivotRoot(virDomainFSDefPtr root)
{
int ret;
@@ -1755,10 +1706,6 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef,
if (virFileResolveAllLinks(LXC_STATE_DIR, &stateDir) < 0)
goto cleanup;
- /* Ensure the root filesystem is mounted */
- if (lxcContainerPrepareRoot(vmDef, root, sec_mount_options) < 0)
- goto cleanup;
-
/* Gives us a private root, leaving all parent OS mounts on /.oldroot */
if (lxcContainerPivotRoot(root) < 0)
goto cleanup;
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 61d9ed07b..d1ae60b1d 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -530,9 +530,12 @@ static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl,
}
-static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
+static int virLXCControllerSetupNBDDeviceFS(virLXCControllerPtr ctrl,
+ virDomainFSDefPtr fs)
{
- char *dev;
+ char *dev, *dst, *tmp, *sec_mount_options;
+ virDomainDefPtr def = ctrl->def;
+ virSecurityManagerPtr securityDriver = ctrl->securityManager;
if (fs->format <= VIR_STORAGE_FILE_NONE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
@@ -540,22 +543,42 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
return -1;
}
+ if (virAsprintf(&dst, "%s/%s.root/",
+ LXC_STATE_DIR, def->name) < 0)
+ return -1;
+
+ if (!(sec_mount_options = virSecurityManagerGetMountOptions(securityDriver, def)))
+ return -1;
+
if (virFileNBDDeviceAssociate(fs->src->path,
fs->format,
fs->readonly,
&dev) < 0)
return -1;
- VIR_DEBUG("Changing fs %s to use type=block for dev %s",
- fs->src->path, dev);
- /*
- * We now change it into a block device type, so that
- * the rest of container setup 'just works'
- */
- fs->type = VIR_DOMAIN_FS_TYPE_BLOCK;
VIR_FREE(fs->src->path);
fs->src->path = dev;
+ tmp = fs->dst;
+ fs->dst = dst;
+
+ if (lxcContainerMountFSBlock(fs, "", sec_mount_options) < 0) {
+ fs->dst = tmp;
+ VIR_FREE(dst);
+ return -1;
+ }
+
+ fs->dst = tmp;
+ fs->type = VIR_DOMAIN_FS_TYPE_MOUNT;
+
+ /* The NBD device will be cleaned up while the cgroup will end.
+ * For this we need to remember the qemu-nbd pid and add it to
+ * the cgroup*/
+ if (virLXCControllerAppendNBDPids(ctrl, fs->src->path) < 0)
+
+ VIR_FREE(fs->src->path);
+ fs->src->path = dst;
+
return 0;
}
@@ -637,13 +660,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
}
ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd;
} else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) {
- if (virLXCControllerSetupNBDDeviceFS(fs) < 0)
- goto cleanup;
-
- /* The NBD device will be cleaned up while the cgroup will end.
- * For this we need to remember the qemu-nbd pid and add it to
- * the cgroup*/
- if (virLXCControllerAppendNBDPids(ctrl, fs->src->path) < 0)
+ if (virLXCControllerSetupNBDDeviceFS(ctrl, fs) < 0)
goto cleanup;
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.14.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/15/2018 05:25 PM, Radostin Stoyanov wrote:
> When user-namespace is enabled we are not allowed
> to mount block/NBD devices.
>
> Instead, mount /dev/nbdX to /run/libvirt/lxc/<domain>.root
> and set:
>
> fs->src->path = /run/libvirt/lxc/<domain>.root
> fs->type = VIR_DOMAIN_FS_TYPE_MOUNT
> ---
> src/lxc/lxc_container.c | 53 ------------------------------------------------
> src/lxc/lxc_controller.c | 49 +++++++++++++++++++++++++++++---------------
> 2 files changed, 33 insertions(+), 69 deletions(-)
>
> diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
> index 3b8cb966e..420bb20ab 100644
> --- a/src/lxc/lxc_container.c
> +++ b/src/lxc/lxc_container.c
> @@ -658,55 +658,6 @@ static int lxcContainerResolveSymlinks(virDomainFSDefPtr fs, bool gentle)
> return 0;
> }
>
Why is this being removed? Not clear from commit message...
> -static int lxcContainerPrepareRoot(virDomainDefPtr def,
> - virDomainFSDefPtr root,
> - const char *sec_mount_options)
> -{
> - char *dst;
> - char *tmp;
> -
> - VIR_DEBUG("Prepare root %d", root->type);
> -
> - if (root->type == VIR_DOMAIN_FS_TYPE_MOUNT)
> - return 0;
> -
> - if (root->type == VIR_DOMAIN_FS_TYPE_FILE) {
> - virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> - _("Unexpected root filesystem without loop device"));
> - return -1;
> - }
> -
> - if (root->type != VIR_DOMAIN_FS_TYPE_BLOCK) {
> - virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> - _("Unsupported root filesystem type %s"),
> - virDomainFSTypeToString(root->type));
> - return -1;
> - }
> -
> - if (lxcContainerResolveSymlinks(root, false) < 0)
> - return -1;
> -
> - if (virAsprintf(&dst, "%s/%s.root",
> - LXC_STATE_DIR, def->name) < 0)
> - return -1;
> -
> - tmp = root->dst;
> - root->dst = dst;
> -
> - if (lxcContainerMountFSBlock(root, "", sec_mount_options) < 0) {
> - root->dst = tmp;
> - VIR_FREE(dst);
> - return -1;
> - }
> -
> - root->dst = tmp;
> - root->type = VIR_DOMAIN_FS_TYPE_MOUNT;
> - VIR_FREE(root->src->path);
> - root->src->path = dst;
> -
> - return 0;
> -}
> -
> static int lxcContainerPivotRoot(virDomainFSDefPtr root)
> {
> int ret;
> @@ -1755,10 +1706,6 @@ static int lxcContainerSetupPivotRoot(virDomainDefPtr vmDef,
> if (virFileResolveAllLinks(LXC_STATE_DIR, &stateDir) < 0)
> goto cleanup;
>
> - /* Ensure the root filesystem is mounted */
> - if (lxcContainerPrepareRoot(vmDef, root, sec_mount_options) < 0)
> - goto cleanup;
> -
> /* Gives us a private root, leaving all parent OS mounts on /.oldroot */
> if (lxcContainerPivotRoot(root) < 0)
> goto cleanup;
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index 61d9ed07b..d1ae60b1d 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -530,9 +530,12 @@ static int virLXCControllerAppendNBDPids(virLXCControllerPtr ctrl,
> }
>
>
> -static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
> +static int virLXCControllerSetupNBDDeviceFS(virLXCControllerPtr ctrl,
> + virDomainFSDefPtr fs)
> {
> - char *dev;
> + char *dev, *dst, *tmp, *sec_mount_options;
There are those that prefer one per line.
> + virDomainDefPtr def = ctrl->def;
> + virSecurityManagerPtr securityDriver = ctrl->securityManager;
>
> if (fs->format <= VIR_STORAGE_FILE_NONE) {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> @@ -540,22 +543,42 @@ static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs)
> return -1;
> }
>
> + if (virAsprintf(&dst, "%s/%s.root/",
> + LXC_STATE_DIR, def->name) < 0)
> + return -1;
> +
> + if (!(sec_mount_options = virSecurityManagerGetMountOptions(securityDriver, def)))
> + return -1;
This would leak dst
> +
> if (virFileNBDDeviceAssociate(fs->src->path,
> fs->format,
> fs->readonly,
> &dev) < 0)
> return -1;
This would leak dst, sec_mount_options
>
> - VIR_DEBUG("Changing fs %s to use type=block for dev %s",
> - fs->src->path, dev);
> - /*
> - * We now change it into a block device type, so that
> - * the rest of container setup 'just works'
> - */
> - fs->type = VIR_DOMAIN_FS_TYPE_BLOCK;
> VIR_FREE(fs->src->path);
> fs->src->path = dev;
>
> + tmp = fs->dst;
> + fs->dst = dst;
> +
> + if (lxcContainerMountFSBlock(fs, "", sec_mount_options) < 0) {
> + fs->dst = tmp;
> + VIR_FREE(dst);
This would leak sec_mount_options
> + return -1;
> + }
> +
> + fs->dst = tmp;
> + fs->type = VIR_DOMAIN_FS_TYPE_MOUNT;
> +
> + /* The NBD device will be cleaned up while the cgroup will end.
> + * For this we need to remember the qemu-nbd pid and add it to
> + * the cgroup*/
> + if (virLXCControllerAppendNBDPids(ctrl, fs->src->path) < 0)
Bad cut-n-paste...
What would we do if < 0?? VIR_FREE(fs->src->path);
otherwise, we'd leak it and just set @dst to fs->src->path
> +
> + VIR_FREE(fs->src->path);
> + fs->src->path = dst;
> +
still leaving sec_mount_options leaked.
Perhaps real cleanup type processing should be used instead and an
initialized "int ret = -1;" with the ret = 0 once everything is
successful. You can use VIR_STEAL_PTR for dst and dev and have cleanup
have the VIR_FREE for dev, dst, and sec_mount_options assuming they are
all initialized to NULL.
> return 0;
> }
>
> @@ -637,13 +660,7 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl)
> }
> ctrl->loopDevFds[ctrl->nloopDevs - 1] = fd;
> } else if (fs->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) {
> - if (virLXCControllerSetupNBDDeviceFS(fs) < 0)
> - goto cleanup;
> -
> - /* The NBD device will be cleaned up while the cgroup will end.
> - * For this we need to remember the qemu-nbd pid and add it to
> - * the cgroup*/
> - if (virLXCControllerAppendNBDPids(ctrl, fs->src->path) < 0)
> + if (virLXCControllerSetupNBDDeviceFS(ctrl, fs) < 0)
> goto cleanup;
Perhaps a patch between this and the last one to move the call to
virLXCControllerAppendNBDPids into virLXCControllerSetupNBDDeviceFS
would be appropriate and return -1 instead of cleanup just before return
0.
That may make this followup patch a bit easier to follow.
John
> } else {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 04/26/2018 08:09 PM, John Ferlan wrote: > > > On 04/15/2018 05:25 PM, Radostin Stoyanov wrote: >> When user-namespace is enabled we are not allowed >> to mount block/NBD devices. >> >> Instead, mount /dev/nbdX to /run/libvirt/lxc/<domain>.root >> and set: >> >> fs->src->path = /run/libvirt/lxc/<domain>.root >> fs->type = VIR_DOMAIN_FS_TYPE_MOUNT >> --- >> src/lxc/lxc_container.c | 53 ------------------------------------------------ >> src/lxc/lxc_controller.c | 49 +++++++++++++++++++++++++++++--------------- >> 2 files changed, 33 insertions(+), 69 deletions(-) As John points out, couple of memleaks and also explanation why you're removing PrepareRoot()? Otherwise looking good. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.