<div dir="ltr"><div>Thanks Daniel and Michal,<br><br></div><div>I have made the changes suggested by both of you. Mainly avoid creating new connection to lxc. Sending another version of diffs named "<h2 id=":14s" class="hP" tabindex="-1">[PATCH] Inherit namespace feature 2</h2>"<br><br></div><div>one point to mention is that reason for adding libxml in driver_la_la is because of error i got <br>/usr/bin/ld: ../src/.libs/libvirt_driver_lxc_impl.a(libvirt_driver_lxc_impl_la-lxc_domain.o): undefined reference to symbol 'xmlXPathRegisterNs@@LIBXML2_2.4.30'<br><br></div><div>and reason to add libvirt-lxc is because of the below error. Other wise lot of code would be duplicated to call virProcessGetNamespace. i.e inorder to get the pid of domain before getting namespace. instead just use virDomainLxcOpenNamespace. <br><br> 8) Test driver "lxc" ... 2015-08-08 17:14:45.434+0000: 19513: info : libvirt version: 1.2.17<br>2015-08-08 17:14:45.434+0000: 19513: error : virDriverLoadModule:73 : failed to load module /home/imran/programming/libvirt/src/.libs/libvirt_driver_lxc.so /home/imran/programming/libvirt/src/.libs/libvirt_driver_lxc.so: undefined symbol: virDomainLxcOpenNamespace<br>FAILED<br><br></div><div><br></div><div>Thanks a lot for your valuable time and experienced comments,<br></div>-imran<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 3, 2015 at 10:08 PM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Aug 03, 2015 at 10:00:29PM +0530, Imran Khan wrote:<br>
> Thanks Daniel and Michal again for your valuable inputs. Please check my<br>
> reply with text <imran> for some of your comments. And request you to help<br>
> on those.<br>
><br>
> BTW: should i reply with rework in the new patch. or i should reply to<br>
> this thread itself? Sorry i am new to the community so yet to get familiar<br>
> with etiquette.<br>
<br>
</span>Different communities often have different rules on this. For libvirt<br>
reply to the comments in the thread, but when posting a new version of<br>
a patch post the patch as a new top level thread.<br>
<br>
ie do *not* use '--in-reply-to' with git send-email<br>
<div><div class="h5"><br>
<br>
<br>
<br>
> > > + ns_fd[VIR_DOMAIN_NAMESPACE_SHARENET] = open(path, O_RDONLY);<br>
> > > + VIR_FREE(path);<br>
> > > + if (ns_fd[VIR_DOMAIN_NAMESPACE_SHARENET] < 0) {<br>
> > > + virReportSystemError(errno,<br>
> > > + _("failed to open netns %s"),<br>
> > lxcDef->ns_val[VIR_DOMAIN_NAMESPACE_SHARENET]);<br>
> > > + return -1;<br>
> > > + }<br>
> > > + }<br>
> > > + for (i = 0; i < VIR_DOMAIN_NAMESPACE_LAST; i++) {<br>
> > > + /* If not yet intialized by above: netns*/<br>
> > > + if (lxcDef->ns_type[i] && ns_fd[i] == -1) {<br>
> > > + pid = strtol(lxcDef->ns_val[i], &eptr, 10);<br>
> > > + if (*eptr != '\0' || pid < 1) {<br>
> > > + /* check if the domain is running, then set the<br>
> > namespaces<br>
> > > + * to that container<br>
> > > + */<br>
> > > + const char *ns[] = { "user", "ipc", "uts", "net",<br>
> > "pid", "mnt" };<br>
> > > + conn = virConnectOpen("lxc:///");<br>
> > > + if (!conn) {<br>
> > > + virReportError(virGetLastError()->code,<br>
> > > + _("unable to get connect to lxc %s"),<br>
> > lxcDef->ns_val[i]);<br>
> > > + rc = -1;<br>
> > > + goto cleanup;<br>
> > > + }<br>
> > > + dom = virDomainLookupByName(conn, lxcDef->ns_val[i]);<br>
> > > + if (!dom) {<br>
> > > + virReportError(virGetLastError()->code,<br>
> > > + _("Unable to lookup peer containeri<br>
> > %s"),<br>
> > > + lxcDef->ns_val[i]);<br>
> > > + rc = -1;<br>
> > > + goto cleanup;<br>
> > > + }<br>
> > > + if ((nfdlist = virDomainLxcOpenNamespace(dom, &fdlist,<br>
> > 0)) < 0) {<br>
> > > + virReportError(virGetLastError()->code,<br>
> > > + _("Unable to open %s"),<br>
> > lxcDef->ns_val[i]);<br>
> > > + rc = -1;<br>
> > > + goto cleanup;<br>
> > > + }<br>
> ><br>
> > I really hate the idea of the libvirt_lxc program opening a connection<br>
> > back to libvirtd using virConnectOpen, as that creates a circular<br>
> > dependancy. It also risks deadlock, because libvirtd will be holding<br>
> > locks while starting up the container, and you're calling back into<br>
> > the driver which may then end up acquiring the same lock.<br>
> ><br>
> > <imran>: This is where i am finding problem to code. All the driver<br>
> functions are static and to access them i might have to change the static<br>
> to non-static.which will not be inline with current design. i don't see<br>
> circular dependency with this approach as the internal connection is just<br>
> used to get the fd's. please share any approach to handle this or hope i<br>
> can keep the current code.<br>
<br>
<br>
</div></div>The code you are interested in for virDomainLxcOpenNamespace is in<br>
lxc_driver.c, which is static. This though, simply calls the global<br>
virProcessGetNamespaces() method which is non-static. So when you<br>
put the code in lxc_process.c you can directly call virProcssGetNamespaces<br>
<span class=""><br>
> > I think a better approach in general is for libvirtd lxc_process.c<br>
> > code to be responsible for getting access to all the namespace<br>
> > file descriptors. We can then pass those pre-opened file descrpitors<br>
> > down into libvirt_lxc using command line args, in the sme way that<br>
> > we pass down file descriptors for pre-opened TAP devices.<br>
> ><br>
> > eg so we end up running<br>
> ><br>
> > libvirt_lxc --netns 23 --pidns 24 --utsns 25<br>
> ><br>
> > or something like that.<br>
> ><br>
><br>
> <imran>: And useability wise its easier to provide names instead of fds. as<br>
> if the shared container goes down. the fds wont be valid.<br>
> with name a simple restart and again get the new fds with names<br>
> automatically.<br>
<br>
</span>The libvirt_lxc program is not something that eend users ever run. It<br>
is launched by libvirtd itself, so we don't need to care about usability<br>
in this particular place. The users still use XML which allows names<br>
as you have in your patch already.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
Regards,<br>
Daniel<br>
--<br>
|: <a href="http://berrange.com" rel="noreferrer" target="_blank">http://berrange.com</a> -o- <a href="http://www.flickr.com/photos/dberrange/" rel="noreferrer" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org" rel="noreferrer" target="_blank">http://libvirt.org</a> -o- <a href="http://virt-manager.org" rel="noreferrer" target="_blank">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org" rel="noreferrer" target="_blank">http://autobuild.org</a> -o- <a href="http://search.cpan.org/~danberr/" rel="noreferrer" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org" rel="noreferrer" target="_blank">http://entangle-photo.org</a> -o- <a href="http://live.gnome.org/gtk-vnc" rel="noreferrer" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
</div></div></blockquote></div><br></div>