Re: [libvirt] [PATCH 8/8] qemu ns: Create chardev backends more frequently

On 06/22/2017 12:18 PM, Michal Privoznik wrote: > Currently, the only type of chardev that we create the backend > for in the namespace is type='dev'. This is not enough, other > backends might have files under /dev too. For instance channels > might have a unix socket under /dev (well, bind mounted under > /dev from a different place). > > Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; > --- > src/qemu/qemu_domain.c | 25 +++++++++++++++++-------- > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 51779c535..65eec26dd 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -7732,7 +7732,7 @@ qemuDomainCreateDeviceRecursive(const char *device, > > isLink = S_ISLNK(sb.st_mode); > isDev = S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode); > - isReg = S_ISREG(sb.st_mode); > + isReg = S_ISREG(sb.st_mode) || S_ISFIFO(sb.st_mode) || S_ISSOCK(sb.st_mode); > > /* Here, @device might be whatever path in the system. We > * should create the path in the namespace iff it's "/dev" > @@ -8129,11 +8129,17 @@ qemuDomainSetupChardev(virDomainDefPtr def ATTRIBUTE_UNUSED, > void *opaque) > { > const struct qemuDomainCreateDeviceData *data = opaque; > + const char *path = NULL; > > - if (dev->source->type != VIR_DOMAIN_CHR_TYPE_DEV) > + if (!(path = virDomainChrSourceDefPath(dev->source))) > return 0; > > - return qemuDomainCreateDevice(dev->source->data.file.path, data, false); > + /* Socket created by qemu. It doesn't exist upfront. */ > + if (dev->source->type == VIR_DOMAIN_CHR_TYPE_UNIX && > + dev->source->data.nix.listen) > + return 0; > + > + return qemuDomainCreateDevice(path, data, true); > } > > > @@ -8531,7 +8537,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, > bool delDevice = false; > bool isLink = S_ISLNK(data->sb.st_mode); > bool isDev = S_ISCHR(data->sb.st_mode) || S_ISBLK(data->sb.st_mode); > - bool isReg = S_ISREG(data->sb.st_mode); > + bool isReg = S_ISREG(data->sb.st_mode) || S_ISFIFO(data->sb.st_mode) || S_ISSOCK(data->sb.st_mode); > > qemuSecurityPostFork(data->driver->securityManager); > > @@ -8576,7 +8582,7 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED, > * as its backing chain. This might however clash here. > * Therefore do the cleanup here. */ > if (umount(data->file) < 0 && > - errno != ENOENT) { > + errno != ENOENT && errno != EINVAL) { Is this something that should be separate or as a result of the current changes? Should it go with the previous patch since that's where it's introduced?

Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt;