[libvirt] [PATCH 3/3] fix other functions to add VIR_NETWORK_FORWARD_VLAN
Signed-off-by: Shi Lei
---
src/conf/domain_conf.c | 1 +
src/conf/virnetworkobj.c | 1 +
src/qemu/qemu_process.c | 1 +
3 files changed, 3 insertions(+)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f4e59f6..bd8b050 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -29912,6 +29912,7 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface)
if ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
(def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
(def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
+(def->forward.type == VIR_NETWORK_FORWARD_VLAN) ||
(def->forward.type == VIR_NETWORK_FORWARD_OPEN)) {
/* for these forward types, the actual net type really *is*
* NETWORK; we just keep the info from the portgroup in
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index e00c8a7..3869021 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -1012,6 +1012,7 @@ virNetworkLoadConfig(virNetworkObjListPtr nets,
if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
def->forward.type == VIR_NETWORK_FORWARD_NAT ||
def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
+def->forward.type == VIR_NETWORK_FORWARD_VLAN ||
def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
if (!def->mac_specified) {
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 40d35cb..0e3e1af 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4434,6 +4434,7 @@ qemuProcessGetNetworkAddress(const char *netname,
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
case VIR_NETWORK_FORWARD_OPEN:
+case VIR_NETWORK_FORWARD_VLAN:
ipdef = virNetworkDefGetIPByIndex(netdef, AF_UNSPEC, 0);
if (!ipdef) {
virReportError(VIR_ERR_INTERNAL_ERROR,
--
2.7.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] support VLan for virtual network based on 8021q kernel module
Support VLan for virtual network based on 8021q kernel module other than ovs backend. Insert vlan-device into the datapath of the network traffic: (outside of host) <--> physical_interface <--> vlan-dev(with VLAN-Tag) <--> bridge <--> guests Then VLAN-Tag will be applied to the guest's network traffic. The VLan network acts as real layer-2 switch which provides 'access' port to guest. 1. Create VLan network by setting mode='vlan' on the forward element, for example: ... vlan10 ... The mode attribute of should be 'vlan' (this patch added). The dev attribute of specifies a physical interface which forwards traffice between this VLan-network and outside. The id attribue of the vlan tag indicates VLAN-Tag. Both vlan element and tag element should be unique in this xml. A guest connects to this VLan network by setting its xml like this: 2. We can enable dhcp for VLan network according to the current way, for example: ... ... ... 3. This can help to build Cross-Host VLan network for guests. We can simplify the work to implement vlan-net of management app (e.g. OpenStack). 1) Distribute the xml of VLan network to all hosts, then 'virsh net-create ...' on each host locally. 2) Makesure the outside switch's port linked to the physical interface (specified by the dev of ) is 'trunk' mode. 3) For each VLan network, ONLY one host can 'net-create' network with ip and dhcp element to avoid dhcp conflict. Shi Lei (3): add functions: load(verify) 8021q module, create/destroy vlan-dev support new forward mode of vlan for virtual network fix other functions to add VIR_NETWORK_FORWARD_VLAN configure.ac| 5 +- src/conf/domain_conf.c | 1 + src/conf/network_conf.c | 12 ++- src/conf/network_conf.h | 1 + src/conf/virnetworkobj.c| 1 + src/libvirt_private.syms| 4 + src/network/bridge_driver.c | 80 -- src/qemu/qemu_process.c | 1 + src/util/virnetdev.c| 195 src/util/virnetdev.h| 14 10 files changed, 301 insertions(+), 13 deletions(-) -- 2.7.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] add functions: load 8021q module, create/destroy vlan-dev
Signed-off-by: Shi Lei
---
configure.ac | 5 +-
src/libvirt_private.syms | 4 +
src/util/virnetdev.c | 195 +++
src/util/virnetdev.h | 14
4 files changed, 216 insertions(+), 2 deletions(-)
diff --git a/configure.ac b/configure.ac
index 59d2d09..141d5b2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -769,8 +769,9 @@ then
fi
AM_CONDITIONAL([WITH_NODE_DEVICES], [test "$with_nodedev" = "yes"])
-dnl GET_VLAN_VID_CMD is required for virNetDevGetVLanID
-AC_CHECK_DECLS([GET_VLAN_VID_CMD], [], [], [[#include ]])
+dnl GET_VLAN_VID_CMD, ADD_VLAN_CMD, DEL_VLAN_CMD is required
+AC_CHECK_DECLS([GET_VLAN_VID_CMD, ADD_VLAN_CMD, DEL_VLAN_CMD],
+ [], [], [[#include ]])
# Check for Linux vs. BSD ifreq members
AC_CHECK_MEMBERS([struct ifreq.ifr_newname,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3e30490..a61ba02 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2266,7 +2266,9 @@ virModuleLoad;
# util/virnetdev.h
virNetDevAddMulti;
+virNetDevCreateVLanDev;
virNetDevDelMulti;
+virNetDevDestroyVLanDev;
virNetDevExists;
virNetDevFeatureTypeFromString;
virNetDevFeatureTypeToString;
@@ -2287,10 +2289,12 @@ virNetDevGetRxFilter;
virNetDevGetVirtualFunctionIndex;
virNetDevGetVirtualFunctionInfo;
virNetDevGetVirtualFunctions;
+virNetDevGetVLanDevName;
virNetDevGetVLanID;
virNetDevIfStateTypeFromString;
virNetDevIfStateTypeToString;
virNetDevIsVirtualFunction;
+virNetDevLoad8021Q;
virNetDevPFGetVF;
virNetDevReadNetConfig;
virNetDevRunEthernetScript;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index c20022f..50a81d2 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -44,6 +44,7 @@
# include
# include
# define VIR_NETDEV_FAMILY AF_UNIX
+# include "virkmod.h"
#elif defined(HAVE_STRUCT_IFREQ) && defined(AF_LOCAL)
# define VIR_NETDEV_FAMILY AF_LOCAL
#else
@@ -1048,6 +1049,200 @@ int virNetDevGetVLanID(const char *ifname
ATTRIBUTE_UNUSED,
#endif /* ! SIOCGIFVLAN */
+#if defined(HAVE_STRUCT_IFREQ)
+
+# define MODULE_8021Q "8021q"
+# define PROC_NET_VLAN_CONFIG "/proc/net/vlan/config"
+
+static int
+validate8021Q(void)
+{
+int fd;
+if ((fd = open(PROC_NET_VLAN_CONFIG, O_RDONLY)) < 0)
+return -1;
+VIR_FORCE_CLOSE(fd);
+return 0;
+}
+
+static int
+getVlanDevName(const char *ifname, unsigned int vlanid,
+ char vdname[], size_t vdname_size)
+{
+if (!vdname || vdname_size < IFNAMSIZ)
+return -1;
+if (snprintf(vdname, IFNAMSIZ, "%s.%d", ifname, vlanid) >= IFNAMSIZ)
+return -1;
+return 0;
+}
+
+static int
+controlVlanDev(unsigned int cmd,
+ const char *ifname, unsigned int vlanid)
+{
+int fd;
+struct vlan_ioctl_args if_request;
+memset(_request, 0, sizeof(struct vlan_ioctl_args));
+if_request.cmd = cmd;
+
+if (cmd == ADD_VLAN_CMD) {
+strcpy(if_request.device1, ifname);
+if_request.u.VID = vlanid;
+} else if (cmd == DEL_VLAN_CMD) {
+char vdname[IFNAMSIZ];
+if (getVlanDevName(ifname, vlanid, vdname, sizeof(vdname)) < 0)
+return -1;
+strcpy(if_request.device1, vdname);
+} else {
+virReportSystemError(ENOSYS,
+ _("unsupport cmd: %d"), cmd);
+return -1;
+}
+
+if ((fd = socket(AF_INET, SOCK_STREAM, 0)) < 0) {
+virReportSystemError(errno, "%s",
+ _("unable to open control socket"));
+return -1;
+}
+
+if (ioctl(fd, SIOCSIFVLAN, _request) < 0) {
+virReportSystemError(errno, "%s",
+ _("control VLAN device error"));
+VIR_FORCE_CLOSE(fd);
+return -1;
+}
+
+VIR_FORCE_CLOSE(fd);
+return 0;
+}
+
+/**
+ * virNetDevLoad8021Q:
+ *
+ * Load 8021q module (since kernel v2.6)
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virNetDevLoad8021Q(void)
+{
+if (validate8021Q() < 0) {
+char *errbuf = NULL;
+if ((errbuf = virKModLoad(MODULE_8021Q, false))) {
+VIR_FREE(errbuf);
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Failed to load 8021q module"));
+return -1;
+}
+if (validate8021Q() < 0) {
+virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("cannot load 8021q module"));
+return -1;
+}
+}
+return 0;
+}
+
+/**
+ * virNetDevCreateVLanDev:
+ * @ifname: name of interface we will create vlan-device on
+ * @vlanid: VLAN ID
+ * @vdname: used to return the name of vlan-device
+ * (it should be pre-defined on stack and its type is char[])
+ * @vdname_size: size of the vlan-device's name (do not less than IFNAMSIZ)
+ *
+ * Create vlan-device which based on 8021q module.
+ *
+ * Returns 0 on success, -1 on failure
+ */
+int
+virNetDevCreateVLanDev(const char *ifname, unsigned int
[libvirt] [PATCH 2/3] support new forward mode of vlan for virtual network
Signed-off-by: Shi Lei
---
src/conf/network_conf.c | 12 ---
src/conf/network_conf.h | 1 +
src/network/bridge_driver.c | 80 +
3 files changed, 82 insertions(+), 11 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 630a87f..6e1de6c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -50,7 +50,7 @@ VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
"none", "nat", "route", "open",
"bridge", "private", "vepa", "passthrough",
- "hostdev")
+ "hostdev", "vlan")
VIR_ENUM_IMPL(virNetworkBridgeMACTableManager,
VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LAST,
@@ -1876,6 +1876,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
*/
switch (def->forward.type) {
case VIR_NETWORK_FORWARD_NONE:
+case VIR_NETWORK_FORWARD_VLAN:
break;
case VIR_NETWORK_FORWARD_ROUTE:
@@ -1963,9 +1964,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
(def->forward.type != VIR_NETWORK_FORWARD_NONE &&
def->forward.type != VIR_NETWORK_FORWARD_NAT &&
def->forward.type != VIR_NETWORK_FORWARD_ROUTE &&
- def->forward.type != VIR_NETWORK_FORWARD_OPEN)) {
+ def->forward.type != VIR_NETWORK_FORWARD_OPEN &&
+ def->forward.type != VIR_NETWORK_FORWARD_VLAN)) {
virReportError(VIR_ERR_XML_ERROR,
- _("mtu size only allowed in open, route, nat, "
+ _("mtu size only allowed in open, route, nat, vlan "
"and isolated mode, not in %s (network '%s')"),
virNetworkForwardTypeToString(def->forward.type),
def->name);
@@ -2474,6 +2476,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
def->forward.type == VIR_NETWORK_FORWARD_NAT ||
def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
+def->forward.type == VIR_NETWORK_FORWARD_VLAN ||
def->bridge || def->macTableManager) {
virBufferAddLit(buf, "forward.type == VIR_NETWORK_FORWARD_NONE ||
def->forward.type == VIR_NETWORK_FORWARD_NAT ||
def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
-def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
+def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
+def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
virBufferAsprintf(buf, " stp='%s' delay='%ld'",
def->stp ? "on" : "off", def->delay);
}
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 54c8ed1..47bb83e 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -53,6 +53,7 @@ typedef enum {
VIR_NETWORK_FORWARD_VEPA,
VIR_NETWORK_FORWARD_PASSTHROUGH,
VIR_NETWORK_FORWARD_HOSTDEV,
+VIR_NETWORK_FORWARD_VLAN,
VIR_NETWORK_FORWARD_LAST,
} virNetworkForwardType;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index da3c32e..314f78c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -451,6 +451,7 @@ networkUpdateState(virNetworkObjPtr obj,
case VIR_NETWORK_FORWARD_NAT:
case VIR_NETWORK_FORWARD_ROUTE:
case VIR_NETWORK_FORWARD_OPEN:
+case VIR_NETWORK_FORWARD_VLAN:
/* If bridge doesn't exist, then mark it inactive */
if (!(def->bridge && virNetDevExists(def->bridge) == 1))
virNetworkObjSetActive(obj, false);
@@ -2092,7 +2093,8 @@ networkRefreshDaemonsHelper(virNetworkObjPtr obj,
((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
(def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
(def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
- (def->forward.type == VIR_NETWORK_FORWARD_OPEN))) {
+ (def->forward.type == VIR_NETWORK_FORWARD_OPEN) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_VLAN))) {
/* Only the three L3 network types that are configured by
* libvirt will have a dnsmasq or radvd daemon associated
* with them. Here we send a SIGHUP to an existing
@@ -2131,7 +2133,8 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
if (virNetworkObjIsActive(obj) &&
((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
(def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
- (def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) {
+ (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
+ (def->forward.type == VIR_NETWORK_FORWARD_VLAN))) {
/* Only three of the L3 network types that are configured by
* libvirt need to have iptables rules reloaded. The 4th L3
* network type, forward='open', doesn't need this because it
@@ -2513,6 +2516,27 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
driver,
if (virNetDevBandwidthSet(def->bridge,
[libvirt] Ping Re: [PATCH] qemu: stop qemu progress when restore failed
On 2018/7/5 10:05, Jie Wang wrote:
>>From 29482622218f525f0133be0b7db74835174035d9 Mon Sep 17 00:00:00 2001
> From: Jie Wang
> Date: Thu, 5 Jul 2018 09:52:03 +0800
> Subject: [PATCH] qemu: stop qemu progress when restore failed
>
> if qemuProcessStartCPUs perform failed in qemuDomainSaveImageStartVM,
> we need to stop qemu progress, otherwise will remains a wild VM
> which can't be managed by libvirt.
>
> Signed-off-by: Jie Wang
> ---
> src/qemu/qemu_driver.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9a35e04a85..639b57316d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6621,6 +6621,8 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
> if (virGetLastErrorCode() == VIR_ERR_OK)
> virReportError(VIR_ERR_OPERATION_FAILED,
> "%s", _("failed to resume domain"));
> +
> +qemuProcessStop(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED, asyncJob,
> 0);
> goto cleanup;
> }
> if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm,
> driver->caps) < 0) {
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFCv2 PATCH 3/5] conf: rename cachetune to restune
On 07/03/2018 12:10 AM, bing.niu wrote: > Hi John, > Thanks for reviewing! Since major questions are from this thread, so I > think we can start from this. > > On 2018年06月30日 06:47, John Ferlan wrote: >> >> >> On 06/15/2018 05:29 AM, bing@intel.com wrote: >>> From: Bing Niu >>> >>> resctrl not only supports cache tuning, but also memory bandwidth >>> tuning. Rename cachetune to restune(resource tuning) to reflect >>> that. >>> >>> Signed-off-by: Bing Niu >>> --- >>> src/conf/domain_conf.c | 44 >>> ++-- >>> src/conf/domain_conf.h | 10 +- >>> src/qemu/qemu_process.c | 18 +- >>> 3 files changed, 36 insertions(+), 36 deletions(-) >>> >> >> Not so sure I'm liking [R|r]estune[s]... Still @cachetunes is an array >> of a structure that contains a vCPU bitmap and a virResctrlAllocPtr for >> the /cputune/cachetunes data. The virResctrlAllocPtr was changed to add >> a the bandwidth allocation, so does this really have to change? >> > The cachetunes from Cache Allocation Technology(CAT) and memorytunes > from Memory Bandwidth Allocation(MBA) are both features from Resource > Directory Technology. RDT is a collection of resource distribution > technology, right now it includes CAT and MBA. Details information can > be found 17.18.1 of Intel's sdm. > https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf > tl;dr :-) So some day soon it'll be more complicated and have even more rules?!?! > > The RDT capability and configuration methods is exposed to user land in > form of resctrl file system by kernel. The basic programming model for > this resctrl fs interface like this: > > 1. create a folder under /sys/fs/resctrl. And this folder is called one > resctrl group. > > 2. writing user desired CAT and MBA config to the file > /sys/fs/resctrl//schemata to allocate the cache or > memory bandwidth. you can set CAT and MBA together or any of them. > > 3. moving the threads you want to that resctrl group by writing threads' > PID to /sys/fs/resctrl//task file. So that those > threads can be assign with the resource allocated in step 2. And those > resources are shared by those threads. > > *NOTE*: A thread only can be put in one resctrl group. This requirement > is from how RDT and resctrl works. IOW: A vCPU or the vCPUs being used by CAT and MBA would need to be the same because in "this world" the group *is* the single vCPU or the range of vCPU's. > Each resctrl group has a closid. The resource configuration in above > step2 is set to that closid's msr(IA32_L?_QoS) to describe the resource > allocation for this closid. > And thread use this closid to claim the allocated resource during > context switch(scheduled_in()) in kernel by writing the closid to the > msr IA32_PQR_ASSOC. > With closid wrote in IA32_PQR_ASSOC, RDT hardware knows current running > closid and allocated resource basing on this closid's IA32_L?_QoS msr. > That why a thread can be put into one restrcl group only. > > Details description of resctrl can be found: > https://github.com/torvalds/linux/blob/v4.17/Documentation/x86/intel_rdt_ui.txt > > > :) More lengthy things that I didn't read... Thanks for the pointers. I think keeping track of all the rules and relationships is going to be a real "challenge". > > Basing on above information, my understanding is that virResctrlAllocPtr > is not only bind to cachetunes. It should be a backing object to > describe a resctrl group. Especially current virresctrl class already > works as that. > Libvirt will create one resctrl group for each virResctrlAllocPtr by > virResctrlAllocCreate() interface. > > So from components view, the big picture is something like below. Yay, pictures! Thanks for taking the time. > > > > +-+ > | > XML | + > parser +---+ > | > | > +--+ > | > | > internal resctrl +--v--+ > backing object | virResctrlAllocPtr | > +--+--+ > | > | > +--+ > | > +--v---+ > | | > | schemata | > /sys/fs/resctrl | tasks | > | . | > | . | > | | > +--+ > > Does this make sense? :) > Yes and I think further has me believe that the vCPUs in this case are the groups and perhaps becomes the internal "key" for all this. I didn't go look, but have to
[libvirt] [dbus PATCH] gitignore: add tests/.pytest_cache
Signed-off-by: Anya Harter --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index ca9e4e1..c41c82c 100644 --- a/.gitignore +++ b/.gitignore @@ -44,3 +44,4 @@ vgcore.* /src/org.libvirt.service /tests/test_util +/tests/.pytest_cache -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [dbus PATCH] connect: fix gchar declaration in connect.h
so that gchar *nodeDevPath; line appears in alphabetical order with the
rest of the lines
Signed-off-by: Anya Harter
---
src/connect.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/connect.h b/src/connect.h
index b81b6a8..0b9ae10 100644
--- a/src/connect.h
+++ b/src/connect.h
@@ -12,9 +12,9 @@ struct virtDBusConnect {
GDBusConnection *bus;
const gchar *uri;
const gchar *connectPath;
-gchar *nodeDevPath;
gchar *domainPath;
gchar *networkPath;
+gchar *nodeDevPath;
gchar *nwfilterPath;
gchar *secretPath;
gchar *storagePoolPath;
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] opening tap devices that are created in a container
On Thu, Jul 5, 2018 at 4:20 PM Jason Baron wrote: > Hi, > > Opening tap devices, such as macvtap, that are created in containers is > problematic because the interface for opening tap devices is via > /dev/tapNN and devtmpfs is not typically mounted inside a container as > its not namespace aware. It is possible to do a mknod() in the > container, once the tap devices are created, however, since the tap > devices are created dynamically its not possible to apriori allow access > to certain major/minor numbers, since we don't know what these are going > to be. In addition, its desirable to not allow the mknod capability in > containers. This behavior, I think is somewhat inconsistent with the > tuntap driver where one can create tuntap devices inside a container by > first opening /dev/net/tun and then using them by supplying the tuntap > device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the > network namespace, one is limited to opening network devices that belong > to your current network namespace. > > Here are some options to this issue, that I wanted to get feedback > about, and just wondering if anybody else has run into this. > > 1) > > Don't create the tap device, such as macvtap in the container. Instead, > create the tap device outside of the container and then move it into the > desired container network namespace. In addition, do a mknod() for the > corresponding /dev/tapNN device from outside the container before doing > chroot(). > > This solution still doesn't allow tap devices to be created inside the > container. Thus, in the case of kubevirt, which runs libvirtd inside of > a container, it would mean changing libvirtd to open existing tap > devices (as opposed to the current behavior of creating new ones). This > would not require any kernel changes, but as mentioned seems > inconsistent with the tuntap interface. > For KubeVirt, apart from how exactly the device ends up in the container, I would want to pursue a way where all network preparations which require privileges happens from a privileged process *outside* of the container. Like CNI solutions do it. They run outside, have privileges and then create devices in the right network/mount namespace or move them there. The final goal for KubeVirt is that our pod with the qemu process is completely unprivileged and privileged setup happens from outside. As a consequence, and depending on which route Dan pursues with the restructured libvirt, I would assume that either a privileged libvirtd-part outside of containers creates the devices by entering the right namespaces, or that libvirt in the container can consume pre-created tun/tap devices, like qemu. Best Regards, Roman > > 2) > > Add a new kernel interface for tap devices similar to how /dev/net/tun > currently works. It might be nice to use TUNSETIFF for tap devices, but > because tap devices have different fops they can't be easily switched > after open(). So the suggestion is a new ioctl (TUNGETFDBYNAME?), where > the tap device name is supplied and a new fd (distinct from the fd > returned by the open of /dev/net/tun) is returned as an output field as > part of the new ioctl parameter. > > It may not make sense to have this new ioctl call for /dev/net/tun since > its really about opening a tap device, so it may make sense to introduce > it as part of a new device, such as /dev/net/tap. This new ioctl could > be used for macvtap and ipvtap (or any tap device). I think it might > also improve performance for tuntap devices themselves, if they are > opened this way since currently all tun operations such as read() and > write() take a reference count on the underlying tuntap device, since it > can be changed via TUNSETIFF. I tested this interface out, so I can > provide the kernel changes if that's helpful for clarification. > > Thanks, > > -Jason > -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] opening tap devices that are created in a container
On Thu, Jul 05, 2018 at 10:20:16AM -0400, Jason Baron wrote: > Hi, > > Opening tap devices, such as macvtap, that are created in containers is > problematic because the interface for opening tap devices is via > /dev/tapNN and devtmpfs is not typically mounted inside a container as > its not namespace aware. It is possible to do a mknod() in the > container, once the tap devices are created, however, since the tap > devices are created dynamically its not possible to apriori allow access > to certain major/minor numbers, since we don't know what these are going > to be. In addition, its desirable to not allow the mknod capability in > containers. This behavior, I think is somewhat inconsistent with the > tuntap driver where one can create tuntap devices inside a container by > first opening /dev/net/tun and then using them by supplying the tuntap > device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the > network namespace, one is limited to opening network devices that belong > to your current network namespace. > > Here are some options to this issue, that I wanted to get feedback > about, and just wondering if anybody else has run into this. > > 1) > > Don't create the tap device, such as macvtap in the container. Instead, > create the tap device outside of the container and then move it into the > desired container network namespace. In addition, do a mknod() for the > corresponding /dev/tapNN device from outside the container before doing > chroot(). > > This solution still doesn't allow tap devices to be created inside the > container. Thus, in the case of kubevirt, which runs libvirtd inside of > a container, it would mean changing libvirtd to open existing tap > devices (as opposed to the current behavior of creating new ones). This > would not require any kernel changes, but as mentioned seems > inconsistent with the tuntap interface. Presumably the /dev/tapNN device name also changes when you rename the tap device interface using SIOCSIFNAME ? eg if it was /dev/tap24 in the host and you called SIOCSIFNAME(eth0) when moving it into the container, it would be /dev/eth0 inside the container ? Anyway, given that this /dev/tapNN approach is what exists today, libvirt will likely want to implement support for this regardless in order to support existing kernels. > 2) > > Add a new kernel interface for tap devices similar to how /dev/net/tun > currently works. It might be nice to use TUNSETIFF for tap devices, but > because tap devices have different fops they can't be easily switched > after open(). So the suggestion is a new ioctl (TUNGETFDBYNAME?), where > the tap device name is supplied and a new fd (distinct from the fd > returned by the open of /dev/net/tun) is returned as an output field as > part of the new ioctl parameter. > > It may not make sense to have this new ioctl call for /dev/net/tun since > its really about opening a tap device, so it may make sense to introduce > it as part of a new device, such as /dev/net/tap. This new ioctl could > be used for macvtap and ipvtap (or any tap device). I think it might > also improve performance for tuntap devices themselves, if they are > opened this way since currently all tun operations such as read() and > write() take a reference count on the underlying tuntap device, since it > can be changed via TUNSETIFF. I tested this interface out, so I can > provide the kernel changes if that's helpful for clarification. Either /dev/net/tun wit new ioctl, or /dev/net/tap with TNUSETIFF would be workable from libvirt's POV. One slight complication with either of the solutions above is that libvirt won't know whether it is given a TAP or a MACVTAP device. It'll only be given the device name. So with code today we would probably have to first try /dev/tapNNN and if that doesn't exist then try /dev/net/tun with TUNSETIFF. If adding a new /dev/net/tap, something could seemlessy accept either a TAP or MACTAP nic name would be nice. Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags
On Thu, Jul 05, 2018 at 02:43:18PM +0200, Michal Prívozník wrote:
> On 07/05/2018 01:36 PM, Daniel P. Berrangé wrote:
> > There are two boolean parameters passed to qemuBuildChrChardevStr,
> > and soon there will be a third. It will be clearer to understand
> > from callers' POV if we use named flags instead.
> >
> > Signed-off-by: Daniel P. Berrangé
> > ---
> > src/qemu/qemu_command.c | 86 ++---
> > 1 file changed, 55 insertions(+), 31 deletions(-)
> >
> > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> > index 9351b9fddb..7f3eccc1bd 100644
> > --- a/src/qemu/qemu_command.c
> > +++ b/src/qemu/qemu_command.c
> > @@ -4935,6 +4935,12 @@ qemuOpenChrChardevUNIXSocket(const
> > virDomainChrSourceDef *dev)
> > return -1;
> > }
> >
> > +
> > +enum {
> > +QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
> > +QEMU_BUILD_CHARDEV_FILE_LOGD = (1 << 1),
> > +};
> > +
> > /* This function outputs a -chardev command line option which describes
> > only the
> > * host side of the character device */
> > static char *
> > @@ -4946,8 +4952,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> > const virDomainChrSourceDef *dev,
> > const char *alias,
> > virQEMUCapsPtr qemuCaps,
> > - bool nowait,
> > - bool chardevStdioLogd)
> > + unsigned int flags)
> > {
> > virBuffer buf = VIR_BUFFER_INITIALIZER;
> > bool telnet;
> > @@ -4986,7 +4991,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> > _("append not supported in this QEMU binary"));
> > goto cleanup;
> > }
> > -if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager :
> > NULL,
> > +if (qemuBuildChrChardevFileStr(flags &
> > QEMU_BUILD_CHARDEV_FILE_LOGD ?
> > + logManager : NULL,
> > cmd, def, ,
> > "path", dev->data.file.path,
> > "append", dev->data.file.append) <
> > 0)
> > @@ -5033,7 +5039,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> >telnet ? ",telnet" : "");
> >
> > if (dev->data.tcp.listen)
> > -virBufferAdd(, nowait ? ",server,nowait" : ",server", -1);
> > +virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
> > + ",server,nowait" : ",server", -1);
>
> Well, since you're touching this: s/virBufferAdd/virBufferAddLit/ and
> also perhaps get rid of the ternary operator?
>
> diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
> index 7f3eccc1bd..63c7ac0f82 100644
> --- i/src/qemu/qemu_command.c
> +++ w/src/qemu/qemu_command.c
> @@ -5038,9 +5038,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>dev->data.tcp.service,
>telnet ? ",telnet" : "");
>
> -if (dev->data.tcp.listen)
> -virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
> - ",server,nowait" : ",server", -1);
> +if (dev->data.tcp.listen) {
> +virBufferAddLit(, ",server");
> +if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
> +virBufferAddLit(, ",nowait");
> +}
>
> qemuBuildChrChardevReconnectStr(, >data.tcp.reconnect);
>
> @@ -5098,9 +5100,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> virBufferAsprintf(, "socket,id=%s,path=", charAlias);
> virQEMUBuildBufferEscapeComma(, dev->data.nix.path);
> }
> -if (dev->data.nix.listen)
> -virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
> - ",server,nowait" : ",server", -1);
> +if (dev->data.nix.listen) {
> +virBufferAddLit(, ",server");
> +if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
> +virBufferAddLit(, ",nowait");
> +}
>
> qemuBuildChrChardevReconnectStr(, >data.nix.reconnect);
> break;
Sure, this is fine.
Regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] opening tap devices that are created in a container
Hi, Opening tap devices, such as macvtap, that are created in containers is problematic because the interface for opening tap devices is via /dev/tapNN and devtmpfs is not typically mounted inside a container as its not namespace aware. It is possible to do a mknod() in the container, once the tap devices are created, however, since the tap devices are created dynamically its not possible to apriori allow access to certain major/minor numbers, since we don't know what these are going to be. In addition, its desirable to not allow the mknod capability in containers. This behavior, I think is somewhat inconsistent with the tuntap driver where one can create tuntap devices inside a container by first opening /dev/net/tun and then using them by supplying the tuntap device name via the ioctl(TUNSETIFF). And since TUNSETIFF validates the network namespace, one is limited to opening network devices that belong to your current network namespace. Here are some options to this issue, that I wanted to get feedback about, and just wondering if anybody else has run into this. 1) Don't create the tap device, such as macvtap in the container. Instead, create the tap device outside of the container and then move it into the desired container network namespace. In addition, do a mknod() for the corresponding /dev/tapNN device from outside the container before doing chroot(). This solution still doesn't allow tap devices to be created inside the container. Thus, in the case of kubevirt, which runs libvirtd inside of a container, it would mean changing libvirtd to open existing tap devices (as opposed to the current behavior of creating new ones). This would not require any kernel changes, but as mentioned seems inconsistent with the tuntap interface. 2) Add a new kernel interface for tap devices similar to how /dev/net/tun currently works. It might be nice to use TUNSETIFF for tap devices, but because tap devices have different fops they can't be easily switched after open(). So the suggestion is a new ioctl (TUNGETFDBYNAME?), where the tap device name is supplied and a new fd (distinct from the fd returned by the open of /dev/net/tun) is returned as an output field as part of the new ioctl parameter. It may not make sense to have this new ioctl call for /dev/net/tun since its really about opening a tap device, so it may make sense to introduce it as part of a new device, such as /dev/net/tap. This new ioctl could be used for macvtap and ipvtap (or any tap device). I think it might also improve performance for tuntap devices themselves, if they are opened this way since currently all tun operations such as read() and write() take a reference count on the underlying tuntap device, since it can be changed via TUNSETIFF. I tested this interface out, so I can provide the kernel changes if that's helpful for clarification. Thanks, -Jason -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] Loop through all resolved addresses in virNetSocketNewListenTCP
On Wed, Jul 04, Daniel P. Berrangé wrote: > On Wed, Jul 04, 2018 at 01:48:56PM +0200, Olaf Hering wrote: > > Since bind() already succeeded (or will succeed) for one address, there is > > no reason to error out. Anyone who connects to an resolved ipv6 address will > > get an error, and moves on to try the resolved ipv4 address. > > libvirt is overdoing things here. > Many of the errnos reported by bind() indicate problems we should not be > ignoring IMHO. So I'd rather handle the actual errnos that indicate > non-fatal problems. So you prefer to keep the existing behavior that all errors from bind() are fatal, even if nsocks!=0? What is the reason for that behavior? So far noone was willing to answer that question. Olaf signature.asc Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/3] Fix regression with vhostuser and chardev FD passing
On 07/05/2018 01:36 PM, Daniel P. Berrangé wrote: > QEMU has a validation bug which prevents vhostuser accepting the chardev > when it uses FD passing, despite it being functionally correct. > > While the QEMU bug is being fixed, we need to avoid hitting this > problem with existing QEMU releases. > > The easiest way todo this is to simply disable FD passing for the > chardev used with vhostuser only. > > Daniel P. Berrangé (3): > qemu: remove chardevStdioLogd param from vhostuser code path > qemu: consolidate parameters of qemuBuildChrChardevStr into flags > qemu: don't use chardev FD passing for vhostuser backend > > src/qemu/qemu_command.c | 117 ++ > tests/qemuxml2argvdata/net-vhostuser.args | 3 +- > tests/qemuxml2argvtest.c | 2 +- > 3 files changed, 75 insertions(+), 47 deletions(-) > ACK series. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags
On 07/05/2018 01:36 PM, Daniel P. Berrangé wrote:
> There are two boolean parameters passed to qemuBuildChrChardevStr,
> and soon there will be a third. It will be clearer to understand
> from callers' POV if we use named flags instead.
>
> Signed-off-by: Daniel P. Berrangé
> ---
> src/qemu/qemu_command.c | 86 ++---
> 1 file changed, 55 insertions(+), 31 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9351b9fddb..7f3eccc1bd 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4935,6 +4935,12 @@ qemuOpenChrChardevUNIXSocket(const
> virDomainChrSourceDef *dev)
> return -1;
> }
>
> +
> +enum {
> +QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
> +QEMU_BUILD_CHARDEV_FILE_LOGD = (1 << 1),
> +};
> +
> /* This function outputs a -chardev command line option which describes only
> the
> * host side of the character device */
> static char *
> @@ -4946,8 +4952,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> const virDomainChrSourceDef *dev,
> const char *alias,
> virQEMUCapsPtr qemuCaps,
> - bool nowait,
> - bool chardevStdioLogd)
> + unsigned int flags)
> {
> virBuffer buf = VIR_BUFFER_INITIALIZER;
> bool telnet;
> @@ -4986,7 +4991,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
> _("append not supported in this QEMU binary"));
> goto cleanup;
> }
> -if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
> +if (qemuBuildChrChardevFileStr(flags & QEMU_BUILD_CHARDEV_FILE_LOGD ?
> + logManager : NULL,
> cmd, def, ,
> "path", dev->data.file.path,
> "append", dev->data.file.append) < 0)
> @@ -5033,7 +5039,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
>telnet ? ",telnet" : "");
>
> if (dev->data.tcp.listen)
> -virBufferAdd(, nowait ? ",server,nowait" : ",server", -1);
> +virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
> + ",server,nowait" : ",server", -1);
Well, since you're touching this: s/virBufferAdd/virBufferAddLit/ and
also perhaps get rid of the ternary operator?
diff --git i/src/qemu/qemu_command.c w/src/qemu/qemu_command.c
index 7f3eccc1bd..63c7ac0f82 100644
--- i/src/qemu/qemu_command.c
+++ w/src/qemu/qemu_command.c
@@ -5038,9 +5038,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
dev->data.tcp.service,
telnet ? ",telnet" : "");
-if (dev->data.tcp.listen)
-virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
- ",server,nowait" : ",server", -1);
+if (dev->data.tcp.listen) {
+virBufferAddLit(, ",server");
+if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
+virBufferAddLit(, ",nowait");
+}
qemuBuildChrChardevReconnectStr(, >data.tcp.reconnect);
@@ -5098,9 +5100,11 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
virBufferAsprintf(, "socket,id=%s,path=", charAlias);
virQEMUBuildBufferEscapeComma(, dev->data.nix.path);
}
-if (dev->data.nix.listen)
-virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
- ",server,nowait" : ",server", -1);
+if (dev->data.nix.listen) {
+virBufferAddLit(, ",server");
+if (flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT)
+virBufferAddLit(, ",nowait");
+}
qemuBuildChrChardevReconnectStr(, >data.nix.reconnect);
break;
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: consolidate parameters of qemuBuildChrChardevStr into flags
There are two boolean parameters passed to qemuBuildChrChardevStr,
and soon there will be a third. It will be clearer to understand
from callers' POV if we use named flags instead.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 86 ++---
1 file changed, 55 insertions(+), 31 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9351b9fddb..7f3eccc1bd 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4935,6 +4935,12 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef
*dev)
return -1;
}
+
+enum {
+QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
+QEMU_BUILD_CHARDEV_FILE_LOGD = (1 << 1),
+};
+
/* This function outputs a -chardev command line option which describes only
the
* host side of the character device */
static char *
@@ -4946,8 +4952,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
const virDomainChrSourceDef *dev,
const char *alias,
virQEMUCapsPtr qemuCaps,
- bool nowait,
- bool chardevStdioLogd)
+ unsigned int flags)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool telnet;
@@ -4986,7 +4991,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
_("append not supported in this QEMU binary"));
goto cleanup;
}
-if (qemuBuildChrChardevFileStr(chardevStdioLogd ? logManager : NULL,
+if (qemuBuildChrChardevFileStr(flags & QEMU_BUILD_CHARDEV_FILE_LOGD ?
+ logManager : NULL,
cmd, def, ,
"path", dev->data.file.path,
"append", dev->data.file.append) < 0)
@@ -5033,7 +5039,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
telnet ? ",telnet" : "");
if (dev->data.tcp.listen)
-virBufferAdd(, nowait ? ",server,nowait" : ",server", -1);
+virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
+ ",server,nowait" : ",server", -1);
qemuBuildChrChardevReconnectStr(, >data.tcp.reconnect);
@@ -5092,7 +5099,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
virQEMUBuildBufferEscapeComma(, dev->data.nix.path);
}
if (dev->data.nix.listen)
-virBufferAdd(, nowait ? ",server,nowait" : ",server", -1);
+virBufferAdd(, flags & QEMU_BUILD_CHARDEV_TCP_NOWAIT ?
+ ",server,nowait" : ",server", -1);
qemuBuildChrChardevReconnectStr(, >data.nix.reconnect);
break;
@@ -5426,6 +5434,9 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
qemuDomainObjPrivatePtr priv)
{
char *chrdev;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+if (priv->chardevStdioLogd)
+cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
if (!priv->monConfig)
return 0;
@@ -5433,8 +5444,7 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
if (!(chrdev = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
priv->monConfig, "monitor",
- priv->qemuCaps, true,
- priv->chardevStdioLogd)))
+ priv->qemuCaps, cdevflags)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, chrdev);
@@ -5559,6 +5569,9 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
char **chr,
bool chardevStdioLogd)
{
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+if (chardevStdioLogd)
+cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
*chr = NULL;
switch ((virDomainRNGBackend) rng->backend) {
@@ -5571,8 +5584,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
if (!(*chr = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
rng->source.chardev,
-rng->info.alias, qemuCaps, true,
-chardevStdioLogd)))
+rng->info.alias, qemuCaps,
+cdevflags)))
return -1;
}
@@ -8215,8 +8228,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
if (!(chardev = qemuBuildChrChardevStr(logManager, secManager,
cmd, cfg, def,
net->data.vhostuser,
-
[libvirt] [PATCH 0/3] Fix regression with vhostuser and chardev FD passing
QEMU has a validation bug which prevents vhostuser accepting the chardev when it uses FD passing, despite it being functionally correct. While the QEMU bug is being fixed, we need to avoid hitting this problem with existing QEMU releases. The easiest way todo this is to simply disable FD passing for the chardev used with vhostuser only. Daniel P. Berrangé (3): qemu: remove chardevStdioLogd param from vhostuser code path qemu: consolidate parameters of qemuBuildChrChardevStr into flags qemu: don't use chardev FD passing for vhostuser backend src/qemu/qemu_command.c | 117 ++ tests/qemuxml2argvdata/net-vhostuser.args | 3 +- tests/qemuxml2argvtest.c | 2 +- 3 files changed, 75 insertions(+), 47 deletions(-) -- 2.17.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] qemu: remove chardevStdioLogd param from vhostuser code path
The vhostuser network backend is only supported with the UNIX domain
socket chardev backend, so passing around chardevStdioLogd is not
required.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 04c5c28438..9351b9fddb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -8195,8 +8195,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
- unsigned int bootindex,
- bool chardevStdioLogd)
+ unsigned int bootindex)
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
@@ -8217,7 +8216,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
cmd, cfg, def,
net->data.vhostuser,
net->info.alias, qemuCaps,
false,
- chardevStdioLogd)))
+ false)))
goto cleanup;
break;
@@ -8291,8 +8290,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
virNetDevVPortProfileOp vmop,
bool standalone,
size_t *nnicindexes,
- int **nicindexes,
- bool chardevStdioLogd)
+ int **nicindexes)
{
int ret = -1;
char *nic = NULL, *host = NULL;
@@ -8415,8 +8413,7 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
ret = qemuBuildVhostuserCommandLine(driver, logManager, secManager,
cmd, def,
-net, qemuCaps, bootindex,
-chardevStdioLogd);
+net, qemuCaps, bootindex);
goto cleanup;
break;
@@ -8600,8 +8597,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
bool standalone,
size_t *nnicindexes,
int **nicindexes,
-unsigned int *bootHostdevNet,
-bool chardevStdioLogd)
+unsigned int *bootHostdevNet)
{
size_t i;
int last_good_net = -1;
@@ -8628,8 +8624,7 @@ qemuBuildNetCommandLine(virQEMUDriverPtr driver,
if (qemuBuildInterfaceCommandLine(driver, logManager, secManager,
cmd, def, net,
qemuCaps, bootNet, vmop,
standalone, nnicindexes,
- nicindexes,
- chardevStdioLogd) < 0)
+ nicindexes) < 0)
goto error;
last_good_net = i;
@@ -10290,8 +10285,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildNetCommandLine(driver, logManager, secManager, cmd, def,
qemuCaps, vmop, standalone,
-nnicindexes, nicindexes, ,
-chardevStdioLogd) < 0)
+nnicindexes, nicindexes, ) < 0)
goto error;
if (qemuBuildSmartcardCommandLine(logManager, secManager, cmd, cfg, def,
qemuCaps,
--
2.17.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: don't use chardev FD passing for vhostuser backend
QEMU chardevs have a bug which makes the vhostuser backend complain
about lack of support for FD passing when validating the chardev.
While this is ultimately QEMU's responsibility to fix, libvirt needs to
avoid tickling the bug.
Simply disabling chardev FD passing just for vhostuser's chardev is
the most prudent approach, avoiding need for a QEMU version number
check.
Signed-off-by: Daniel P. Berrangé
---
src/qemu/qemu_command.c | 31 +++
tests/qemuxml2argvdata/net-vhostuser.args | 3 +--
tests/qemuxml2argvtest.c | 2 +-
3 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7f3eccc1bd..aaa6d0ceda 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4939,6 +4939,7 @@ qemuOpenChrChardevUNIXSocket(const virDomainChrSourceDef
*dev)
enum {
QEMU_BUILD_CHARDEV_TCP_NOWAIT = (1 << 0),
QEMU_BUILD_CHARDEV_FILE_LOGD = (1 << 1),
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS = (1 << 2),
};
/* This function outputs a -chardev command line option which describes only
the
@@ -5080,7 +5081,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
break;
case VIR_DOMAIN_CHR_TYPE_UNIX:
-if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
+if ((flags & QEMU_BUILD_CHARDEV_UNIX_FD_PASS) &&
+virQEMUCapsGet(qemuCaps, QEMU_CAPS_CHARDEV_FD_PASS)) {
if (qemuSecuritySetSocketLabel(secManager, (virDomainDefPtr)def) <
0)
goto cleanup;
int fd = qemuOpenChrChardevUNIXSocket(dev);
@@ -5434,7 +5436,8 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
qemuDomainObjPrivatePtr priv)
{
char *chrdev;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (priv->chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -5569,7 +5572,8 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
char **chr,
bool chardevStdioLogd)
{
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
*chr = NULL;
@@ -8708,7 +8712,8 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *database;
const char *contAlias = NULL;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -8944,7 +8949,8 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager,
virBuffer buf = VIR_BUFFER_INITIALIZER;
char *devstr = NULL;
int rc;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9104,7 +9110,8 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
{
size_t i;
bool havespice = false;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9167,7 +9174,8 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9204,7 +9212,8 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9261,7 +9270,8 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
bool chardevStdioLogd)
{
size_t i;
-unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT;
+unsigned int cdevflags = QEMU_BUILD_CHARDEV_TCP_NOWAIT |
+QEMU_BUILD_CHARDEV_UNIX_FD_PASS;
if (chardevStdioLogd)
cdevflags |= QEMU_BUILD_CHARDEV_FILE_LOGD;
@@ -9441,7 +9451,8 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
bool
Re: [libvirt] [PATCH 4/5] qemu: mark graphics ports as used on migration
Please disregard, this one is definetly a wrong one.
On 04.07.2018 14:03, Nikolay Shirokovskiy wrote:
> Signed-off-by: Nikolay Shirokovskiy
> ---
> src/qemu/qemu_migration.c | 6 ++
> src/qemu/qemu_process.c | 2 +-
> src/qemu/qemu_process.h | 3 +++
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 435cd17..7e12ff6 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -4928,6 +4928,7 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
> qemuDomainJobInfoPtr jobInfo = NULL;
> bool inPostCopy = false;
> bool doKill = true;
> +size_t i;
>
> VIR_DEBUG("driver=%p, dconn=%p, vm=%p, cookiein=%s, cookieinlen=%d, "
>"cookieout=%p, cookieoutlen=%p, flags=0x%lx, retcode=%d",
> @@ -5000,6 +5001,11 @@ qemuMigrationDstFinish(virQEMUDriverPtr driver,
> if (qemuConnectAgent(driver, vm) < 0)
> goto endjob;
>
> +for (i = 0; i < vm->def->ngraphics; i++) {
> +if (qemuProcessGraphicsReservePorts(vm->def->graphics[i], true) < 0)
> +goto endjob;
> +}
> +
> if (flags & VIR_MIGRATE_PERSIST_DEST) {
> if (qemuMigrationDstPersist(driver, vm, mig, !v3proto) < 0) {
> /* Hmpf. Migration was successful, but making it persistent
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 09e0327..9f41313 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -4303,7 +4303,7 @@ qemuProcessStartHook(virQEMUDriverPtr driver,
> }
>
>
> -static int
> +int
> qemuProcessGraphicsReservePorts(virDomainGraphicsDefPtr graphics,
> bool reconnect)
> {
> diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h
> index 07ce3a9..5d234f0 100644
> --- a/src/qemu/qemu_process.h
> +++ b/src/qemu/qemu_process.h
> @@ -214,4 +214,7 @@ int qemuProcessStartManagedPRDaemon(virDomainObjPtr vm);
>
> void qemuProcessKillManagedPRDaemon(virDomainObjPtr vm);
>
> +int qemuProcessGraphicsReservePorts(virDomainGraphicsDefPtr graphics,
> +bool reconnect);
> +
> #endif /* __QEMU_PROCESS_H__ */
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 0/5] AUTHORS: Cleanups and fixes
On 07/04/2018 02:47 PM, Andrea Bolognani wrote: > Comparing the generated AUTHORS file before and after these > changes: > > AUTHORS | 51 +-- > 1 file changed, 1 insertion(+), 50 deletions(-) > > There would be plenty of room for additional deduplication, > but that's how far I can get based on concrete data as opposed > to relying on (more or less educated) guesses. > > Andrea Bolognani (5): > AUTHORS: Remove unnecessary remark > AUTHORS: Fix entries disagreeing with mailmap > mailmap: Fix consolidation rules > mailmap: Consolidate current and past maintainers > AUTHORS: Avoid duplicated entries > > .mailmap| 16 +--- > AUTHORS.in | 6 ++ > Makefile.am | 18 +- > 3 files changed, 28 insertions(+), 12 deletions(-) > ACK series. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: initialize character pointer xml value to avoid random crash
Thanks for your quick review ! :)
Have a nice day !
Luyao
- Original Message -
From: "Michal Prívozník"
To: "Luyao Huang" , libvir-list@redhat.com
Sent: Thursday, July 5, 2018 5:11:41 PM
Subject: Re: [libvirt] [PATCH] conf: initialize character pointer xml value to
avoid random crash
On 07/05/2018 06:34 AM, Luyao Huang wrote:
> If the code jump to the cleanup before assagin value to xml pointer,
> libvirtd may get crashed when try to free an uninitialized pointer.
>
> backtrace:
>
> 0 0x7428d59c in free () from /lib64/libc.so.6
> 1 0x7721314a in virFree (ptrptr=ptrptr@entry=0x7fffc67f1b00) at
> util/viralloc.c:582
> 2 0x77345ac4 in virDomainConfNWFilterInstantiate (vmname= out>,
>vmuuid=vmuuid@entry=0x7fffc0181ca8 "߉\237\\۔H\262\206z\340\302f\265\233z",
> net=,
>ignoreExists=ignoreExists@entry=true) at conf/domain_nwfilter.c:122
> 3 0x7fffca5a77f6 in qemuProcessFiltersInstantiate (ignoreExists=true,
> def=0x7fffc0181ca0) at qemu/qemu_process.c:3028
> 4 qemuProcessReconnect (opaque=) at qemu/qemu_process.c:7653
> 5 0x772c4895 in virThreadHelper (data=) at
> util/virthread.c:206
> 6 0x745dcdd5 in start_thread () from /lib64/libpthread.so.0
> 7 0x74305ead in clone () from /lib64/libc.so.6
>
> Signed-off-by: Luyao Huang
> ---
> src/conf/domain_nwfilter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
> index 948b324..24b5f42 100644
> --- a/src/conf/domain_nwfilter.c
> +++ b/src/conf/domain_nwfilter.c
> @@ -90,7 +90,7 @@ virDomainConfNWFilterInstantiate(const char *vmname,
> virConnectPtr conn = virGetConnectNWFilter();
> virNWFilterBindingDefPtr def = NULL;
> virNWFilterBindingPtr binding = NULL;
> -char *xml;
> +char *xml = NULL;
> int ret = -1;
>
> VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
>
Slightly reworked the commit message, ACKed and pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: initialize character pointer xml value to avoid random crash
On 07/05/2018 06:34 AM, Luyao Huang wrote:
> If the code jump to the cleanup before assagin value to xml pointer,
> libvirtd may get crashed when try to free an uninitialized pointer.
>
> backtrace:
>
> 0 0x7428d59c in free () from /lib64/libc.so.6
> 1 0x7721314a in virFree (ptrptr=ptrptr@entry=0x7fffc67f1b00) at
> util/viralloc.c:582
> 2 0x77345ac4 in virDomainConfNWFilterInstantiate (vmname= out>,
>vmuuid=vmuuid@entry=0x7fffc0181ca8 "߉\237\\۔H\262\206z\340\302f\265\233z",
> net=,
>ignoreExists=ignoreExists@entry=true) at conf/domain_nwfilter.c:122
> 3 0x7fffca5a77f6 in qemuProcessFiltersInstantiate (ignoreExists=true,
> def=0x7fffc0181ca0) at qemu/qemu_process.c:3028
> 4 qemuProcessReconnect (opaque=) at qemu/qemu_process.c:7653
> 5 0x772c4895 in virThreadHelper (data=) at
> util/virthread.c:206
> 6 0x745dcdd5 in start_thread () from /lib64/libpthread.so.0
> 7 0x74305ead in clone () from /lib64/libc.so.6
>
> Signed-off-by: Luyao Huang
> ---
> src/conf/domain_nwfilter.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/conf/domain_nwfilter.c b/src/conf/domain_nwfilter.c
> index 948b324..24b5f42 100644
> --- a/src/conf/domain_nwfilter.c
> +++ b/src/conf/domain_nwfilter.c
> @@ -90,7 +90,7 @@ virDomainConfNWFilterInstantiate(const char *vmname,
> virConnectPtr conn = virGetConnectNWFilter();
> virNWFilterBindingDefPtr def = NULL;
> virNWFilterBindingPtr binding = NULL;
> -char *xml;
> +char *xml = NULL;
> int ret = -1;
>
> VIR_DEBUG("vmname=%s portdev=%s filter=%s ignoreExists=%d",
>
Slightly reworked the commit message, ACKed and pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 6/6] qemu: Fetch pr-helper process info on reconnect
If qemu-pr-helper process died while libvirtd was not running no
event is emitted. Therefore, when reconnecting to the monitor we
must check the qemu-pr-helper process status and act accordingly.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_process.c | 61 +
1 file changed, 61 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index ac2f73c99e..b6db337e76 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2071,6 +2071,64 @@ qemuRefreshVirtioChannelState(virQEMUDriverPtr driver,
return ret;
}
+
+static int
+qemuProcessRefreshPRManagerState(virDomainObjPtr vm,
+ virHashTablePtr info)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+qemuMonitorPRManagerInfoPtr prManagerInfo;
+const char *managedAlias = qemuDomainGetManagedPRAlias();
+int ret = -1;
+
+if (!(prManagerInfo = virHashLookup(info, managedAlias))) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ _("missing info on pr-manager %s"),
+ managedAlias);
+goto cleanup;
+}
+
+priv->prDaemonRunning = prManagerInfo->connected;
+
+if (!priv->prDaemonRunning &&
+virDomainDefHasManagedPR(vm->def) &&
+qemuProcessStartManagedPRDaemon(vm) < 0)
+goto cleanup;
+
+ret = 0;
+ cleanup:
+return ret;
+}
+
+
+static int
+qemuRefreshPRManagerState(virQEMUDriverPtr driver,
+ virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+virHashTablePtr info = NULL;
+int ret = -1;
+
+if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER) ||
+!virDomainDefHasManagedPR(vm->def))
+return 0;
+
+qemuDomainObjEnterMonitor(driver, vm);
+ret = qemuMonitorGetPRManagerInfo(priv->mon, );
+if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ret = -1;
+
+if (ret < 0)
+goto cleanup;
+
+ret = qemuProcessRefreshPRManagerState(vm, info);
+
+ cleanup:
+virHashFree(info);
+return ret;
+}
+
+
static void
qemuRefreshRTC(virQEMUDriverPtr driver,
virDomainObjPtr vm)
@@ -7724,6 +7782,9 @@ qemuProcessReconnect(void *opaque)
if (qemuRefreshVirtioChannelState(driver, obj, QEMU_ASYNC_JOB_NONE) < 0)
goto error;
+if (qemuRefreshPRManagerState(driver, obj) < 0)
+goto error;
+
/* If querying of guest's RTC failed, report error, but do not kill the
domain. */
qemuRefreshRTC(driver, obj);
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/6] qemu: Wire up PR_MANAGER_STATUS_CHANGED event
This event is emitted on the monitor if one of pr-managers lost
connection to its pr-helper process. What libvirt needs to do is
restart the pr-helper process iff it corresponds to managed
pr-manager.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_driver.c | 17 ++
src/qemu/qemu_monitor.c | 15
src/qemu/qemu_monitor.h | 11 +
src/qemu/qemu_monitor_json.c | 23 ++
src/qemu/qemu_process.c | 55
7 files changed, 123 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c80b7870c8..73873c0110 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -13003,6 +13003,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
case QEMU_PROCESS_EVENT_MONITOR_EOF:
VIR_FREE(event->data);
break;
+case QEMU_PROCESS_EVENT_PR_DISCONNECT:
case QEMU_PROCESS_EVENT_LAST:
break;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 30d186a921..e748d78adb 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -477,6 +477,7 @@ typedef enum {
QEMU_PROCESS_EVENT_SERIAL_CHANGED,
QEMU_PROCESS_EVENT_BLOCK_JOB,
QEMU_PROCESS_EVENT_MONITOR_EOF,
+QEMU_PROCESS_EVENT_PR_DISCONNECT,
QEMU_PROCESS_EVENT_LAST
} qemuProcessEventType;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 9a35e04a85..5de9aaefbb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4778,6 +4778,20 @@ processMonitorEOFEvent(virQEMUDriverPtr driver,
}
+static void
+processPRDisconnectEvent(virDomainObjPtr vm)
+{
+qemuDomainObjPrivatePtr priv = vm->privateData;
+
+if (!virDomainObjIsActive(vm))
+return;
+
+if (!priv->prDaemonRunning &&
+virDomainDefHasManagedPR(vm->def))
+qemuProcessStartManagedPRDaemon(vm);
+}
+
+
static void qemuProcessEventHandler(void *data, void *opaque)
{
struct qemuProcessEvent *processEvent = data;
@@ -4815,6 +4829,9 @@ static void qemuProcessEventHandler(void *data, void
*opaque)
case QEMU_PROCESS_EVENT_MONITOR_EOF:
processMonitorEOFEvent(driver, vm);
break;
+case QEMU_PROCESS_EVENT_PR_DISCONNECT:
+processPRDisconnectEvent(vm);
+break;
case QEMU_PROCESS_EVENT_LAST:
break;
}
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6ed475ede0..ca95f6f94a 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1669,6 +1669,21 @@ qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon,
}
+int
+qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon,
+ const char *prManager,
+ bool connected)
+{
+int ret = -1;
+VIR_DEBUG("mon=%p, prManager='%s', connected=%d", mon, prManager,
connected);
+
+QEMU_MONITOR_CALLBACK(mon, ret, domainPRManagerStatusChanged,
+ mon->vm, prManager, connected);
+
+return ret;
+}
+
+
int
qemuMonitorSetCapabilities(qemuMonitorPtr mon)
{
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index b3d62324b4..f1ea0bc541 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -273,6 +273,12 @@ typedef int
(*qemuMonitorDomainDumpCompletedCallback)(qemuMonitorPtr mon,
const char *error,
void *opaque);
+typedef int (*qemuMonitorDomainPRManagerStatusChangedCallback)(qemuMonitorPtr
mon,
+ virDomainObjPtr
vm,
+ const char
*prManager,
+ bool connected,
+ void *opaque);
+
typedef struct _qemuMonitorCallbacks qemuMonitorCallbacks;
typedef qemuMonitorCallbacks *qemuMonitorCallbacksPtr;
struct _qemuMonitorCallbacks {
@@ -305,6 +311,7 @@ struct _qemuMonitorCallbacks {
qemuMonitorDomainAcpiOstInfoCallback domainAcpiOstInfo;
qemuMonitorDomainBlockThresholdCallback domainBlockThreshold;
qemuMonitorDomainDumpCompletedCallback domainDumpCompleted;
+qemuMonitorDomainPRManagerStatusChangedCallback
domainPRManagerStatusChanged;
};
char *qemuMonitorEscapeArg(const char *in);
@@ -433,6 +440,10 @@ int qemuMonitorEmitDumpCompleted(qemuMonitorPtr mon,
qemuMonitorDumpStatsPtr stats,
const char *error);
+int qemuMonitorEmitPRManagerStatusChanged(qemuMonitorPtr mon,
+ const char *prManager,
+ bool connected);
+
int qemuMonitorStartCPUs(qemuMonitorPtr mon);
int
[libvirt] [PATCH v2 5/6] qemu_monitor: Introduce qemuMonitorJSONGetPRManagerInfo
This function fetches status of all pr-managers. So far, qemu
reports only a single attribute "connected" but that fits our
needs.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_monitor.c | 25 +
src/qemu/qemu_monitor.h | 9 +
src/qemu/qemu_monitor_json.c | 83
src/qemu/qemu_monitor_json.h | 4 +++
4 files changed, 121 insertions(+)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index ca95f6f94a..3514e9f8a1 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4335,3 +4335,28 @@ qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon)
return qemuMonitorJSONGetSEVMeasurement(mon);
}
+
+
+int
+qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon,
+virHashTablePtr *retinfo)
+{
+int ret = -1;
+virHashTablePtr info = NULL;
+
+*retinfo = NULL;
+
+QEMU_CHECK_MONITOR(mon);
+
+if (!(info = virHashCreate(10, virHashValueFree)))
+goto cleanup;
+
+if (qemuMonitorJSONGetPRManagerInfo(mon, info) < 0)
+goto cleanup;
+
+VIR_STEAL_PTR(*retinfo, info);
+ret = 0;
+ cleanup:
+virHashFree(info);
+return ret;
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index f1ea0bc541..e588d73678 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1156,4 +1156,13 @@ int qemuMonitorBlockdevDel(qemuMonitorPtr mon,
char *
qemuMonitorGetSEVMeasurement(qemuMonitorPtr mon);
+typedef struct _qemuMonitorPRManagerInfo qemuMonitorPRManagerInfo;
+typedef qemuMonitorPRManagerInfo *qemuMonitorPRManagerInfoPtr;
+struct _qemuMonitorPRManagerInfo {
+bool connected;
+};
+
+int qemuMonitorGetPRManagerInfo(qemuMonitorPtr mon,
+virHashTablePtr *retinfo);
+
#endif /* QEMU_MONITOR_H */
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 03c94cd88b..291a505d5d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8065,3 +8065,86 @@ qemuMonitorJSONGetSEVMeasurement(qemuMonitorPtr mon)
virJSONValueFree(reply);
return measurement;
}
+
+
+/*
+ * Example return data
+ *
+ * "return": [
+ * { "connected": true, "id": "pr-helper0" }
+ * ]}
+ *
+ */
+static int
+qemuMonitorJSONExtractPRManagerInfo(virJSONValuePtr reply,
+virHashTablePtr info)
+{
+qemuMonitorPRManagerInfoPtr entry = NULL;
+virJSONValuePtr data;
+int ret = -1;
+size_t i;
+
+data = virJSONValueObjectGetArray(reply, "return");
+
+for (i = 0; i < virJSONValueArraySize(data); i++) {
+virJSONValuePtr prManager = virJSONValueArrayGet(data, i);
+const char *alias;
+
+if (!prManager)
+goto malformed;
+
+if (!(alias = virJSONValueObjectGetString(prManager, "id")))
+goto malformed;
+
+if (VIR_ALLOC(entry) < 0)
+goto cleanup;
+
+if (virJSONValueObjectGetBoolean(prManager,
+ "connected",
+ >connected) < 0) {
+goto malformed;
+}
+
+if (virHashAddEntry(info, alias, entry) < 0)
+goto cleanup;
+
+entry = NULL;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(entry);
+return ret;
+
+ malformed:
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("malformed prManager reply"));
+goto cleanup;
+}
+
+
+int
+qemuMonitorJSONGetPRManagerInfo(qemuMonitorPtr mon,
+virHashTablePtr info)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("query-pr-managers",
+ NULL)))
+return -1;
+
+if (qemuMonitorJSONCommand(mon, cmd, ) < 0)
+goto cleanup;
+
+if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0)
+goto cleanup;
+
+ret = qemuMonitorJSONExtractPRManagerInfo(reply, info);
+ cleanup:
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 6bc0dd3ad2..66536ceb97 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -550,4 +550,8 @@ int qemuMonitorJSONBlockdevDel(qemuMonitorPtr mon,
const char *nodename)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuMonitorJSONGetPRManagerInfo(qemuMonitorPtr mon,
+virHashTablePtr info)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
#endif /* QEMU_MONITOR_JSON_H */
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/6] virStoragePRDefFormat: Suppress path formatting for migratable XML
If there are managed reservations for a disk source, the path to
the pr-helper socket is generated automatically by libvirt when
needed and points somewhere under priv->libDir. Therefore it is
very unlikely that the path will work even on migration
destination (the libDir is derived from domain short name and its
ID).
Signed-off-by: Michal Privoznik
---
src/conf/domain_conf.c| 3 ++-
src/util/virstoragefile.c | 6 --
src/util/virstoragefile.h | 3 ++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f4e59f6c91..70eb45f03a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -23548,7 +23548,8 @@ virDomainStorageSourceFormat(virBufferPtr attrBuf,
return -1;
if (src->pr)
-virStoragePRDefFormat(childBuf, src->pr);
+virStoragePRDefFormat(childBuf, src->pr,
+ flags & VIR_DOMAIN_DEF_FORMAT_MIGRATABLE);
return 0;
}
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 6ede542df6..58f67278da 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1982,11 +1982,13 @@ virStoragePRDefParseXML(xmlXPathContextPtr ctxt)
void
virStoragePRDefFormat(virBufferPtr buf,
- virStoragePRDefPtr prd)
+ virStoragePRDefPtr prd,
+ bool migratable)
{
virBufferAsprintf(buf, "managed));
-if (prd->path) {
+if (prd->path &&
+(prd->managed == VIR_TRISTATE_BOOL_NO || !migratable)) {
virBufferAddLit(buf, ">\n");
virBufferAdjustIndent(buf, 2);
virBufferAddLit(buf, "https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/6] qemuDomainValidateStorageSource: Relax PR validation
Rather than rejecting the user provided path and alias for the
managed PR reservation we will ignore the provided path. The
reason is that migration XML does contain path even for managed
reservations.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_domain.c | 19 ++-
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b10bbc40a4..c80b7870c8 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4615,19 +4615,11 @@ qemuDomainValidateStorageSource(virStorageSourcePtr src,
}
}
-if (src->pr) {
-if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("reservations not supported with this QEMU
binary"));
-return -1;
-}
-
-if (virStoragePRDefIsManaged(src->pr) && src->pr->path) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("'path' attribute should not be provided for "
- "managed reservations"));
-return -1;
-}
+if (src->pr &&
+!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PR_MANAGER_HELPER)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("reservations not supported with this QEMU binary"));
+return -1;
}
return 0;
@@ -12855,6 +12847,7 @@ qemuDomainPrepareStorageSourcePR(virStorageSourcePtr
src,
return 0;
if (virStoragePRDefIsManaged(src->pr)) {
+VIR_FREE(src->pr->path);
if (!(src->pr->path = qemuDomainGetManagedPRSocketPath(priv)))
return -1;
if (VIR_STRDUP(src->pr->mgralias, qemuDomainGetManagedPRAlias()) < 0)
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/6] Couple of PR fixes and improvements
v2 of: https://www.redhat.com/archives/libvir-list/2018-July/msg00243.html Diff to v1: - Dropped 4/7 from the original series, as it's no longer needed - Reworked alias matching, instead of iterating through all disks trying to find a matching alias, retval of qemuDomainGetManagedPRAlias() is compared directly - The PR_MANAGER_STATUS_CHANGED event handling is done from worker pool rather than event loop. Patches 1 and 3 were ACKed already. Michal Privoznik (6): qemuProcessStartPRDaemonHook: Try to set NS iff domain was started with one qemuDomainValidateStorageSource: Relax PR validation virStoragePRDefFormat: Suppress path formatting for migratable XML qemu: Wire up PR_MANAGER_STATUS_CHANGED event qemu_monitor: Introduce qemuMonitorJSONGetPRManagerInfo qemu: Fetch pr-helper process info on reconnect src/conf/domain_conf.c | 3 +- src/qemu/qemu_domain.c | 20 +++ src/qemu/qemu_domain.h | 1 + src/qemu/qemu_driver.c | 17 ++ src/qemu/qemu_monitor.c | 40 ++ src/qemu/qemu_monitor.h | 20 +++ src/qemu/qemu_monitor_json.c | 106 +++ src/qemu/qemu_monitor_json.h | 4 ++ src/qemu/qemu_process.c | 128 +-- src/util/virstoragefile.c| 6 +- src/util/virstoragefile.h| 3 +- 11 files changed, 326 insertions(+), 22 deletions(-) -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/6] qemuProcessStartPRDaemonHook: Try to set NS iff domain was started with one
Users have possibility to disable qemu namespace feature (e.g.
because they are running on *BSD which lacks Linux NS support).
If that's the case we should not try to move qemu-pr-helper into
the same namespace as qemu is in.
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_process.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 40d35cbe6b..f200729cb1 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2522,12 +2522,14 @@ qemuProcessStartPRDaemonHook(void *opaque)
int *fds = NULL;
int ret = -1;
-if (virProcessGetNamespaces(vm->pid, , ) < 0)
-return ret;
+if (qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT)) {
+if (virProcessGetNamespaces(vm->pid, , ) < 0)
+return ret;
-if (nfds > 0 &&
-virProcessSetNamespaces(nfds, fds) < 0)
-goto cleanup;
+if (nfds > 0 &&
+virProcessSetNamespaces(nfds, fds) < 0)
+goto cleanup;
+}
ret = 0;
cleanup:
--
2.16.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh.pod: Drop --persistent for detach-device-alias
https://bugzilla.redhat.com/show_bug.cgi?id=1598087 The detach-device-alias never supported --persistent argument. Drop it from the man page. Signed-off-by: Michal Privoznik --- Pushed under trivial rule. tools/virsh.pod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh.pod b/tools/virsh.pod index dc100db9f3..003becb7c3 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -3197,7 +3197,7 @@ Note that older versions of virsh used I<--config> as an alias for I<--persistent>. =item B I I -[[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] +[[[I<--live>] [I<--config>] | [I<--current> Detach a device with given I from the I. -- 2.16.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
