On 14.11.2016 17:55, Daniel P. Berrange wrote:
> On Mon, Nov 14, 2016 at 05:43:30PM +0100, Michal Privoznik wrote:
>> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
>> ---
>> src/qemu/qemu_domain.c | 233 ++++++++++++++++++++++++++++++++++++++++++++++++
>> src/qemu/qemu_domain.h | 8 ++
>> src/qemu/qemu_process.c | 13 +++
>> 3 files changed, 254 insertions(+)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 8cba755..3a0170c 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -55,6 +55,7 @@
>>
>> #include <sys/time.h>
>> #include <fcntl.h>
>> +#include <sys/mount.h>
>>
>> #include <libxml/xpathInternals.h>
>>
>> @@ -86,6 +87,21 @@ VIR_ENUM_IMPL(qemuDomainAsyncJob, QEMU_ASYNC_JOB_LAST,
>> "start",
>> );
>>
>> +#define QEMU_DEV_MAJ_MEMORY 1
>> +#define QEMU_DEV_MAJ_TTY 5
>> +#define QEMU_DEV_MAJ_KVM 10
>> +#define QEMU_DEV_MAJ_PTY 136
>> +
>> +#define QEMU_DEV_MIN_CONSOLE 1
>> +#define QEMU_DEV_MIN_FULL 7
>> +#define QEMU_DEV_MIN_FUSE 229
>> +#define QEMU_DEV_MIN_KVM 232
>> +#define QEMU_DEV_MIN_NULL 3
>> +#define QEMU_DEV_MIN_PTMX 2
>> +#define QEMU_DEV_MIN_RANDOM 8
>> +#define QEMU_DEV_MIN_TTY 0
>> +#define QEMU_DEV_MIN_URANDOM 9
>> +#define QEMU_DEV_MIN_ZERO 5
>>
>> struct _qemuDomainLogContext {
>> int refs;
>> @@ -6658,3 +6674,220 @@ qemuDomainSupportsVideoVga(virDomainVideoDefPtr video,
>>
>> return true;
>> }
>> +
>> +
>> +static int
>> +qemuDomainPopulateDevices(virQEMUDriverPtr driver ATTRIBUTE_UNUSED,
>> + virDomainObjPtr vm ATTRIBUTE_UNUSED,
>> + const char *path)
>> +{
>> + int ret = -1;
>> + virFileDevices devs[] = {
>> + { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_NULL, 0666, "/null" },
>> + { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_ZERO, 0666, "/zero" },
>> + { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_FULL, 0666, "/full" },
>> + { QEMU_DEV_MAJ_KVM, QEMU_DEV_MIN_KVM, 0660, "/kvm"},
>> + { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_RANDOM, 0666, "/random" },
>> + { QEMU_DEV_MAJ_MEMORY, QEMU_DEV_MIN_URANDOM, 0666, "/urandom" },
>> + { QEMU_DEV_MAJ_TTY, QEMU_DEV_MIN_TTY, 0666, "/tty" },
>> + { 0, 0, 0, NULL},
>> + };
>> +
>> + if (virFilePopulateDevices(path, devs) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + cleanup:
>> + return ret;
>> +}
>> +
>> +
>> +static int
>> +qemuDomainSetupDev(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + const char *path)
>> +{
>> + char *mount_options = NULL;
>> + char *opts = NULL;
>> + int ret = -1;
>> +
>> + VIR_DEBUG("Setting up /dev/ for domain %s", vm->def->name);
>> +
>> + mount_options = virSecurityManagerGetMountOptions(driver->securityManager,
>> + vm->def);
>> +
>> + if (!mount_options &&
>> + VIR_STRDUP(mount_options, "") < 0)
>> + goto cleanup;
>> +
>> + /*
>> + * tmpfs is limited to 64kb, since we only have device nodes in there
>> + * and don't want to DOS the entire OS RAM usage
>> + */
>> + if (virAsprintf(&opts,
>> + "mode=755,size=65536%s", mount_options) < 0)
>> + goto cleanup;
>> +
>> + if (virFileSetupDev(path, opts) < 0)
>> + goto cleanup;
>> +
>> + if (qemuDomainPopulateDevices(driver, vm, path) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + cleanup:
>> + VIR_FREE(opts);
>> + VIR_FREE(mount_options);
>> + return ret;
>> +}
>> +
>> +
>> +static int
>> +qemuDomainSetupDevPTS(virQEMUDriverPtr driver,
>> + virDomainObjPtr vm,
>> + const char *path)
>> +{
>> + char *mount_options = NULL;
>> + char *opts = NULL;
>> + int ret = -1;
>> + const gid_t ptsgid = 5;
>> +
>> + mount_options = virSecurityManagerGetMountOptions(driver->securityManager,
>> + vm->def);
>> +
>> + if (!mount_options &&
>> + VIR_STRDUP(mount_options, "") < 0)
>> + goto cleanup;
>> +
>> + if (virAsprintf(&opts, "newinstance,ptmxmode=0666,mode=0620,gid=%u%s",
>> + ptsgid, (mount_options ? mount_options : "")) < 0)
>> + goto cleanup;
>> +
>> + if (virFileSetupDevPTS(path, opts, NULL) < 0)
>> + goto cleanup;
>> +
>> + ret = 0;
>> + cleanup:
>> + VIR_FREE(opts);
>> + VIR_FREE(mount_options);
>> + return ret;
>> +}
>
> This is going to cause problems - the /dev/pts/NNN nodes that
> QEMU has open (for its chardev backend) are exposed in the
> guest XML and are intended to be accessed by other processes
> on the host. By setting up a new /dev/pts mount with newinstance,
> the /dev/pts/NNN nodes for chardevs will be invisible to the host
> OS.
>
> So while you want to replace /dev completely, you need to keep
> the original /dev/pts mount unchanged. Practically speaking
> this means that after doing the unshare() you need to use
> mount() with MS_MOVE to move /dev/pts somewhere safe, then
> unmount the old /dev, mount our new /dev instance and then
> finally MS_MOVE the saved /dev/pts back in place.
Ah, good point. I will fix this.
Thanks for such quick review.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list