- Devel - libvirt List Archives

[libvirt] [Patch]: spice agent-mouse support
by Zhou Peng 02 Mar '12

02 Mar '12

Signed-off-by: Zhou Peng <zhoupeng(a)nfs.iscas.ac.cn> spice agent-mouse support diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f9654f1..79d5ac9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -852,6 +852,7 @@ void virDomainGraphicsDefFree(virDomainGraphicsDefPtr def) break; case VIR_DOMAIN_GRAPHICS_TYPE_SPICE: + VIR_FREE(def->data.spice.agentmouse); VIR_FREE(def->data.spice.keymap); virDomainGraphicsAuthDefClear(&def->data.spice.auth); break; @@ -5543,6 +5544,8 @@ virDomainGraphicsDefParseXML(xmlNodePtr node, VIR_FREE(autoport); } + def->data.spice.agentmouse = virXMLPropString(node, "agentmouse"); + def->data.spice.keymap = virXMLPropString(node, "keymap"); if (virDomainGraphicsAuthDefParseXML(node, &def->data.spice.auth, @@ -11364,6 +11367,10 @@ virDomainGraphicsDefFormat(virBufferPtr buf, if (listenAddr) virBufferAsprintf(buf, " listen='%s'", listenAddr); + if (def->data.spice.agentmouse) + virBufferEscapeString(buf, " agentmouse='%s'", + def->data.spice.agentmouse); + if (def->data.spice.keymap) virBufferEscapeString(buf, " keymap='%s'", def->data.spice.keymap); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 596be4d..e55995c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1072,6 +1072,7 @@ struct _virDomainGraphicsDef { struct { int port; int tlsPort; + char *agentmouse; char *keymap; virDomainGraphicsAuthDef auth; unsigned int autoport :1; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 01adf0d..531ecbe 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5391,6 +5391,10 @@ qemuBuildCommandLine(virConnectPtr conn, VIR_FREE(netAddr); + if (def->graphics[0]->data.spice.agentmouse) + virBufferAsprintf(&opt, ",agent-mouse=%s", + def->graphics[0]->data.spice.agentmouse); + /* In the password case we set it via monitor command, to avoid * making it visible on CLI, so there's no use of password=XXX * in this bit of the code */ diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args index 681f7c2..746c116 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.args @@ -5,5 +5,5 @@ virtio-serial-pci,id=virtio-serial1,bus=pci.0,addr=0xa -hda \ /dev/HostVG/QEMUGuest1 -chardev spicevmc,id=charchannel0,name=vdagent -device \ virtserialport,bus=virtio-serial1.0,nr=3,chardev=charchannel0,id=channel0\ ,name=com.redhat.spice.0 -usb -spice port=5903,tls-port=5904,addr=127.0.0.1,\ -x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ +agent-mouse=off,x509-dir=/etc/pki/libvirt-spice,tls-channel=main -device \ virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml index 6505b55..266a4ed 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-spicevmc.xml @@ -23,7 +23,7 @@ <controller type='virtio-serial' index='1'> <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/> </controller> - <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1'> + <graphics type='spice' port='5903' tlsPort='5904' autoport='no' listen='127.0.0.1' agentmouse='off'> <channel name='main' mode='secure'/> </graphics> <channel type='spicevmc'> -- Zhou Peng

3 6

[libvirt] [PATCH] Use the same MAC address that is defined in domain XML for attached-mac field.
by Ansis Atteka 02 Mar '12

02 Mar '12

Currently libvirt sets the attached-mac to altered MAC address that has first byte set to FE. This patch will change that behavior by using the original (unaltered) MAC address from the domain XML configuration file. --- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 5 +---- src/uml/uml_conf.c | 5 +---- src/util/virnetdevtap.c | 11 ++++++++++- src/util/virnetdevtap.h | 1 + 5 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 8575d3e..3e1e031 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1766,7 +1766,7 @@ networkStartNetworkVirtual(struct network_driver *driver, } if (virNetDevTapCreateInBridgePort(network->def->bridge, &macTapIfName, network->def->mac, - 0, false, NULL, NULL) < 0) { + false, 0, false, NULL, NULL) < 0) { VIR_FREE(macTapIfName); goto err0; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5a34504..671054c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -180,7 +180,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, int tapfd = -1; int vnet_hdr = 0; bool template_ifname = false; - unsigned char tapmac[VIR_MAC_BUFLEN]; int actualType = virDomainNetGetActualType(net); if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { @@ -244,9 +243,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; - memcpy(tapmac, net->mac, VIR_MAC_BUFLEN); - tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ - err = virNetDevTapCreateInBridgePort(brname, &net->ifname, tapmac, + err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, true, vnet_hdr, true, &tapfd, virDomainNetGetActualVirtPortProfile(net)); virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0); diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index dbbbfda..c7b29a0 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -127,7 +127,6 @@ umlConnectTapDevice(virConnectPtr conn, const char *bridge) { bool template_ifname = false; - unsigned char tapmac[VIR_MAC_BUFLEN]; if (!net->ifname || STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || @@ -139,9 +138,7 @@ umlConnectTapDevice(virConnectPtr conn, template_ifname = true; } - memcpy(tapmac, net->mac, VIR_MAC_BUFLEN); - tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ - if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, tapmac, + if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, true, 0, true, NULL, virDomainNetGetActualVirtPortProfile(net)) < 0) { if (template_ifname) diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 0fce08d..868ba57 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -22,6 +22,7 @@ #include <config.h> +#include "virmacaddr.h" #include "virnetdevtap.h" #include "virnetdev.h" #include "virnetdevbridge.h" @@ -248,6 +249,7 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) * @brname: the bridge name * @ifname: the interface name (or name template) * @macaddr: desired MAC address (VIR_MAC_BUFLEN long) + * @discourage: whether bridge should be discouraged from using macaddr * @vnet_hdr: whether to try enabling IFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device * @virtPortProfile: bridge/port specific configuration @@ -265,11 +267,14 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, + bool discourage, int vnet_hdr, bool up, int *tapfd, virNetDevVPortProfilePtr virtPortProfile) { + unsigned char tapmac[VIR_MAC_BUFLEN]; + if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0) return -1; @@ -279,7 +284,11 @@ int virNetDevTapCreateInBridgePort(const char *brname, * seeing the kernel allocate random MAC for the TAP * device before we set our static MAC. */ - if (virNetDevSetMAC(*ifname, macaddr) < 0) + memcpy(tapmac, macaddr, VIR_MAC_BUFLEN); + if (discourage) + tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ + + if (virNetDevSetMAC(*ifname, tapmac) < 0) goto error; /* We need to set the interface MTU before adding it diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index 918f3dc..fc50e22 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -37,6 +37,7 @@ int virNetDevTapDelete(const char *ifname) int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, + bool discourage, int vnet_hdr, bool up, int *tapfd, -- 1.7.5.4

3 12

[libvirt] [PATCH] util: combine three bools in virNetDevTapCreateInBridgePort into one flags
by Laine Stump 02 Mar '12

02 Mar '12

With an additional new bool added to determine whether or not to discourage the use of the supplied MAC address by the bridge itself, virNetDevTapCreateInBridgePort had three booleans (well, 2 bools and an int used as a bool) in the arg list, which made it increasingly difficult to follow what was going on. This patch combines those three into a single flags arg, which not only shortens the arg list, but makes it more self-documenting. --- Does this make more sense as a PATCH 2/1 to be associated with the first patch in this thread: http://www.redhat.com/archives/libvir-list/2012-February/msg00760.html or should I squash them both together? (I'm leaning towards two separate patches, but could be convinced either way) src/network/bridge_driver.c | 3 +- src/qemu/qemu_command.c | 13 ++++++----- src/uml/uml_conf.c | 8 +++--- src/util/virnetdevtap.c | 46 +++++++++++++++++++++++++----------------- src/util/virnetdevtap.h | 20 +++++++++++++----- 5 files changed, 54 insertions(+), 36 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 3e1e031..cf75d26 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1766,7 +1766,8 @@ networkStartNetworkVirtual(struct network_driver *driver, } if (virNetDevTapCreateInBridgePort(network->def->bridge, &macTapIfName, network->def->mac, - false, 0, false, NULL, NULL) < 0) { + NULL, NULL, + VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) { VIR_FREE(macTapIfName); goto err0; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7e121c7..acfd38c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -178,7 +178,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, char *brname = NULL; int err; int tapfd = -1; - int vnet_hdr = 0; + unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; bool template_ifname = false; int actualType = virDomainNetGetActualType(net); @@ -240,12 +240,13 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) && - net->model && STREQ(net->model, "virtio")) - vnet_hdr = 1; + net->model && STREQ(net->model, "virtio")) { + tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; + } - err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, true, - vnet_hdr, true, &tapfd, - virDomainNetGetActualVirtPortProfile(net)); + err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, &tapfd, + virDomainNetGetActualVirtPortProfile(net), + tap_create_flags); virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0); if (err < 0) { if (template_ifname) diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index c7b29a0..89fdd9f 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -1,7 +1,7 @@ /* * uml_conf.c: UML driver configuration * - * Copyright (C) 2006-2011 Red Hat, Inc. + * Copyright (C) 2006-2012 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -138,9 +138,9 @@ umlConnectTapDevice(virConnectPtr conn, template_ifname = true; } - if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, true, - 0, true, NULL, - virDomainNetGetActualVirtPortProfile(net)) < 0) { + if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, NULL, + virDomainNetGetActualVirtPortProfile(net), + VIR_NETDEV_TAP_CREATE_IFUP) < 0) { if (template_ifname) VIR_FREE(net->ifname); goto error; diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 868ba57..fb0a8d2 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -107,22 +107,25 @@ virNetDevProbeVnetHdr(int tapfd) #ifdef TUNSETIFF /** - * brCreateTap: + * virNetDevTapCreate: * @ifname: the interface name - * @vnet_hr: whether to try enabling IFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device + * @flags: OR of virNetDevTapCreateFlags. Only one flag is recognized: + * + * VIR_NETDEV_TAP_CREATE_VNET_HDR + * - Enable IFF_VNET_HDR on the tap device * * Creates a tap interface. * If the @tapfd parameter is supplied, the open tap device file * descriptor will be returned, otherwise the TAP device will be made - * persistent and closed. The caller must use brDeleteTap to remove - * a persistent TAP devices when it is no longer needed. + * persistent and closed. The caller must use virNetDevTapDelete to + * remove a persistent TAP devices when it is no longer needed. * * Returns 0 in case of success or an errno code in case of failure. */ int virNetDevTapCreate(char **ifname, - int vnet_hdr ATTRIBUTE_UNUSED, - int *tapfd) + int *tapfd, + unsigned int flags ATTRIBUTE_UNUSED) { int fd; struct ifreq ifr; @@ -139,7 +142,8 @@ int virNetDevTapCreate(char **ifname, ifr.ifr_flags = IFF_TAP|IFF_NO_PI; # ifdef IFF_VNET_HDR - if (vnet_hdr && virNetDevProbeVnetHdr(fd)) + if ((flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) && + virNetDevProbeVnetHdr(fd)) ifr.ifr_flags |= IFF_VNET_HDR; # endif @@ -228,8 +232,8 @@ cleanup: } #else /* ! TUNSETIFF */ int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED, - int vnet_hdr ATTRIBUTE_UNUSED, - int *tapfd ATTRIBUTE_UNUSED) + int *tapfd ATTRIBUTE_UNUSED, + unsigned int flags ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Unable to create TAP devices on this platform")); @@ -249,17 +253,23 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) * @brname: the bridge name * @ifname: the interface name (or name template) * @macaddr: desired MAC address (VIR_MAC_BUFLEN long) - * @discourage: whether bridge should be discouraged from using macaddr - * @vnet_hdr: whether to try enabling IFF_VNET_HDR * @tapfd: file descriptor return value for the new tap device * @virtPortProfile: bridge/port specific configuration + * @flags: OR of virNetDevTapCreateFlags: + + * VIR_NETDEV_TAP_CREATE_IFUP + * - Bring the interface up + * VIR_NETDEV_TAP_CREATE_VNET_HDR + * - Enable IFF_VNET_HDR on the tap device + * VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE + * - Set this interface's MAC as the bridge's MAC address * * This function creates a new tap device on a bridge. @ifname can be either * a fixed name or a name template with '%d' for dynamic name allocation. * in either case the final name for the bridge will be stored in @ifname. * If the @tapfd parameter is supplied, the open tap device file * descriptor will be returned, otherwise the TAP device will be made - * persistent and closed. The caller must use brDeleteTap to remove + * persistent and closed. The caller must use virNetDevTapDelete to remove * a persistent TAP devices when it is no longer needed. * * Returns 0 in case of success or -1 on failure @@ -267,15 +277,13 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED) int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, - bool discourage, - int vnet_hdr, - bool up, int *tapfd, - virNetDevVPortProfilePtr virtPortProfile) + virNetDevVPortProfilePtr virtPortProfile, + unsigned int flags) { unsigned char tapmac[VIR_MAC_BUFLEN]; - if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0) + if (virNetDevTapCreate(ifname, tapfd, flags) < 0) return -1; /* We need to set the interface MAC before adding it @@ -285,7 +293,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, * device before we set our static MAC. */ memcpy(tapmac, macaddr, VIR_MAC_BUFLEN); - if (discourage) + if (!(flags & VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE)) tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */ if (virNetDevSetMAC(*ifname, tapmac) < 0) @@ -308,7 +316,7 @@ int virNetDevTapCreateInBridgePort(const char *brname, goto error; } - if (virNetDevSetOnline(*ifname, up) < 0) + if (virNetDevSetOnline(*ifname, !!(flags & VIR_NETDEV_TAP_CREATE_IFUP)) < 0) goto error; return 0; diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h index fc50e22..971b166 100644 --- a/src/util/virnetdevtap.h +++ b/src/util/virnetdevtap.h @@ -27,21 +27,29 @@ # include "virnetdevvportprofile.h" int virNetDevTapCreate(char **ifname, - int vnet_hdr, - int *tapfd) + int *tapfd, + unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virNetDevTapDelete(const char *ifname) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; +typedef enum { + VIR_NETDEV_TAP_CREATE_NONE = 0, + /* Bring the interface up */ + VIR_NETDEV_TAP_CREATE_IFUP = 1 << 0, + /* Enable IFF_VNET_HDR on the tap device */ + VIR_NETDEV_TAP_CREATE_VNET_HDR = 1 << 1, + /* Set this interface's MAC as the bridge's MAC address */ + VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE = 1 << 2, +} virNetDevTapCreateFlags; + int virNetDevTapCreateInBridgePort(const char *brname, char **ifname, const unsigned char *macaddr, - bool discourage, - int vnet_hdr, - bool up, int *tapfd, - virNetDevVPortProfilePtr virtPortProfile) + virNetDevVPortProfilePtr virtPortProfile, + unsigned int flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; -- 1.7.7.6

3 3

[libvirt] Downloading and wiping assumes volume is a device or file
by Wido den Hollander 02 Mar '12

02 Mar '12

Hi, I'm still working on the RBD (RADOS / Ceph) storage driver for libvirt and I noticed the virStorageVolDownload and virStorageVolWipe methods. I assumed those would be passed on to the storage backend, but it doesn't. In the storageDriver the method storageVolumeDownload simply opens a file descriptor and reads the device. Until now libvirt only had support for storage drivers who presented regular files or block devices, but RBD doesn't. (Well, RBD could, but I'm currently going for Qemu-RBD). In the future we might see more storage drivers in libvirt for a project like Sheepdog as well. Sheepdog and RBD both have drivers in Qemu. What would the way be to approach this? Should the download, upload and wipe methods be moved to the storage backends? There could also be an exception? If virStoragePoolType matches VIR_STORAGE_POOL_RBD or VIR_STORAGE_POOL_SHEEPDOG the storage backend could be invoked instead of opening the file descriptor? Any thoughts on this? Thanks, Wido

2 3

[libvirt] [PATCH] avoid global variable shadowed
by Hu Tao 02 Mar '12

02 Mar '12

If "conf/domain_conf.h" included in file nodeinfo.c, gcc complains about a variable shadowing. fix it for potential further including of the file. ref: https://www.redhat.com/archives/libvir-list/2012-February/msg00955.html --- src/nodeinfo.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/nodeinfo.c b/src/nodeinfo.c index e0b66f7..0c86a7e 100644 --- a/src/nodeinfo.c +++ b/src/nodeinfo.c @@ -206,7 +206,7 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, DIR *cpudir = NULL; struct dirent *cpudirent = NULL; unsigned int cpu; - unsigned long core, socket, cur_threads; + unsigned long core, sock, cur_threads; cpu_set_t core_mask; cpu_set_t socket_mask; int online; @@ -311,9 +311,9 @@ int linuxNodeInfoCPUPopulate(FILE *cpuinfo, } /* Parse socket */ - socket = parse_socket(cpu); - if (!CPU_ISSET(socket, &socket_mask)) { - CPU_SET(socket, &socket_mask); + sock = parse_socket(cpu); + if (!CPU_ISSET(sock, &socket_mask)) { + CPU_SET(sock, &socket_mask); nodeinfo->sockets++; } -- 1.7.4.4

2 1

[libvirt] [PATCH 0/2] two cosmetic fixes to netlink code
by Laine Stump 02 Mar '12

02 Mar '12

I pushed the following under the trivial rule yesterday. Much to my discredit, I screwed up twice: 1) I ran git send-email to send the patches to the list along with a "pushed under trivial rule" message, but then got distracted in the middle of the OOOOH!!!! Something shiny!! - yes, just like that - and ended up not hitting the final send confirmation, so the list was never notified. 2) My re-ordering to alphabetize the list of functions wasn't complete - my visual scans aren't as thorough as those of Eric, who noticed that virNetlinkEventServiceStop was still placed before virNetlinkEventServiceStart. As penance for this transgression, I forced myself to actually get a full night's sleep last night. Maybe in the future I'll be more alert.

1 2

[libvirt] [PATCH 0/2] Make managed-save path configurable
by Michal Privoznik 02 Mar '12

02 Mar '12

Some users/sysadmins don't really create big partition for / or /var; Therefore number of domains that can be managed-save is limited to only a few because we simply run out of free space. Yes, one can override the path at compile time, but it's sort of overkill. Having said that, I think this is nice to have feature. We can make more paths configurable if this gains enough fame. Michal Privoznik (2): qemu_conf: Move paths initialization to qemudLoadDriverConfig qemu_conf: Allow overriding of path for managed-save src/qemu/qemu.conf | 7 +++ src/qemu/qemu_conf.c | 60 ++++++++++++++++++++++++++++- src/qemu/qemu_conf.h | 4 +- src/qemu/qemu_driver.c | 101 ++++++++++++++---------------------------------- 4 files changed, 98 insertions(+), 74 deletions(-) -- 1.7.3.4

3 4

[libvirt] [PATCH 1/1] lxc: handle shutdown (and detect, but mis-handle reboot)
by Serge Hallyn 02 Mar '12

02 Mar '12

The -mm tree has Daniel Lezcano's patch changing the handling of sys_reboot in a non-init pidns. That means that, with that support, (a) it is safe to grant CAP_SYS_BOOT to a container, and (b) it's possible to distinguish between reboot and shutdown. I've implemented partial support of this for libvirt in the patch below. If Daniel's patch is not in the running kernel, then CAP_SYS_BOOT will be dropped for the container. Otherwise, it will be kept in. When the container exits, if it was determined to be a shutdown, the container will terminate. However, I didn't know how to properly do the reboot part. The patch below shows how to detect it (and sets the static bool wantreboot to true in that case), but I didn't know quite what to do with that. It looks like the code flow between lxcControllerRun and lxcControllerMain would need to be changed a bit so that we could re-run the lxcContainerStart() without causing the monitor.serverFD (or whichever pipe sends monitor events to lxc_driver.c to trigger autodestroy) to be closed. So for now I'm sending this patch, and hoping the sorcerers on this list can hook reboot up as well, or show the best way how. thanks, -serge Subject: [PATCH 1/1] lxc: handle shutdown (and detect, but mis-handle reboot) If Daniel Lezcano's pidns reboot patch is in the kernel, then don't drop CAP_SYS_BOOT. When container calls shutdown, terminate the container. This patch detects when the container wanted to reboot, but goes ahead and terminates the container because I don't know how to best structure the code to support restarting a container that wanted to reboot. Signed-off-by: Serge Hallyn <serge.hallyn(a)canonical.com> --- src/lxc/lxc_container.c | 13 ++++-- src/lxc/lxc_container.h | 3 +- src/lxc/lxc_controller.c | 97 ++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 103 insertions(+), 10 deletions(-) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index e93fda5..793cb19 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -102,6 +102,7 @@ struct __lxc_child_argv { char **ttyPaths; size_t nttyPaths; int handshakefd; + bool dropreboot; }; @@ -1216,7 +1217,7 @@ static int lxcContainerSetupMounts(virDomainDefPtr vmDef, * It removes some capabilities that could be dangerous to * host system, since they are not currently "containerized" */ -static int lxcContainerDropCapabilities(void) +static int lxcContainerDropCapabilities(bool dropreboot) { #if HAVE_CAPNG int ret; @@ -1226,11 +1227,11 @@ static int lxcContainerDropCapabilities(void) if ((ret = capng_updatev(CAPNG_DROP, CAPNG_EFFECTIVE | CAPNG_PERMITTED | CAPNG_INHERITABLE | CAPNG_BOUNDING_SET, - CAP_SYS_BOOT, /* No use of reboot */ CAP_SYS_MODULE, /* No kernel module loading */ CAP_SYS_TIME, /* No changing the clock */ CAP_AUDIT_CONTROL, /* No messing with auditing status */ CAP_MAC_ADMIN, /* No messing with LSM config */ + dropreboot ? CAP_SYS_BOOT : -1, /* No use of reboot? */ -1 /* sentinal */)) < 0) { lxcError(VIR_ERR_INTERNAL_ERROR, _("Failed to remove capabilities: %d"), ret); @@ -1343,7 +1344,7 @@ static int lxcContainerChild( void *data ) } /* drop a set of root capabilities */ - if (lxcContainerDropCapabilities() < 0) + if (lxcContainerDropCapabilities(argv->dropreboot) < 0) goto cleanup; if (lxcContainerSendContinue(argv->handshakefd) < 0) { @@ -1416,6 +1417,7 @@ const char *lxcContainerGetAlt32bitArch(const char *arch) * @veths: interface names * @control: control FD to the container * @ttyPath: path of tty to set as the container console + * @dropreboot: do we need to drop CAP_SYS_BOOT * * Starts a container process by calling clone() with the namespace flags * @@ -1428,7 +1430,8 @@ int lxcContainerStart(virDomainDefPtr def, int control, int handshakefd, char **ttyPaths, - size_t nttyPaths) + size_t nttyPaths, + bool dropreboot) { pid_t pid; int cflags; @@ -1436,7 +1439,7 @@ int lxcContainerStart(virDomainDefPtr def, char *stack, *stacktop; lxc_child_argv_t args = { def, securityDriver, nveths, veths, control, - ttyPaths, nttyPaths, handshakefd}; + ttyPaths, nttyPaths, handshakefd, dropreboot}; /* allocate a stack for the container */ if (VIR_ALLOC_N(stack, stacksize) < 0) { diff --git a/src/lxc/lxc_container.h b/src/lxc/lxc_container.h index 77fb9b2..15738c8 100644 --- a/src/lxc/lxc_container.h +++ b/src/lxc/lxc_container.h @@ -56,7 +56,8 @@ int lxcContainerStart(virDomainDefPtr def, int control, int handshakefd, char **ttyPaths, - size_t nttyPaths); + size_t nttyPaths, + bool dropreboot); int lxcContainerAvailable(int features); diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 8f336f5..e9aa904 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -708,6 +708,7 @@ ignorable_accept_errno(int errnum) } static bool quit = false; +static bool wantreboot = false; static virMutex lock; static int sigpipe[2]; @@ -721,12 +722,33 @@ static void lxcSignalChildIO(int watch ATTRIBUTE_UNUSED, int events ATTRIBUTE_UNUSED, void *opaque) { char buf[1]; - int ret; + int ret, status; int *container = opaque; ignore_value(read(sigpipe[0], buf, 1)); - ret = waitpid(-1, NULL, WNOHANG); + ret = waitpid(-1, &status, WNOHANG); if (ret == *container) { + if (WIFSIGNALED(status)) { + switch(WTERMSIG(status)) { + case SIGINT: /* halt */ + VIR_DEBUG("XXX Container halting"); + virMutexLock(&lock); + quit = true; + virMutexUnlock(&lock); + VIR_DEBUG("XXX set quit to true"); + return; + case SIGHUP: /* reboot */ + VIR_DEBUG("XXX Container rebooting"); + virMutexLock(&lock); + wantreboot = true; + virMutexUnlock(&lock); + VIR_DEBUG("XXX set wantreboot true (i'm pid %d)", getpid()); + return; + default: + VIR_DEBUG("XXX unknown exit status for init: %d\n", WTERMSIG(status)); + break; + } + } virMutexLock(&lock); quit = true; virMutexUnlock(&lock); @@ -1082,6 +1104,62 @@ error: virMutexUnlock(&lock); } +#include <sys/reboot.h> +#include <linux/reboot.h> + +/* + * reboot(LINUX_REBOOT_CMD_CAD_ON) will return -EINVAL + * in a child pid namespace if container reboot support exists. + * Otherwise, it will either succeed or return -EPERM. + */ +static int container_reboot_supported(void *arg) +{ + int *cmd = arg; + int ret; + + ret = reboot(*cmd); + if (ret == -1 && errno == EINVAL) + return 1; + return 0; +} + +static int container_reboot_is_supported(void) +{ + FILE *f = fopen("/proc/sys/kernel/ctrl-alt-del", "r"); + int ret, cmd, v; + long stack_size = 4096; + void *stack = alloca(stack_size) + stack_size; + int status; + pid_t pid; + + if (!f) { + VIR_DEBUG("failed to open /proc/sys/kernel/ctrl-alt-del"); + return 0; + } + + ret = fscanf(f, "%d", &v); + fclose(f); + if (ret != 1) { + VIR_DEBUG("Failed to read /proc/sys/kernel/ctrl-alt-del"); + return 0; + } + cmd = v ? LINUX_REBOOT_CMD_CAD_ON : LINUX_REBOOT_CMD_CAD_OFF; + + pid = clone(container_reboot_supported, stack, CLONE_NEWPID | SIGCHLD, &cmd); + if (pid < 0) { + VIR_DEBUG("failed to clone\n"); + return 0; + } + if (wait(&status) < 0) { + VIR_DEBUG("unexpected wait error: %m\n"); + return 0; + } + + if (WEXITSTATUS(status) != 1) + return 0; + + return 1; +} /** * lxcControllerMain @@ -1214,13 +1292,19 @@ static int lxcControllerMain(int serverFd, } virMutexLock(&lock); - while (!quit) { + while (!quit && !wantreboot) { virMutexUnlock(&lock); if (virEventRunDefaultImpl() < 0) goto cleanup; virMutexLock(&lock); } virMutexUnlock(&lock); + VIR_DEBUG("XXX (pid %d) container is done", getpid()); + if (wantreboot) { + rc = 0; + VIR_DEBUG("XXX (pid %d) wantreboot is true", getpid()); + //goto cleanup2; + } err = virGetLastError(); if (!err || err->code == VIR_ERR_OK) @@ -1385,6 +1469,7 @@ lxcControllerRun(virDomainDefPtr def, size_t nloopDevs = 0; int *loopDevs = NULL; size_t i; + bool dropreboot = true; if (VIR_ALLOC_N(containerTtyFDs, nttyFDs) < 0) { virReportOOMError(); @@ -1542,6 +1627,7 @@ lxcControllerRun(virDomainDefPtr def, if (lxcSetPersonality(def) < 0) goto cleanup; + dropreboot = !container_reboot_is_supported(); if ((container = lxcContainerStart(def, securityDriver, @@ -1550,7 +1636,8 @@ lxcControllerRun(virDomainDefPtr def, control[1], containerhandshake[1], containerTtyPaths, - nttyFDs)) < 0) + nttyFDs, + dropreboot)) < 0) goto cleanup; VIR_FORCE_CLOSE(control[1]); VIR_FORCE_CLOSE(containerhandshake[1]); @@ -1603,7 +1690,9 @@ lxcControllerRun(virDomainDefPtr def, } } + VIR_DEBUG("XXX starting lxcControllerMain (i'm pid %d)\n", getpid()); rc = lxcControllerMain(monitor, client, ttyFDs, containerTtyFDs, nttyFDs, container); + VIR_DEBUG("XXX lxcControllerMain returned %d (i'm pid %d)\n", rc, getpid()); monitor = client = -1; cleanup: -- 1.7.9

2 2

[libvirt] [libvirt-glib] a leak in libvirt-glib
by Marc-André Lureau 02 Mar '12

02 Mar '12

Hi, In libvirt-glib, we call virStreamEventAddCallback() and give it a ref, that is supposed to be unref when the stream event is removed. But this doesn't happen! I tracked it down to: static void remoteStreamCallbackFree(void *opaque) { struct remoteStreamCallbackData *cbdata = opaque; if (!cbdata->cb && cbdata->ff) (cbdata->ff)(cbdata->opaque); Why are we checking for cbdata->cb here? That gives us a leak when taking screenshots. So far I solved it with the attached patch for libvirt-glib. I noticed it because that of a resulting process & fd leakage in the libvirtd side. It might be that the fix should be in libvirt, but anyway, the proposed patch doesn't need a libvirt depedency update and also keeps the object reference manangement at the libvirt-glib level, which is nice. -- Marc-André Lureau

2 1

[libvirt] [libvirt-glib 1/6] Set correct target node attribute for domain interface
by Zeeshan Ali (Khattak) 01 Mar '12

01 Mar '12

From: "Zeeshan Ali (Khattak)" <zeeshanak(a)gnome.org> gvir_config_domain_interface_set_ifname() should be setting 'dev' attribute under 'target', not 'device'. --- libvirt-gconfig/libvirt-gconfig-domain-interface.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gconfig/libvirt-gconfig-domain-interface.c b/libvirt-gconfig/libvirt-gconfig-domain-interface.c index 85cc194..eab4313 100644 --- a/libvirt-gconfig/libvirt-gconfig-domain-interface.c +++ b/libvirt-gconfig/libvirt-gconfig-domain-interface.c @@ -57,7 +57,7 @@ void gvir_config_domain_interface_set_ifname(GVirConfigDomainInterface *interfac g_return_if_fail(GVIR_CONFIG_IS_DOMAIN_INTERFACE(interface)); gvir_config_object_replace_child_with_attribute(GVIR_CONFIG_OBJECT(interface), - "target", "device", ifname); + "target", "dev", ifname); } void gvir_config_domain_interface_set_link_state(GVirConfigDomainInterface *interface, -- 1.7.7.6

2 12