Re: [libvirt] [PATCH 2/3] Auto-add a root <filesystem> element to LXC containers on startup

On 04/03/2013 10:02 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > Currently the LXC container code has two codepaths, depending on > whether there is a <filesystem> element with a target path of '/'. > If we automatically add a <filesystem> device with src=/ and dst=/, > for any container which has not specified a root filesystem, then > we only need one codepath for setting up the filesystem. > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > --- > src/lxc/lxc_container.c | 113 +++--------------------------------------------- > src/lxc/lxc_process.c | 38 ++++++++++++++++ > 2 files changed, 44 insertions(+), 107 deletions(-) Nice cleanup! > +++ b/src/lxc/lxc_process.c > @@ -981,6 +981,41 @@ virLXCProcessReadLogOutput(virDomainObjPtr vm, > return ret; > } > > + > +static int > +virLXCProcessEnsureRootFS(virDomainObjPtr vm) > +{ > + virDomainFSDefPtr root = virDomainGetRootFilesystem(vm->def); > + > + if (root) > + return 0; > + > + if (VIR_ALLOC(root) < 0) > + goto no_memory; > + > + root->type = VIR_DOMAIN_FS_TYPE_MOUNT; > + > + if (!(root->src = strdup("/")) || > + !(root->dst = strdup("/"))) > + goto no_memory; Might be a fun merge conflict, depending on whether this or VIR_STRDUP gets merged first. :) > + > + if (VIR_EXPAND_N(vm->def->fss, > + vm->def->nfss, 1) < 0) > + goto no_memory; > + > + memmove(vm->def->fss + 1, > + vm->def->fss, > + vm->def->nfss * sizeof(virDomainFSDefPtr)); > + vm->def->fss[0] = root; Instead of VIR_EXPAND_N/memmove, you should use VIR_INSERT_ELEMENT. I like the concept of the patch, but I'm debating whether to ack this or require a v2 just so we make sure the VIR_INSERT_ELEMENT usage is correct.