Re: [libvirt] [PATCH V3] Add proxy FS support to libvirt

> docs/formatdomain.html.in | 2 + > src/conf/domain_conf.c | 3 +- > src/conf/domain_conf.h | 2 +- Opps, forgot to mention in previous reviews that you must update docs/schemas/domaincommon.rng to list the new attribute > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 5472267..88b7e87 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -144,7 +144,6 @@ VIR_ENUM_IMPL(qemuCaps, QEMU_CAPS_LAST, > "ich9-ahci", > "no-acpi", > "fsdev-readonly", > - > "virtio-blk-pci.scsi", /* 80 */ > "blk-sg-io", > "drive-copy-on-read", Bogus whitespace removal > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index 4ca3047..3f78635 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -46,6 +46,7 @@ > #include <sys/utsname.h> > #include <sys/stat.h> > #include <fcntl.h> > +#include <libgen.h> You don't use any functions from this header, so it can be removed. > @@ -2632,9 +2634,59 @@ error: > return NULL; > } > > +/* > + * Invokes the Proxy Helper with one of the socketpair as its parameter > + * > + */ > +static int qemuInvokeProxyHelper(const char *emulator, int sock, const char *path) > +{ > +#define HELPER "virtfs-proxy-helper" > + int ret_val, status; > + virCommandPtr cmd; > + char *helper, *dname, *dp; > + > + dp = strdup(emulator); > + if (!dp) { > + virReportOOMError(); > + return -1; This doesn't close 'sock' like other error paths do

Hmm, right here we are in the middle of constructing the command line args for QEMU. We should not be launching at processes at this point, not least since we could be running this from the test suite.

Also you don't appear to have any code to tell the daemonized proxy to shutdown when QEMU is stopped ?

We really need to push the invocation of the proxy up into the code which actually launches QEMU, and merely have this part construct the command line arguments.