Re: [libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces

2015-08-28 Thread Daniel P. Berrange 
On Fri, Aug 28, 2015 at 06:40:39AM -0400, John Ferlan wrote:
 
 
 On 08/26/2015 09:06 PM, Michal Privoznik wrote:
  Now that virProcessSetNamespaces() does accept FD list in the
  correct format, we can simply turn lxcAttachNS into calling
  virProcessSetNamespaces().
  
  Signed-off-by: Michal Privoznik mpriv...@redhat.com
  ---
   src/lxc/lxc_container.c | 22 +++---
   1 file changed, 3 insertions(+), 19 deletions(-)
  
  diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
  index feb8fad..eb7cad6 100644
  --- a/src/lxc/lxc_container.c
  +++ b/src/lxc/lxc_container.c
  @@ -2184,25 +2184,9 @@ static int 
  lxcContainerDropCapabilities(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
   static int lxcAttachNS(int *ns_fd)
   {
  -size_t i;
  -if (ns_fd)
  -for (i = 0; i  VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) {
  -if (ns_fd[i]  0)
  -continue;
  -VIR_DEBUG(Setting into namespace\n);
  -/* We get EINVAL if new NS is same as the current
  - * NS, or if the fd namespace doesn't match the
  - * type passed to setns()'s second param. Since we
  - * pass 0, we know the EINVAL is harmless
  - */
  -if (setns(ns_fd[i], 0)  0 
  -errno != EINVAL) {
  -virReportSystemError(errno, _(failed to set namespace 
  '%s'),
  - virLXCDomainNamespaceTypeToString(i));
  -return -1;
  -}
  -VIR_FORCE_CLOSE(ns_fd[i]);
  -}
  +if (ns_fd 
  +virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)  0)
 
 Coverity wasn't very happy with this one - I got:
 
 (1) Event suspicious_sizeof:  Passing argument ns_fd of type int *
 and argument VIR_LXC_DOMAIN_NAMESPACE_LAST to function
 virProcessSetNamespaces is suspicious because a multiple of sizeof
 (int) /*4*/ is expected.

IIUC, coverity is saying here that 'size_t nfdlist' is supposed to
be a multiple of sizeof(int), which is clearly wrong. it is the
number of int elements in the array, so sizeof() doesn't apply.

I don't see any code change required here at all, I think coverity
is simply wrong.


Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces

2015-08-28 Thread John Ferlan 


On 08/26/2015 09:06 PM, Michal Privoznik wrote:
 Now that virProcessSetNamespaces() does accept FD list in the
 correct format, we can simply turn lxcAttachNS into calling
 virProcessSetNamespaces().
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/lxc/lxc_container.c | 22 +++---
  1 file changed, 3 insertions(+), 19 deletions(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index feb8fad..eb7cad6 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -2184,25 +2184,9 @@ static int 
 lxcContainerDropCapabilities(virDomainDefPtr def ATTRIBUTE_UNUSED,
   */
  static int lxcAttachNS(int *ns_fd)
  {
 -size_t i;
 -if (ns_fd)
 -for (i = 0; i  VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) {
 -if (ns_fd[i]  0)
 -continue;
 -VIR_DEBUG(Setting into namespace\n);
 -/* We get EINVAL if new NS is same as the current
 - * NS, or if the fd namespace doesn't match the
 - * type passed to setns()'s second param. Since we
 - * pass 0, we know the EINVAL is harmless
 - */
 -if (setns(ns_fd[i], 0)  0 
 -errno != EINVAL) {
 -virReportSystemError(errno, _(failed to set namespace 
 '%s'),
 - virLXCDomainNamespaceTypeToString(i));
 -return -1;
 -}
 -VIR_FORCE_CLOSE(ns_fd[i]);
 -}
 +if (ns_fd 
 +virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)  0)

Coverity wasn't very happy with this one - I got:

(1) Event suspicious_sizeof:Passing argument ns_fd of type int *
and argument VIR_LXC_DOMAIN_NAMESPACE_LAST to function
virProcessSetNamespaces is suspicious because a multiple of sizeof
(int) /*4*/ is expected.

Changing 'arg1' to virProcessSetNamespaces from size_t to unsigned int
cleared the error - whether that's right or not, I'm not sure.  I do
note the only other caller virDomainLxcEnterNamespace passes an
'unsigned int' which is why I tried that first.

John
 +return -1;
  return 0;
  }
  
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces

2015-08-27 Thread Daniel P. Berrange 
On Thu, Aug 27, 2015 at 03:06:53AM +0200, Michal Privoznik wrote:
 Now that virProcessSetNamespaces() does accept FD list in the
 correct format, we can simply turn lxcAttachNS into calling
 virProcessSetNamespaces().
 
 Signed-off-by: Michal Privoznik mpriv...@redhat.com
 ---
  src/lxc/lxc_container.c | 22 +++---
  1 file changed, 3 insertions(+), 19 deletions(-)
 
 diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
 index feb8fad..eb7cad6 100644
 --- a/src/lxc/lxc_container.c
 +++ b/src/lxc/lxc_container.c
 @@ -2184,25 +2184,9 @@ static int 
 lxcContainerDropCapabilities(virDomainDefPtr def ATTRIBUTE_UNUSED,
   */
  static int lxcAttachNS(int *ns_fd)
  {
 -size_t i;
 -if (ns_fd)
 -for (i = 0; i  VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) {
 -if (ns_fd[i]  0)
 -continue;
 -VIR_DEBUG(Setting into namespace\n);
 -/* We get EINVAL if new NS is same as the current
 - * NS, or if the fd namespace doesn't match the
 - * type passed to setns()'s second param. Since we
 - * pass 0, we know the EINVAL is harmless
 - */
 -if (setns(ns_fd[i], 0)  0 
 -errno != EINVAL) {
 -virReportSystemError(errno, _(failed to set namespace 
 '%s'),
 - virLXCDomainNamespaceTypeToString(i));
 -return -1;
 -}
 -VIR_FORCE_CLOSE(ns_fd[i]);
 -}
 +if (ns_fd 
 +virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)  0)
 +return -1;
  return 0;
  }

ACK, or we could just inline this call and get rid fo the
lxcAttachNS method entirely

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/4] lxc_container: Turn lxcAttachNS into calling virProcessSetNamespaces

2015-08-26 Thread Michal Privoznik 
Now that virProcessSetNamespaces() does accept FD list in the
correct format, we can simply turn lxcAttachNS into calling
virProcessSetNamespaces().

Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
 src/lxc/lxc_container.c | 22 +++---
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c
index feb8fad..eb7cad6 100644
--- a/src/lxc/lxc_container.c
+++ b/src/lxc/lxc_container.c
@@ -2184,25 +2184,9 @@ static int lxcContainerDropCapabilities(virDomainDefPtr 
def ATTRIBUTE_UNUSED,
  */
 static int lxcAttachNS(int *ns_fd)
 {
-size_t i;
-if (ns_fd)
-for (i = 0; i  VIR_LXC_DOMAIN_NAMESPACE_LAST; i++) {
-if (ns_fd[i]  0)
-continue;
-VIR_DEBUG(Setting into namespace\n);
-/* We get EINVAL if new NS is same as the current
- * NS, or if the fd namespace doesn't match the
- * type passed to setns()'s second param. Since we
- * pass 0, we know the EINVAL is harmless
- */
-if (setns(ns_fd[i], 0)  0 
-errno != EINVAL) {
-virReportSystemError(errno, _(failed to set namespace '%s'),
- virLXCDomainNamespaceTypeToString(i));
-return -1;
-}
-VIR_FORCE_CLOSE(ns_fd[i]);
-}
+if (ns_fd 
+virProcessSetNamespaces(VIR_LXC_DOMAIN_NAMESPACE_LAST, ns_fd)  0)
+return -1;
 return 0;
 }
 
-- 
2.4.6

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list