On Wed, Aug 31, 2011 at 02:02:10PM -0500, Serge Hallyn wrote: > Quoting Scott Moser (smoser@xxxxxxxxxx): > > Currently, the lxc implementation invokes 'ip' and 'ifconfig' commands > > inside a container using 'virRun'. That has the side effect of requiring > > those commands to be present and to function in a manner consistent with > > the usage. Some small roots (such as ttylinux) may not have 'ip' or > > 'ifconfig'. > > > > This patch replaces the use of these commands with usage of > > netdevice. The result is that lxc containers do not have to implement > > those commands, and lxc in libvirt is only dependent on the netdevice > > interface. > > Requiring fewer tools in the container seems like a very good thing, > especially when trying to create a tiny busybox-based container. Yep, and we already directly do this kind of thing elsewhere in libvirt without external tools, so there is precedent. > > I've tested this patch locally against the ubuntu libvirt version enough > > to verify its generally sane. I attempted to build upstream today, but > > failed with: > > /usr/bin/ld: > > ../src/.libs/libvirt_driver_qemu.a(libvirt_driver_qemu_la-qemu_domain.o): > > undefined reference to symbol 'xmlXPathRegisterNs@@LIBXML2_2.4.30 > > > > Thats probably a local issue only, but I wanted to get this patch up and > > see what others thought of it. This is ubuntu bug > > https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/828211 . > > > > diff --git a/src/lxc/veth.c b/src/lxc/veth.c > > index 34cb804..c24df91 100644 > > --- a/src/lxc/veth.c > > +++ b/src/lxc/veth.c > > @@ -12,8 +12,11 @@ > > > > #include <config.h> > > > > +#include <linux/sockios.h> > > +#include <net/if.h> > > #include <string.h> > > #include <stdio.h> > > +#include <sys/ioctl.h> > > #include <sys/types.h> > > #include <sys/wait.h> > > > > @@ -186,41 +189,49 @@ int vethDelete(const char *veth) > > * @veth: name of veth device > > * @upOrDown: 0 => down, 1 => up > > * > > - * Enables a veth device using the ifconfig command. A NULL inetAddress > > - * will cause it to be left off the command line. > > + * Enables a veth device using SIOCSIFFLAGS > > * > > - * Returns 0 on success or -1 in case of error > > + * Returns 0 on success, -1 on failure, with errno set > > */ > > int vethInterfaceUpOrDown(const char* veth, int upOrDown) > > { > > - int rc; > > - const char *argv[] = {"ifconfig", veth, NULL, NULL}; > > - int cmdResult = 0; > > + struct ifreq ifr; > > + int fd, ret; > > > > - if (0 == upOrDown) > > - argv[2] = "down"; > > - else > > - argv[2] = "up"; > > + if ((fd = socket(PF_PACKET, SOCK_DGRAM, 0)) == -1) > > + return(-1); > > > > - rc = virRun(argv, &cmdResult); > > + memset(&ifr, 0, sizeof(struct ifreq)); > > > > - if (rc != 0 || > > - (WIFEXITED(cmdResult) && WEXITSTATUS(cmdResult) != 0)) { > > - if (0 == upOrDown) > > + if (virStrcpyStatic(ifr.ifr_name, veth) == NULL) { > > + errno = EINVAL; > > + return -1; > > + } > > + > > + if ((ret = ioctl(fd, SIOCGIFFLAGS, &ifr)) == 0) { > > + if (upOrDown) > > + ifr.ifr_flags |= IFF_UP; > > + else > > + ifr.ifr_flags &= ~(IFF_UP | IFF_RUNNING); > > + > > + ret = ioctl(fd, SIOCSIFFLAGS, &ifr); > > + } > > + > > + close(fd); > > + if (ret == -1) > > + if (upOrDown == 0) > > /* > > * Prevent overwriting an error log which may be set > > * where an actual failure occurs. > > */ > > - VIR_DEBUG("Failed to disable '%s' (%d)", > > - veth, WEXITSTATUS(cmdResult)); > > + VIR_DEBUG("Failed to disable '%s'", veth); > > else > > vethError(VIR_ERR_INTERNAL_ERROR, > > - _("Failed to enable '%s' (%d)"), > > - veth, WEXITSTATUS(cmdResult)); > > - rc = -1; > > - } > > + _("Failed to enable '%s'"), veth); > > + else > > + ret = 0; > > > > - return rc; > > + return(ret); > > } > > > > /** > > @@ -279,17 +290,29 @@ int setMacAddr(const char* iface, const char* macaddr) > > * @iface: name of device > > * @new: new name of @iface > > * > > - * Changes the name of the given device with the > > - * given new name using this command: > > - * ip link set @iface name @new > > + * Changes the name of the given device. > > * > > - * Returns 0 on success or -1 in case of error > > + * Returns 0 on success, -1 on failure with errno set. > > */ > > int setInterfaceName(const char* iface, const char* new) > > { > > - const char *argv[] = { > > - "ip", "link", "set", iface, "name", new, NULL > > - }; > > + struct ifreq ifr; > > + int fd = socket(PF_PACKET, SOCK_DGRAM, 0); > > > > - return virRun(argv, NULL); > > + memset(&ifr, 0, sizeof(struct ifreq)); > > + > > + if (virStrcpyStatic(ifr.ifr_name, iface) == NULL) { > > + errno = EINVAL; > > + return -1; > > + } > > + > > + if (virStrcpyStatic(ifr.ifr_newname, new) == NULL) { > > + errno = EINVAL; > > + return -1; > > + } > > + > > + if (ioctl(fd, SIOCSIFNAME, &ifr)) > > + return -1; > > + > > + return 0; > > } > > ACK 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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list