Re: [PATCH 6/9] Introduce an LXC specific public API & library

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/21/2012 10:08 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
> 
> This patch introduces support for LXC specific public APIs. In
> common with what was done for QEMU, this creates a libvirt_lxc.so
> library and libvirt/libvirt-lxc.h header file.
> 
> The actual APIs are
> 
>   int virDomainLxcOpenNamespace(virDomainPtr domain,
>                                 int **fdlist,
>                                 unsigned int flags);
> 
>   int virDomainLxcEnterNamespace(virDomainPtr domain,
>                                  unsigned int flags);

This commit message signature...

> 
> which provide a way to use the setns() system call to move the
> calling process into the container's namespace. This is not
> practical to write in a generically applicable manner. The
> nearest that we could get to such an API would be an API which
> allows to pass a command + argv to be executed inside a
> container. Even if we had such a generic API, this LXC specific
> API is still useful, because it allows the caller to maintain
> the current process context, in particular any I/O streams they
> have open.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---

> @@ -152,7 +157,8 @@ todo:
>  	$(MAKE) todo.html
>  
>  hvsupport.html.in: $(srcdir)/hvsupport.pl $(srcdir)/../src/libvirt_public.syms \
> -	 $(srcdir)/../src/libvirt_qemu.syms $(srcdir)/../src/driver.h
> +	 $(srcdir)/../src/libvirt_qemu.syms $(srcdir)/../src/libvirt_lxc.syms \
> +	$(srcdir)/../src/driver.h

Indentation looks weird here, but that's just cosmetic.

> +++ b/include/libvirt/libvirt-lxc.h
> @@ -0,0 +1,50 @@
> +/* -*- c -*-
> + * libvirt-lxc.h: Interfaces specific for LXC driver
> + * Summary: lxc specific interfaces
> + * Description: Provides the interfaces of the libvirt library to handle
> + *              LXC specific methods
> + *
> + * Copyright (C) 2012 Red Hat, Inc.

2013.


> +int virDomainLxcEnterNamespace(virDomainPtr domain,
> +                               unsigned int nfdlist,
> +                               int *fdlist,
> +                               unsigned int *noldfdlist,
> +                               int **oldfdlist,
> +                               unsigned int flags);

...doesn't match the actual code.

> +++ b/python/libvirt-lxc-override-api.xml
> @@ -0,0 +1,19 @@
> +<?xml version="1.0"?>

Should we be sticking copyright comments in these types of files?  But
that's fine for a followup, since there are other files needing the same
cleanup.


> +++ b/python/libvirt-lxc-override.c
> @@ -0,0 +1,141 @@
> +/*
> + * libvir.c: this modules implements the main part of the glue of the
> + *           libvir library and the Python interpreter. It provides the
> + *           entry points where an automatically generated stub is
> + *           unpractical
> + *
> + * Copyright (C) 2011-2012 Red Hat, Inc.

Also 2013.


> +libvirt_lxc.def: $(srcdir)/libvirt_lxc.syms
> +	$(AM_V_GEN)rm -f -- $@-tmp $@ ; \
> +	printf 'EXPORTS\n' > $@-tmp && \
> +	sed -e '/^$$/d; /#/d; /:/d; /}/d; /\*/d; /LIBVIRT_/d; s/[	 ]*\(.*\)\;/    \1/g' $^ >> $@-tmp && \

Should we break this long line with multiple -e to fit in 80 columns?
But that's copy and paste, so it would be a separate cleanup.

> +++ b/src/driver.h
> @@ -915,6 +915,11 @@ typedef int
>                            unsigned long long minimum,
>                            unsigned int flags);
>  
> +typedef int
> +    (*virDrvDomainLxcOpenNamespace)(virDomainPtr dom,
> +                                    int **fdlist,
> +                                    unsigned int flags);
> +
>  /**
>   * _virDriver:
>   *
> @@ -1107,6 +1112,7 @@ struct _virDriver {
>      virDrvNodeGetCPUMap                 nodeGetCPUMap;
>      virDrvDomainFSTrim                  domainFSTrim;
>      virDrvDomainSendProcessSignal       domainSendProcessSignal;
> +    virDrvDomainLxcOpenNamespace        domainLxcOpenNamespace;

No EnterNamespace driver callback?

> +++ b/src/libvirt-lxc.c
> @@ -0,0 +1,165 @@
> +/*
> + * libvirt-lxc.c: Interfaces for the libvirt library to handle lxc-specific
> + *                 APIs.
> + *
> + * Copyright (C) 2012 Red Hat, Inc.

2013

> +++ b/src/lxc/lxc_driver.c
> @@ -4544,6 +4544,7 @@ static virDriver lxcDriver = {
>      .domainShutdown = lxcDomainShutdown, /* 1.0.1 */
>      .domainShutdownFlags = lxcDomainShutdownFlags, /* 1.0.1 */
>      .domainReboot = lxcDomainReboot, /* 1.0.1 */
> +    .domainLxcOpenNamespace = lxcDomainOpenNamespace, /* 1.0.2 */

Again, no EnterNamespace callback registration?

ACK with those issues fixed.  It made it nice that we already have
libvirt-qemu to copy from.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]