8:38 a.m.
v2 of:
https://www.redhat.com/archives/libvir-list/2016-August/msg00806.html
The first patches are mostly code movement and cleanups.
diff to v1:
- Hopefully all John's review suggestions are worked in now
Michal Privoznik (14):
virDomainNetDefParseXML: Realign
virDomainNetGetActualType: Return type is virDomainNetType
qemuBuildInterfaceCommandLine: Move hostdev handling a bit further
qemuBuildInterfaceCommandLine: Move vhostuser handling a bit further
qemuBuildInterfaceCommandLine: Move from if-else forest to switch
qemuDomainAttachNetDevice: Move hostdev handling a bit further
qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug
qemuBuildHostNetStr: Explicitly enumerate net types
qemuBuildChrChardevStr: Introduce @nowait argument
qemuBuildVhostuserCommandLine: Reuse qemuBuildChrChardevStr
qemuBuildInterfaceCommandLine: Pass proper args
qemuBuildVhostuserCommandLine: Unify -netdev creation
qemuBuildHostNetStr: Support VIR_DOMAIN_NET_TYPE_VHOSTUSER
qemu_hotplug: Support interface type of vhost-user hotplug
src/bhyve/bhyve_command.c | 2 +-
src/bhyve/bhyve_process.c | 2 +-
src/conf/domain_conf.c | 16 +-
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_driver.c | 9 +-
src/qemu/qemu_command.c | 182 +++++++++++++--------
src/qemu/qemu_hotplug.c | 130 +++++++++++----
src/qemu/qemu_process.c | 13 +-
.../qemuxml2argv-net-hostdev-fail.xml | 39 +++++
.../qemuxml2argv-net-vhostuser-fail.xml | 36 ++++
.../qemuxml2argv-net-vhostuser-multiq.args | 6 +-
.../qemuxml2argv-net-vhostuser.args | 4 +-
tests/qemuxml2argvtest.c | 7 +
15 files changed, 334 insertions(+), 118 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
--
2.8.4
8:38 a.m.
New subject: [libvirt] [PATCH v2 01/14] virDomainNetDefParseXML: Realign
There are couple of formatting issues. No functional change
though.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f562323..8f04e6f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9170,8 +9170,8 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
ifname = virXMLPropString(cur, "dev");
if (ifname &&
(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
- (STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX) ||
- (prefix && STRPREFIX(ifname, prefix)))) {
+ (STRPREFIX(ifname, VIR_NET_GENERATED_PREFIX) ||
+ (prefix && STRPREFIX(ifname, prefix)))) {
/* An auto-generated target name, blank it out */
VIR_FREE(ifname);
}
@@ -9732,9 +9732,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
filter = NULL;
def->filterparams = filterparams;
filterparams = NULL;
- break;
+ break;
default:
- break;
+ break;
}
}
--
2.8.4
9:57 a.m.
New subject: [libvirt] [PATCH v2 01/14] virDomainNetDefParseXML: Realign
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
There are couple of formatting issues. No functional change
though.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/conf/domain_conf.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
ACK
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 02/14] virDomainNetGetActualType: Return type is virDomainNetType
This function for some weird reason returns integer instead of
virDomainNetType type. It is important to return the correct type
so that we know what values we can expect.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/bhyve/bhyve_command.c | 2 +-
src/bhyve/bhyve_process.c | 2 +-
src/conf/domain_conf.c | 8 ++++----
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_driver.c | 9 +++++++--
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_hotplug.c | 4 ++--
src/qemu/qemu_process.c | 13 ++++++++++++-
10 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 9ad3f9b..55ad950 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -52,7 +52,7 @@ bhyveBuildNetArgStr(const virDomainDef *def,
char macaddr[VIR_MAC_STRING_BUFLEN];
char *realifname = NULL;
char *brname = NULL;
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 6db070f..9d80976 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -77,7 +77,7 @@ bhyveNetCleanup(virDomainObjPtr vm)
for (i = 0; i < vm->def->nnets; i++) {
virDomainNetDefPtr net = vm->def->nets[i];
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
if (net->ifname) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f04e6f..9343770 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20804,7 +20804,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
bool inSubelement,
unsigned int flags)
{
- int actualType = virDomainNetGetActualType(def);
+ virDomainNetType actualType = virDomainNetGetActualType(def);
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def),
@@ -20884,7 +20884,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
unsigned int flags)
{
- unsigned int type;
+ virDomainNetType type;
const char *typeStr;
if (!def)
@@ -21039,7 +21039,7 @@ virDomainNetDefFormat(virBufferPtr buf,
char *prefix,
unsigned int flags)
{
- unsigned int actualType = virDomainNetGetActualType(def);
+ virDomainNetType actualType = virDomainNetGetActualType(def);
bool publicActual = false;
int sourceLines = 0;
const char *typeStr;
@@ -24781,7 +24781,7 @@ virDomainStateReasonFromString(virDomainState state, const char
*reason)
* otherwise return the value from the NetDef.
*/
-int
+virDomainNetType
virDomainNetGetActualType(virDomainNetDefPtr iface)
{
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a70bc21..95bcf4f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2814,7 +2814,7 @@ int virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr
def,
const char *socket)
ATTRIBUTE_NONNULL(1);
-int virDomainNetGetActualType(virDomainNetDefPtr iface);
+virDomainNetType virDomainNetGetActualType(virDomainNetDefPtr iface);
const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface);
int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface);
const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index db2c1dc..3df04cf 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -937,7 +937,7 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
- int actualType;
+ virDomainNetType actualType;
/* If appropriate, grab a physical device from the configured
* network's pool of devices, or resolve bridge device name
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b66cb1f..f87f549 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3328,7 +3328,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
virDomainNetDefPtr net)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- int actualType;
+ virDomainNetType actualType;
libxl_device_nic nic;
int ret = -1;
char mac[VIR_MAC_STRING_BUFLEN];
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a9e664c..91f67f2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3943,7 +3943,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
{
virLXCDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
- int actualType;
+ virDomainNetType actualType;
virNetDevBandwidthPtr actualBandwidth;
char *veth = NULL;
@@ -4030,6 +4030,10 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
case VIR_DOMAIN_NET_TYPE_DIRECT:
ignore_value(virNetDevMacVLanDelete(veth));
break;
+
+ default:
+ /* no-op */
+ break;
}
}
@@ -4430,7 +4434,8 @@ static int
lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
- int detachidx, actualType, ret = -1;
+ int detachidx, ret = -1;
+ virDomainNetType actualType;
virDomainNetDefPtr detach = NULL;
virNetDevVPortProfilePtr vport = NULL;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 578ff8b..2aebf8a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7910,7 +7910,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
size_t vhostfdSize = 0;
char **tapfdName = NULL;
char **vhostfdName = NULL;
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = NULL;
virNetDevBandwidthPtr actualBandwidth;
size_t i;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 72dd93b..bb8ea0d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -927,7 +927,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
int vlan;
bool releaseaddr = false;
bool iface_connected = false;
- int actualType;
+ virDomainNetType actualType;
virNetDevBandwidthPtr actualBandwidth;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainCCWAddressSetPtr ccwaddrs = NULL;
@@ -2373,7 +2373,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainNetDefPtr newdev = dev->data.net;
virDomainNetDefPtr *devslot = NULL;
virDomainNetDefPtr olddev;
- int oldType, newType;
+ virDomainNetType oldType, newType;
bool needReconnect = false;
bool needBridgeChange = false;
bool needFilterChange = false;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a4bc082..aa6c36f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4628,7 +4628,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
- int actualType;
+ virDomainNetType actualType;
/* If appropriate, grab a physical device from the configured
* network's pool of devices, or resolve bridge device name
@@ -6038,6 +6038,17 @@ void qemuProcessStop(virQEMUDriverPtr driver,
ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
#endif
break;
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ /* No special cleanup procedure for these types. */
+ break;
}
/* release the physical device (or any other resources used by
* this interface in the network driver
--
2.8.4
9:57 a.m.
New subject: [libvirt] [PATCH v2 02/14] virDomainNetGetActualType: Return type is virDomainNetType
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
This function for some weird reason returns integer instead of
virDomainNetType type. It is important to return the correct type
so that we know what values we can expect.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/bhyve/bhyve_command.c | 2 +-
src/bhyve/bhyve_process.c | 2 +-
src/conf/domain_conf.c | 8 ++++----
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_driver.c | 9 +++++++--
src/qemu/qemu_command.c | 2 +-
src/qemu/qemu_hotplug.c | 4 ++--
src/qemu/qemu_process.c | 13 ++++++++++++-
10 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index 9ad3f9b..55ad950 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -52,7 +52,7 @@ bhyveBuildNetArgStr(const virDomainDef *def,
char macaddr[VIR_MAC_STRING_BUFLEN];
char *realifname = NULL;
char *brname = NULL;
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0)
diff --git a/src/bhyve/bhyve_process.c b/src/bhyve/bhyve_process.c
index 6db070f..9d80976 100644
--- a/src/bhyve/bhyve_process.c
+++ b/src/bhyve/bhyve_process.c
@@ -77,7 +77,7 @@ bhyveNetCleanup(virDomainObjPtr vm)
for (i = 0; i < vm->def->nnets; i++) {
virDomainNetDefPtr net = vm->def->nets[i];
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
if (net->ifname) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8f04e6f..9343770 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -20804,7 +20804,7 @@ virDomainActualNetDefContentsFormat(virBufferPtr buf,
bool inSubelement,
unsigned int flags)
{
- int actualType = virDomainNetGetActualType(def);
+ virDomainNetType actualType = virDomainNetGetActualType(def);
if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
if (virDomainHostdevDefFormatSubsys(buf, virDomainNetGetActualHostdev(def),
@@ -20884,7 +20884,7 @@ virDomainActualNetDefFormat(virBufferPtr buf,
virDomainNetDefPtr def,
unsigned int flags)
{
- unsigned int type;
+ virDomainNetType type;
const char *typeStr;
if (!def)
@@ -21039,7 +21039,7 @@ virDomainNetDefFormat(virBufferPtr buf,
char *prefix,
unsigned int flags)
{
- unsigned int actualType = virDomainNetGetActualType(def);
+ virDomainNetType actualType = virDomainNetGetActualType(def);
bool publicActual = false;
int sourceLines = 0;
const char *typeStr;
@@ -24781,7 +24781,7 @@ virDomainStateReasonFromString(virDomainState state, const char
*reason)
* otherwise return the value from the NetDef.
*/
-int
+virDomainNetType
virDomainNetGetActualType(virDomainNetDefPtr iface)
{
if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a70bc21..95bcf4f 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2814,7 +2814,7 @@ int virDomainGraphicsListenAppendSocket(virDomainGraphicsDefPtr
def,
const char *socket)
ATTRIBUTE_NONNULL(1);
-int virDomainNetGetActualType(virDomainNetDefPtr iface);
+virDomainNetType virDomainNetGetActualType(virDomainNetDefPtr iface);
const char *virDomainNetGetActualBridgeName(virDomainNetDefPtr iface);
int virDomainNetGetActualBridgeMACTableManager(virDomainNetDefPtr iface);
const char *virDomainNetGetActualDirectDev(virDomainNetDefPtr iface);
diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
index db2c1dc..3df04cf 100644
--- a/src/libxl/libxl_domain.c
+++ b/src/libxl/libxl_domain.c
@@ -937,7 +937,7 @@ libxlNetworkPrepareDevices(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
- int actualType;
+ virDomainNetType actualType;
/* If appropriate, grab a physical device from the configured
* network's pool of devices, or resolve bridge device name
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index b66cb1f..f87f549 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3328,7 +3328,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver,
virDomainNetDefPtr net)
{
libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
- int actualType;
+ virDomainNetType actualType;
libxl_device_nic nic;
int ret = -1;
char mac[VIR_MAC_STRING_BUFLEN];
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index a9e664c..91f67f2 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3943,7 +3943,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
{
virLXCDomainObjPrivatePtr priv = vm->privateData;
int ret = -1;
- int actualType;
+ virDomainNetType actualType;
virNetDevBandwidthPtr actualBandwidth;
char *veth = NULL;
@@ -4030,6 +4030,10 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn,
case VIR_DOMAIN_NET_TYPE_DIRECT:
ignore_value(virNetDevMacVLanDelete(veth));
break;
+
+ default:
+ /* no-op */
+ break;
Naturally this draws the attention of the [dead_error_condition] for
Coverity. Oh and if each of the other cases are added instead of using
default, then there's multiple dead error conditions.... It just
doesn't make sense <sigh>.
Even stranger still if I replace the actualType with just using
virDomainNetGetActualType(net) directly, then the Coverity warnings go
away. So there's something about assigning the variable that causes
Coverity to stick it's nose where it doesn't belong... <double sigh>
IDC if you keep things as is, I'll just add a false positive in my local
build...
Not that it matters (or is related), but the switch/case before this one
has no reason to go to cleanup - each condition could just return -1.
The only reason to go to cleanup is if "veth" exists and the only way
for that to happen is if the previous cases are successful.
}
}
@@ -4430,7 +4434,8 @@ static int
lxcDomainDetachDeviceNetLive(virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
- int detachidx, actualType, ret = -1;
+ int detachidx, ret = -1;
+ virDomainNetType actualType;
virDomainNetDefPtr detach = NULL;
virNetDevVPortProfilePtr vport = NULL;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 578ff8b..2aebf8a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7910,7 +7910,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
size_t vhostfdSize = 0;
char **tapfdName = NULL;
char **vhostfdName = NULL;
- int actualType = virDomainNetGetActualType(net);
+ virDomainNetType actualType = virDomainNetGetActualType(net);
virQEMUDriverConfigPtr cfg = NULL;
virNetDevBandwidthPtr actualBandwidth;
size_t i;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 72dd93b..bb8ea0d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -927,7 +927,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
int vlan;
bool releaseaddr = false;
bool iface_connected = false;
- int actualType;
+ virDomainNetType actualType;
virNetDevBandwidthPtr actualBandwidth;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainCCWAddressSetPtr ccwaddrs = NULL;
@@ -2373,7 +2373,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
virDomainNetDefPtr newdev = dev->data.net;
virDomainNetDefPtr *devslot = NULL;
virDomainNetDefPtr olddev;
- int oldType, newType;
+ virDomainNetType oldType, newType;
bool needReconnect = false;
bool needBridgeChange = false;
bool needFilterChange = false;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index a4bc082..aa6c36f 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -4628,7 +4628,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def)
for (i = 0; i < def->nnets; i++) {
virDomainNetDefPtr net = def->nets[i];
- int actualType;
+ virDomainNetType actualType;
/* If appropriate, grab a physical device from the configured
* network's pool of devices, or resolve bridge device name
@@ -6038,6 +6038,17 @@ void qemuProcessStop(virQEMUDriverPtr driver,
ignore_value(virNetDevTapDelete(net->ifname, net->backend.tap));
#endif
break;
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ /* No special cleanup procedure for these types. */
+ break;
Ironically Coverity doesn't complain on this one. There's no rhyme or
reason... This of course is where/why I got the idea to not use the
local variable.
ACK for the changes
John
}
/* release the physical device (or any other resources used by
* this interface in the network driver
8:38 a.m.
New subject: [libvirt] [PATCH v2 03/14] qemuBuildInterfaceCommandLine: Move hostdev handling a bit further
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 13 ++++----
.../qemuxml2argv-net-hostdev-fail.xml | 39 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 4 +++
3 files changed, 49 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2aebf8a..0da2add 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7922,13 +7922,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)
return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
- if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
- /* NET_TYPE_HOSTDEV devices are really hostdev devices, so
- * their commandlines are constructed with other hostdevs.
- */
- return 0;
- }
-
/* Currently nothing besides TAP devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
@@ -8008,6 +8001,12 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
if (qemuInterfaceEthernetConnect(def, driver, net,
tapfd, tapfdSize) < 0)
goto cleanup;
+ } else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ /* NET_TYPE_HOSTDEV devices are really hostdev devices, so
+ * their commandlines are constructed with other hostdevs.
+ */
+ ret = 0;
+ goto cleanup;
}
/* For types whose implementations use a netdev on the host, add
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
new file mode 100644
index 0000000..7807d79
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
@@ -0,0 +1,39 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <interface type='hostdev' managed='yes'>
+ <mac address='00:11:22:33:44:55'/>
+ <source>
+ <address type='pci' domain='0x0000' bus='0x03'
slot='0x07' function='0x1'/>
+ </source>
+ <model type='virtio'/>
+ <filterref filter='myfilter'/>
+ <backend tap='/dev/mytap'/>
+ <driver queues='4'/>
+ </interface>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 4b9ecb8..442ab31 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1104,6 +1104,10 @@ mymain(void)
QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_HOST_PCI_MULTIDOMAIN);
DO_TEST_FAILURE("net-hostdev-vfio-multidomain",
QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VFIO_PCI);
+ DO_TEST_FAILURE("net-hostdev-fail",
+ QEMU_CAPS_NODEFCONFIG,
+ QEMU_CAPS_DEVICE_VFIO_PCI);
+
DO_TEST("serial-vc", NONE);
DO_TEST("serial-pty", NONE);
--
2.8.4
9:58 a.m.
New subject: [libvirt] [PATCH v2 03/14] qemuBuildInterfaceCommandLine: Move hostdev handling a bit further
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 13 ++++----
.../qemuxml2argv-net-hostdev-fail.xml | 39 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 4 +++
3 files changed, 49 insertions(+), 7 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
Is it safe to assume that previously if the XML had "<filterref
filter='myfilter'/>, <backend tap='/dev/mytap'/>, and/or
"<driver
queues='4'/>" that it would be ignored, but now there will be an error
at command line build time?
Hopefully we get no complaints ;-)
ACK
John
12:50 a.m.
New subject: [libvirt] [PATCH v2 03/14] qemuBuildInterfaceCommandLine: Move hostdev handling a bit further
On 13.10.2016 22:58, John Ferlan wrote:
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
> The idea is to have function that does some checking of the
> arguments at its beginning and then have one big switch for all
> the interface types it supports. Each one of them generating the
> corresponding part of the command line.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 13 ++++----
> .../qemuxml2argv-net-hostdev-fail.xml | 39 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 4 +++
> 3 files changed, 49 insertions(+), 7 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
>
Is it safe to assume that previously if the XML had "<filterref
filter='myfilter'/>, <backend tap='/dev/mytap'/>, and/or
"<driver
queues='4'/>" that it would be ignored, but now there will be an error
at command line build time?
Yeah. I mean, we were lying to users before letting them think we set
one of those things you mentioned. We should error out.
Hopefully we get no complaints ;-)
Well, if we do, we can at least tell them sorry for fooling you :-)
Michal
8:38 a.m.
New subject: [libvirt] [PATCH v2 04/14] qemuBuildInterfaceCommandLine: Move vhostuser handling a bit further
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 9 +++---
.../qemuxml2argv-net-vhostuser-fail.xml | 36 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 3 ++
3 files changed, 44 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0da2add..5ca82ec 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7919,15 +7919,13 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
if (!bootindex)
bootindex = net->info.bootIndex;
- if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)
- return qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
-
/* Currently nothing besides TAP devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
- actualType == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
+ actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
+ actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Multiqueue network is not supported for: %s"),
virDomainNetTypeToString(actualType));
@@ -8007,6 +8005,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
*/
ret = 0;
goto cleanup;
+ } else if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ ret = qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
+ goto cleanup;
}
/* For types whose implementations use a netdev on the host, add
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
new file mode 100644
index 0000000..e9fe14f
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219136</memory>
+ <currentMemory unit='KiB'>219136</currentMemory>
+ <vcpu placement='static'>1</vcpu>
+ <os>
+ <type arch='i686' machine='pc'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/bin/qemu</emulator>
+ <disk type='block' device='disk'>
+ <driver name='qemu' type='raw'/>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='hda' bus='ide'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <interface type='vhostuser'>
+ <mac address='52:54:00:ee:96:6b'/>
+ <source type='unix' path='/tmp/vhost0.sock'
mode='server'/>
+ <model type='virtio'/>
+ <filterref filter='myfilter'/>
+ <backend tap='/dev/mytap'/>
+ <driver queues='4'/>
+ </interface>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 442ab31..8deb651 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1066,6 +1066,9 @@ mymain(void)
DO_TEST("net-vhostuser-multiq",
QEMU_CAPS_NETDEV, QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
DO_TEST_FAILURE("net-vhostuser-multiq", QEMU_CAPS_NETDEV);
+ DO_TEST_FAILURE("net-vhostuser-fail",
+ QEMU_CAPS_NETDEV,
+ QEMU_CAPS_VHOSTUSER_MULTIQUEUE);
DO_TEST("net-user", NONE);
DO_TEST("net-virtio", NONE);
DO_TEST("net-virtio-device",
--
2.8.4
9:58 a.m.
New subject: [libvirt] [PATCH v2 04/14] qemuBuildInterfaceCommandLine: Move vhostuser handling a bit further
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
The idea is to have function that does some checking of the
arguments at its beginning and then have one big switch for all
the interface types it supports. Each one of them generating the
corresponding part of the command line.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 9 +++---
.../qemuxml2argv-net-vhostuser-fail.xml | 36 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 3 ++
3 files changed, 44 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
Similarly to patch 3, if a vhost-user entry contains 'filterref' or
'backend tap', then bulding will fail. Although queues is Ok.
BTW: I just realized cfg isn't even used in this function other than the
GetConfig call and Unref call.
ACK,
John
12:51 a.m.
New subject: [libvirt] [PATCH v2 04/14] qemuBuildInterfaceCommandLine: Move vhostuser handling a bit further
On 13.10.2016 22:58, John Ferlan wrote:
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
> The idea is to have function that does some checking of the
> arguments at its beginning and then have one big switch for all
> the interface types it supports. Each one of them generating the
> corresponding part of the command line.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_command.c | 9 +++---
> .../qemuxml2argv-net-vhostuser-fail.xml | 36 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 3 ++
> 3 files changed, 44 insertions(+), 4 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
>
Similarly to patch 3, if a vhost-user entry contains 'filterref' or
'backend tap', then bulding will fail. Although queues is Ok.
BTW: I just realized cfg isn't even used in this function other than the
GetConfig call and Unref call.
Yes, I've noticed that too and I'm fixing it somewhere in a later patch.
Michal
8:38 a.m.
New subject: [libvirt] [PATCH v2 05/14] qemuBuildInterfaceCommandLine: Move from if-else forest to switch
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5ca82ec..cc3b24f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7956,8 +7956,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
cfg = virQEMUDriverGetConfig(driver);
- if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
- actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) {
+ switch (actualType) {
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
tapfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = 1;
@@ -7971,7 +7972,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
if (qemuInterfaceBridgeConnect(def, driver, net,
tapfd, &tapfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
tapfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = 1;
@@ -7985,7 +7988,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
if (qemuInterfaceDirectConnect(def, driver, net,
tapfd, tapfdSize, vmop) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
tapfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = 1;
@@ -7997,17 +8002,32 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
memset(tapfd, -1, tapfdSize * sizeof(tapfd[0]));
if (qemuInterfaceEthernetConnect(def, driver, net,
- tapfd, tapfdSize) < 0)
+ tapfd, tapfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
/* NET_TYPE_HOSTDEV devices are really hostdev devices, so
* their commandlines are constructed with other hostdevs.
*/
ret = 0;
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
ret = qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ /* nada */
+ break;
}
/* For types whose implementations use a netdev on the host, add
--
2.8.4
9:58 a.m.
New subject: [libvirt] [PATCH v2 05/14] qemuBuildInterfaceCommandLine: Move from if-else forest to switch
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 34 +++++++++++++++++++++++++++-------
1 file changed, 27 insertions(+), 7 deletions(-)
ACK
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 06/14] qemuDomainAttachNetDevice: Move hostdev handling a bit further
The idea is to have function that does some checking at its
beginning and then have one big switch for all the interface
types it supports.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index bb8ea0d..95bbb35 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -946,20 +946,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
actualType = virDomainNetGetActualType(net);
- if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
- /* This is really a "smart hostdev", so it should be attached
- * as a hostdev (the hostdev code will reach over into the
- * netdev-specific code as appropriate), then also added to
- * the nets list (see cleanup:) if successful.
- *
- * qemuDomainAttachHostDevice uses a connection to resolve
- * a SCSI hostdev secret, which is not this case, so pass NULL.
- */
- ret = qemuDomainAttachHostDevice(NULL, driver, vm,
- virDomainNetGetActualHostdev(net));
- goto cleanup;
- }
-
/* Currently only TAP/macvtap devices supports multiqueue. */
if (net->driver.virtio.queues > 0 &&
!(actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
@@ -1037,6 +1023,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (qemuInterfaceOpenVhostNet(vm->def, net, priv->qemuCaps,
vhostfd, &vhostfdSize) < 0)
goto cleanup;
+ } else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ /* This is really a "smart hostdev", so it should be attached
+ * as a hostdev (the hostdev code will reach over into the
+ * netdev-specific code as appropriate), then also added to
+ * the nets list (see cleanup:) if successful.
+ *
+ * qemuDomainAttachHostDevice uses a connection to resolve
+ * a SCSI hostdev secret, which is not this case, so pass NULL.
+ */
+ ret = qemuDomainAttachHostDevice(NULL, driver, vm,
+ virDomainNetGetActualHostdev(net));
+ goto cleanup;
}
/* Set device online immediately */
--
2.8.4
9:58 a.m.
New subject: [libvirt] [PATCH v2 06/14] qemuDomainAttachNetDevice: Move hostdev handling a bit further
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
The idea is to have function that does some checking at its
beginning and then have one big switch for all the interface
types it supports.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
Similar to patch 3, we'll now fail if filterref and queues exist in the
to be attached device's XML, but it doesn't check the backend tap. Not a
problem with this patch per se, but perhaps something that could be put
on the virtual todo list (make the checks consistent between building
command line and hotplug and of course do so by adding some helper
function rather than duplicating the checks which are prone to issues)...
ACK for what's here though
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 07/14] qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug
Instead of blindly claim support for hot-plugging of every
interface type out there we should copy approach we have for
device types: white listing supported types and explicitly error
out on unsupported ones.
For instance, trying to hotplug vhostuser interface results in
nothing usable from guest currently. vhostuser typed interfaces
require additional work on our side.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 95bbb35..1b2a075 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -970,8 +970,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
return -1;
}
- if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
- actualType == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ switch (actualType) {
+ case VIR_DOMAIN_NET_TYPE_BRIDGE:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
tapfdSize = vhostfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = vhostfdSize = 1;
@@ -988,7 +989,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (qemuInterfaceOpenVhostNet(vm->def, net, priv->qemuCaps,
vhostfd, &vhostfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_DIRECT:
tapfdSize = vhostfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = vhostfdSize = 1;
@@ -1006,7 +1009,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (qemuInterfaceOpenVhostNet(vm->def, net, priv->qemuCaps,
vhostfd, &vhostfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_ETHERNET:
tapfdSize = vhostfdSize = net->driver.virtio.queues;
if (!tapfdSize)
tapfdSize = vhostfdSize = 1;
@@ -1017,13 +1022,15 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize);
if (qemuInterfaceEthernetConnect(vm->def, driver, net,
- tapfd, tapfdSize) < 0)
+ tapfd, tapfdSize) < 0)
goto cleanup;
iface_connected = true;
if (qemuInterfaceOpenVhostNet(vm->def, net, priv->qemuCaps,
vhostfd, &vhostfdSize) < 0)
goto cleanup;
- } else if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
/* This is really a "smart hostdev", so it should be attached
* as a hostdev (the hostdev code will reach over into the
* netdev-specific code as appropriate), then also added to
@@ -1035,6 +1042,20 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
ret = qemuDomainAttachHostDevice(NULL, driver, vm,
virDomainNetGetActualHostdev(net));
goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ case VIR_DOMAIN_NET_TYPE_SERVER:
+ case VIR_DOMAIN_NET_TYPE_CLIENT:
+ case VIR_DOMAIN_NET_TYPE_MCAST:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ case VIR_DOMAIN_NET_TYPE_UDP:
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("hotplug of interface type of %s is not implemented
yet"),
+ virDomainNetTypeToString(actualType));
+ goto cleanup;
}
/* Set device online immediately */
--
2.8.4
9:59 a.m.
New subject: [libvirt] [PATCH v2 07/14] qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
Instead of blindly claim support for hot-plugging of every
interface type out there we should copy approach we have for
device types: white listing supported types and explicitly error
out on unsupported ones.
For instance, trying to hotplug vhostuser interface results in
nothing usable from guest currently. vhostuser typed interfaces
require additional work on our side.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)
Should have just kept quiet in the v1 review about a switch ;-)
FWIW: If someone tries to add a vhostuser it will fail before the
unsupported message because the "if (net->driver.virtio.queues > 0 &&
..." doesn't have VHOSTUSER in the list like it does for command line.
I'm OK with adding that in here now or as a separate patch...
ACK for what's here.
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 08/14] qemuBuildHostNetStr: Explicitly enumerate net types
We tend to prevent using 'default' in switches. And it is for a
good reason - control may end up in paths we wouldn't want for
new values. In this specific case, if qemuBuildHostNetStr is
called over VIR_DOMAIN_NET_TYPE_VHOSTUSER it would produce
meaningless output. Fortunately, there no such call yet.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cc3b24f..4a67d80 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3711,9 +3711,21 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
case VIR_DOMAIN_NET_TYPE_USER:
- default:
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
virBufferAddLit(&buf, "user");
break;
+
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ /* Should have been handled earlier via PCI/USB hotplug code. */
+ virObjectUnref(cfg);
+ return NULL;
+
+ case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ /* Unsupported yet. */
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_LAST:
+ break;
}
if (vlan >= 0) {
--
2.8.4
9:59 a.m.
New subject: [libvirt] [PATCH v2 08/14] qemuBuildHostNetStr: Explicitly enumerate net types
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
We tend to prevent using 'default' in switches. And it is for
a
good reason - control may end up in paths we wouldn't want for
new values. In this specific case, if qemuBuildHostNetStr is
called over VIR_DOMAIN_NET_TYPE_VHOSTUSER it would produce
meaningless output. Fortunately, there no such call yet.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
Doesn't seem anything changed from the original patch (which is fine).
Yet another function that doesn't use/need the 'cfg' variable...
ACK to what's here, I'd support removing cfg as well
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 09/14] qemuBuildChrChardevStr: Introduce @nowait argument
This alone makes not much sense. But the aim is to reuse this
function in qemuBuildVhostuserCommandLine() where 'nowait' is not
supported for vhost-user devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4a67d80..71067ee 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4934,7 +4934,8 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
const virDomainDef *def,
const virDomainChrSourceDef *dev,
const char *alias,
- virQEMUCapsPtr qemuCaps)
+ virQEMUCapsPtr qemuCaps,
+ bool nowait)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
bool telnet;
@@ -5007,12 +5008,14 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
case VIR_DOMAIN_CHR_TYPE_TCP:
telnet = dev->data.tcp.protocol == VIR_DOMAIN_CHR_TCP_PROTOCOL_TELNET;
virBufferAsprintf(&buf,
- "socket,id=char%s,host=%s,port=%s%s%s",
+ "socket,id=char%s,host=%s,port=%s%s",
alias,
dev->data.tcp.host,
dev->data.tcp.service,
- telnet ? ",telnet" : "",
- dev->data.tcp.listen ? ",server,nowait" :
"");
+ telnet ? ",telnet" : "");
+
+ if (dev->data.tcp.listen)
+ virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
if (cfg->chardevTLS) {
char *objalias = NULL;
@@ -5034,7 +5037,7 @@ qemuBuildChrChardevStr(virLogManagerPtr logManager,
virBufferAsprintf(&buf, "socket,id=char%s,path=", alias);
virQEMUBuildBufferEscapeComma(&buf, dev->data.nix.path);
if (dev->data.nix.listen)
- virBufferAddLit(&buf, ",server,nowait");
+ virBufferAdd(&buf, nowait ? ",server,nowait" :
",server", -1);
break;
case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
@@ -5358,7 +5361,7 @@ qemuBuildMonitorCommandLine(virLogManagerPtr logManager,
if (!(chrdev = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
monitor_chr, "monitor",
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, chrdev);
@@ -5516,7 +5519,7 @@ qemuBuildRNGBackendChrdevStr(virLogManagerPtr logManager,
case VIR_DOMAIN_RNG_BACKEND_EGD:
if (!(*chr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
rng->source.chardev,
- rng->info.alias, qemuCaps)))
+ rng->info.alias, qemuCaps, true)))
return -1;
}
@@ -8375,7 +8378,7 @@ qemuBuildSmartcardCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&smartcard->data.passthru,
smartcard->info.alias,
- qemuCaps))) {
+ qemuCaps, true))) {
virBufferFreeAndReset(&opt);
return -1;
}
@@ -8464,7 +8467,7 @@ qemuBuildShmemBackendChrStr(virLogManagerPtr logManager,
devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&shmem->server.chr,
- shmem->info.alias, qemuCaps);
+ shmem->info.alias, qemuCaps, true);
return devstr;
}
@@ -8568,7 +8571,7 @@ qemuBuildSerialCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&serial->source,
serial->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8607,7 +8610,7 @@ qemuBuildParallelsCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
¶llel->source,
parallel->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8653,7 +8656,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&channel->source,
channel->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8676,7 +8679,7 @@ qemuBuildChannelsCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&channel->source,
channel->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8719,7 +8722,7 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&console->source,
console->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8733,7 +8736,7 @@ qemuBuildConsoleCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&console->source,
console->info.alias,
- qemuCaps)))
+ qemuCaps, true)))
return -1;
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, devstr);
@@ -8862,7 +8865,7 @@ qemuBuildRedirdevCommandLine(virLogManagerPtr logManager,
if (!(devstr = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
&redirdev->source.chr,
redirdev->info.alias,
- qemuCaps))) {
+ qemuCaps, true))) {
return -1;
}
--
2.8.4
9:59 a.m.
New subject: [libvirt] [PATCH v2 09/14] qemuBuildChrChardevStr: Introduce @nowait argument
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
This alone makes not much sense. But the aim is to reuse this
function in qemuBuildVhostuserCommandLine() where 'nowait' is not
supported for vhost-user devices.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 35 +++++++++++++++++++----------------
1 file changed, 19 insertions(+), 16 deletions(-)
ACK
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 10/14] qemuBuildVhostuserCommandLine: Reuse qemuBuildChrChardevStr
There's no need to reinvent the wheel here. We already have a
function to format virDomainChrSourceDefPtr. It's called
qemuBuildChrChardevStr(). Use that instead of some dummy
virBufferAsprintf().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 71067ee..246ffe0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7828,7 +7828,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
virQEMUCapsPtr qemuCaps,
unsigned int bootindex)
{
- virBuffer chardev_buf = VIR_BUFFER_INITIALIZER;
+ char *chardev = NULL;
virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
unsigned int queues = net->driver.virtio.queues;
char *nic = NULL;
@@ -7841,9 +7841,10 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
switch ((virDomainChrType) net->data.vhostuser->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
- virBufferAsprintf(&chardev_buf, "socket,id=char%s,path=%s%s",
- net->info.alias, net->data.vhostuser->data.nix.path,
- net->data.vhostuser->data.nix.listen ?
",server" : "");
+ if (!(chardev = qemuBuildChrChardevStr(NULL, NULL, NULL, def,
+ net->data.vhostuser,
+ net->info.alias, qemuCaps, false)))
+ goto error;
break;
case VIR_DOMAIN_CHR_TYPE_NULL:
@@ -7861,7 +7862,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
case VIR_DOMAIN_CHR_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("vhost-user type '%s' not supported"),
- virDomainChrTypeToString(net->data.vhostuser->type));
+ virDomainChrTypeToString(net->data.vhostuser->type));
goto error;
}
@@ -7879,7 +7880,8 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
}
virCommandAddArg(cmd, "-chardev");
- virCommandAddArgBuffer(cmd, &chardev_buf);
+ virCommandAddArg(cmd, chardev);
+ VIR_FREE(chardev);
virCommandAddArg(cmd, "-netdev");
virCommandAddArgBuffer(cmd, &netdev_buf);
@@ -7897,7 +7899,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
return 0;
error:
- virBufferFreeAndReset(&chardev_buf);
+ VIR_FREE(chardev);
virBufferFreeAndReset(&netdev_buf);
VIR_FREE(nic);
--
2.8.4
9:59 a.m.
New subject: [libvirt] [PATCH v2 10/14] qemuBuildVhostuserCommandLine: Reuse qemuBuildChrChardevStr
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
There's no need to reinvent the wheel here. We already have a
function to format virDomainChrSourceDefPtr. It's called
qemuBuildChrChardevStr(). Use that instead of some dummy
virBufferAsprintf().
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
If someone bisected to here, they'd have issues because patch 11 needs
to be merged here...
ACK if merged with patch 11
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 11/14] qemuBuildInterfaceCommandLine: Pass proper args
Instead of various NULL arguments, we can pass proper qemu driver
config, log manager, and virCommand.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 246ffe0..895bb23 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7822,12 +7822,15 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
}
static int
-qemuBuildVhostuserCommandLine(virCommandPtr cmd,
+qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
unsigned int bootindex)
{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
unsigned int queues = net->driver.virtio.queues;
@@ -7841,7 +7844,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
switch ((virDomainChrType) net->data.vhostuser->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
- if (!(chardev = qemuBuildChrChardevStr(NULL, NULL, NULL, def,
+ if (!(chardev = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
net->data.vhostuser,
net->info.alias, qemuCaps, false)))
goto error;
@@ -7895,10 +7898,12 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
virCommandAddArgList(cmd, "-device", nic, NULL);
VIR_FREE(nic);
+ virObjectUnref(cfg);
return 0;
error:
+ virObjectUnref(cfg);
VIR_FREE(chardev);
virBufferFreeAndReset(&netdev_buf);
VIR_FREE(nic);
@@ -7907,8 +7912,9 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
}
static int
-qemuBuildInterfaceCommandLine(virCommandPtr cmd,
- virQEMUDriverPtr driver,
+qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
@@ -7928,7 +7934,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
virDomainNetType actualType = virDomainNetGetActualType(net);
- virQEMUDriverConfigPtr cfg = NULL;
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7971,8 +7976,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
return -1;
}
- cfg = virQEMUDriverGetConfig(driver);
-
switch (actualType) {
case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_BRIDGE:
@@ -8032,7 +8035,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
break;
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
- ret = qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
+ ret = qemuBuildVhostuserCommandLine(driver, logManager, cmd, def,
+ net, qemuCaps, bootindex);
goto cleanup;
break;
@@ -8205,7 +8209,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
VIR_FREE(host);
VIR_FREE(tapfdName);
VIR_FREE(vhostfdName);
- virObjectUnref(cfg);
return ret;
}
@@ -8215,8 +8218,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
* API domainSetSecurityTapFDLabel that doesn't use the const format.
*/
static int
-qemuBuildNetCommandLine(virCommandPtr cmd,
- virQEMUDriverPtr driver,
+qemuBuildNetCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virNetDevVPortProfileOp vmop,
@@ -8254,7 +8258,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
else
vlan = i;
- if (qemuBuildInterfaceCommandLine(cmd, driver, def, net,
+ if (qemuBuildInterfaceCommandLine(driver, logManager, cmd, def, net,
qemuCaps, vlan, bootNet, vmop,
standalone, nnicindexes,
nicindexes) < 0)
@@ -9481,7 +9485,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildNetCommandLine(cmd, driver, def, qemuCaps, vmop, standalone,
+ if (qemuBuildNetCommandLine(driver, logManager, cmd, def,
+ qemuCaps, vmop, standalone,
nnicindexes, nicindexes, &bootHostdevNet) < 0)
goto error;
--
2.8.4
9:59 a.m.
New subject: [libvirt] [PATCH v2 11/14] qemuBuildInterfaceCommandLine: Pass proper args
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
Instead of various NULL arguments, we can pass proper qemu driver
config, log manager, and virCommand.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 31 ++++++++++++++++++-------------
1 file changed, 18 insertions(+), 13 deletions(-)
This needs to be merged with patch 10
see note below
ACK w/ the merged patches - let's avoid a round of ATTRIBUTE_NONNULL
John
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 246ffe0..895bb23 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7822,12 +7822,15 @@ qemuBuildGraphicsCommandLine(virQEMUDriverConfigPtr cfg,
}
static int
-qemuBuildVhostuserCommandLine(virCommandPtr cmd,
+qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
unsigned int bootindex)
{
+ virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
unsigned int queues = net->driver.virtio.queues;
@@ -7841,7 +7844,7 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
switch ((virDomainChrType) net->data.vhostuser->type) {
case VIR_DOMAIN_CHR_TYPE_UNIX:
- if (!(chardev = qemuBuildChrChardevStr(NULL, NULL, NULL, def,
+ if (!(chardev = qemuBuildChrChardevStr(logManager, cmd, cfg, def,
net->data.vhostuser,
net->info.alias, qemuCaps, false)))
goto error;
@@ -7895,10 +7898,12 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
virCommandAddArgList(cmd, "-device", nic, NULL);
VIR_FREE(nic);
+ virObjectUnref(cfg);
return 0;
error:
+ virObjectUnref(cfg);
VIR_FREE(chardev);
virBufferFreeAndReset(&netdev_buf);
VIR_FREE(nic);
@@ -7907,8 +7912,9 @@ qemuBuildVhostuserCommandLine(virCommandPtr cmd,
}
static int
-qemuBuildInterfaceCommandLine(virCommandPtr cmd,
- virQEMUDriverPtr driver,
+qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virDomainNetDefPtr net,
virQEMUCapsPtr qemuCaps,
@@ -7928,7 +7934,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
char **tapfdName = NULL;
char **vhostfdName = NULL;
virDomainNetType actualType = virDomainNetGetActualType(net);
- virQEMUDriverConfigPtr cfg = NULL;
NOTE: Ironically I had suggested removing cfg in patch 4. You can
either do it then or now, it doesn't matter!
virNetDevBandwidthPtr actualBandwidth;
size_t i;
@@ -7971,8 +7976,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
return -1;
}
- cfg = virQEMUDriverGetConfig(driver);
-
switch (actualType) {
case VIR_DOMAIN_NET_TYPE_NETWORK:
case VIR_DOMAIN_NET_TYPE_BRIDGE:
@@ -8032,7 +8035,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
break;
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
- ret = qemuBuildVhostuserCommandLine(cmd, def, net, qemuCaps, bootindex);
+ ret = qemuBuildVhostuserCommandLine(driver, logManager, cmd, def,
+ net, qemuCaps, bootindex);
goto cleanup;
break;
@@ -8205,7 +8209,6 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
VIR_FREE(host);
VIR_FREE(tapfdName);
VIR_FREE(vhostfdName);
- virObjectUnref(cfg);
return ret;
}
@@ -8215,8 +8218,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
* API domainSetSecurityTapFDLabel that doesn't use the const format.
*/
static int
-qemuBuildNetCommandLine(virCommandPtr cmd,
- virQEMUDriverPtr driver,
+qemuBuildNetCommandLine(virQEMUDriverPtr driver,
+ virLogManagerPtr logManager,
+ virCommandPtr cmd,
virDomainDefPtr def,
virQEMUCapsPtr qemuCaps,
virNetDevVPortProfileOp vmop,
@@ -8254,7 +8258,7 @@ qemuBuildNetCommandLine(virCommandPtr cmd,
else
vlan = i;
- if (qemuBuildInterfaceCommandLine(cmd, driver, def, net,
+ if (qemuBuildInterfaceCommandLine(driver, logManager, cmd, def, net,
qemuCaps, vlan, bootNet, vmop,
standalone, nnicindexes,
nicindexes) < 0)
@@ -9481,7 +9485,8 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0)
goto error;
- if (qemuBuildNetCommandLine(cmd, driver, def, qemuCaps, vmop, standalone,
+ if (qemuBuildNetCommandLine(driver, logManager, cmd, def,
+ qemuCaps, vmop, standalone,
nnicindexes, nicindexes, &bootHostdevNet) < 0)
goto error;
8:38 a.m.
New subject: [libvirt] [PATCH v2 12/14] qemuBuildVhostuserCommandLine: Unify -netdev creation
Currently, what we do for vhost-user network is generate the
following part of command line:
-netdev type=vhost-user,id=hostnet0,chardev=charnet0
There's no need for 'type=' it is the default. Drop it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args | 6 +++---
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 895bb23..e8d4d8d 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7869,7 +7869,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
goto error;
}
- virBufferAsprintf(&netdev_buf,
"type=vhost-user,id=host%s,chardev=char%s",
+ virBufferAsprintf(&netdev_buf, "vhost-user,id=host%s,chardev=char%s",
net->info.alias, net->info.alias);
if (queues > 1) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
index bab15ad..4360e5e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
@@ -20,17 +20,17 @@ QEMU_AUDIO_DRV=none \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-chardev socket,id=charnet0,path=/tmp/vhost0.sock,server \
--netdev type=vhost-user,id=hostnet0,chardev=charnet0 \
+-netdev vhost-user,id=hostnet0,chardev=charnet0 \
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,\
addr=0x3 \
-chardev socket,id=charnet1,path=/tmp/vhost1.sock \
--netdev type=vhost-user,id=hostnet1,chardev=charnet1 \
+-netdev vhost-user,id=hostnet1,chardev=charnet1 \
-device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\
addr=0x4 \
-netdev socket,listen=:2015,id=hostnet2 \
-device rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,\
addr=0x5 \
-chardev socket,id=charnet3,path=/tmp/vhost2.sock \
--netdev type=vhost-user,id=hostnet3,chardev=charnet3,queues=4 \
+-netdev vhost-user,id=hostnet3,chardev=charnet3,queues=4 \
-device virtio-net-pci,mq=on,vectors=10,netdev=hostnet3,id=net3,\
mac=52:54:00:ee:96:6d,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
index ce8d669..47c1d84 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
@@ -20,11 +20,11 @@ QEMU_AUDIO_DRV=none \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-chardev socket,id=charnet0,path=/tmp/vhost0.sock,server \
--netdev type=vhost-user,id=hostnet0,chardev=charnet0 \
+-netdev vhost-user,id=hostnet0,chardev=charnet0 \
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,\
addr=0x3 \
-chardev socket,id=charnet1,path=/tmp/vhost1.sock \
--netdev type=vhost-user,id=hostnet1,chardev=charnet1 \
+-netdev vhost-user,id=hostnet1,chardev=charnet1 \
-device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\
addr=0x4 \
-netdev socket,listen=:2015,id=hostnet2 \
--
2.8.4
10 a.m.
New subject: [libvirt] [PATCH v2 12/14] qemuBuildVhostuserCommandLine: Unify -netdev creation
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
Currently, what we do for vhost-user network is generate the
following part of command line:
-netdev type=vhost-user,id=hostnet0,chardev=charnet0
There's no need for 'type=' it is the default. Drop it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args | 6 +++---
tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
Still an ACK,
John
8:38 a.m.
New subject: [libvirt] [PATCH v2 13/14] qemuBuildHostNetStr: Support VIR_DOMAIN_NET_TYPE_VHOSTUSER
https://bugzilla.redhat.com/show_bug.cgi?id=1366505
So far, this function lacked support for
VIR_DOMAIN_NET_TYPE_VHOSTUSER leaving callers to hack around the
problem by constructing the command line on their own. This is
not ideal as it blocks hot plug support.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 38 +++++++++++++---------
.../qemuxml2argv-net-vhostuser-multiq.args | 6 ++--
.../qemuxml2argv-net-vhostuser.args | 4 +--
3 files changed, 28 insertions(+), 20 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e8d4d8d..95e40de 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3721,7 +3721,13 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
return NULL;
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
- /* Unsupported yet. */
+ virBufferAsprintf(&buf, "vhost-user%cchardev=char%s",
+ type_sep,
+ net->info.alias);
+ type_sep = ',';
+ if (net->driver.virtio.queues > 1)
+ virBufferAsprintf(&buf, ",queues=%u",
+ net->driver.virtio.queues);
break;
case VIR_DOMAIN_NET_TYPE_LAST:
@@ -7832,7 +7838,7 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
{
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
char *chardev = NULL;
- virBuffer netdev_buf = VIR_BUFFER_INITIALIZER;
+ char *netdev = NULL;
unsigned int queues = net->driver.virtio.queues;
char *nic = NULL;
@@ -7869,25 +7875,27 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
goto error;
}
- virBufferAsprintf(&netdev_buf, "vhost-user,id=host%s,chardev=char%s",
- net->info.alias, net->info.alias);
-
- if (queues > 1) {
- if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("multi-queue is not supported for vhost-user "
- "with this QEMU binary"));
- goto error;
- }
- virBufferAsprintf(&netdev_buf, ",queues=%u", queues);
+ if (queues > 1 &&
+ !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VHOSTUSER_MULTIQUEUE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("multi-queue is not supported for vhost-user "
+ "with this QEMU binary"));
+ goto error;
}
+ if (!(netdev = qemuBuildHostNetStr(net, driver,
+ ',', -1,
+ NULL, 0, NULL, 0)))
+ goto error;
+
+
virCommandAddArg(cmd, "-chardev");
virCommandAddArg(cmd, chardev);
VIR_FREE(chardev);
virCommandAddArg(cmd, "-netdev");
- virCommandAddArgBuffer(cmd, &netdev_buf);
+ virCommandAddArg(cmd, netdev);
+ VIR_FREE(netdev);
if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex,
queues, qemuCaps))) {
@@ -7904,8 +7912,8 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver,
error:
virObjectUnref(cfg);
+ VIR_FREE(netdev);
VIR_FREE(chardev);
- virBufferFreeAndReset(&netdev_buf);
VIR_FREE(nic);
return -1;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
index 4360e5e..59929c1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-multiq.args
@@ -20,17 +20,17 @@ QEMU_AUDIO_DRV=none \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-chardev socket,id=charnet0,path=/tmp/vhost0.sock,server \
--netdev vhost-user,id=hostnet0,chardev=charnet0 \
+-netdev vhost-user,chardev=charnet0,id=hostnet0 \
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,\
addr=0x3 \
-chardev socket,id=charnet1,path=/tmp/vhost1.sock \
--netdev vhost-user,id=hostnet1,chardev=charnet1 \
+-netdev vhost-user,chardev=charnet1,id=hostnet1 \
-device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\
addr=0x4 \
-netdev socket,listen=:2015,id=hostnet2 \
-device rtl8139,netdev=hostnet2,id=net2,mac=52:54:00:95:db:c0,bus=pci.0,\
addr=0x5 \
-chardev socket,id=charnet3,path=/tmp/vhost2.sock \
--netdev vhost-user,id=hostnet3,chardev=charnet3,queues=4 \
+-netdev vhost-user,chardev=charnet3,queues=4,id=hostnet3 \
-device virtio-net-pci,mq=on,vectors=10,netdev=hostnet3,id=net3,\
mac=52:54:00:ee:96:6d,bus=pci.0,addr=0x6
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
index 47c1d84..e15d735 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
@@ -20,11 +20,11 @@ QEMU_AUDIO_DRV=none \
-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-ide0-0-0 \
-device ide-drive,bus=ide.0,unit=0,drive=drive-ide0-0-0,id=ide0-0-0 \
-chardev socket,id=charnet0,path=/tmp/vhost0.sock,server \
--netdev vhost-user,id=hostnet0,chardev=charnet0 \
+-netdev vhost-user,chardev=charnet0,id=hostnet0 \
-device virtio-net-pci,netdev=hostnet0,id=net0,mac=52:54:00:ee:96:6b,bus=pci.0,\
addr=0x3 \
-chardev socket,id=charnet1,path=/tmp/vhost1.sock \
--netdev vhost-user,id=hostnet1,chardev=charnet1 \
+-netdev vhost-user,chardev=charnet1,id=hostnet1 \
-device virtio-net-pci,netdev=hostnet1,id=net1,mac=52:54:00:ee:96:6c,bus=pci.0,\
addr=0x4 \
-netdev socket,listen=:2015,id=hostnet2 \
--
2.8.4
10 a.m.
New subject: [libvirt] [PATCH v2 13/14] qemuBuildHostNetStr: Support VIR_DOMAIN_NET_TYPE_VHOSTUSER
On 10/06/2016 09:38 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1366505
So far, this function lacked support for
VIR_DOMAIN_NET_TYPE_VHOSTUSER leaving callers to hack around the
problem by constructing the command line on their own. This is
not ideal as it blocks hot plug support.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_command.c | 38 +++++++++++++---------
.../qemuxml2argv-net-vhostuser-multiq.args | 6 ++--
.../qemuxml2argv-net-vhostuser.args | 4 +--
3 files changed, 28 insertions(+), 20 deletions(-)
ACK still applies,
John
8:39 a.m.
New subject: [libvirt] [PATCH v2 14/14] qemu_hotplug: Support interface type of vhost-user hotplug
https://bugzilla.redhat.com/show_bug.cgi?id=1366108
There are couple of things that needs to be done in order to
allow vhost-user hotplug. Firstly, vhost-user requires a chardev
which is connected to vhost-user bridge and through which qemu
communicates with the bridge (no acutal guest traffic is sent
through there, just some metadata). In order to generate proper
chardev alias, we must assign device alias way sooner.
Then, because we are plugging the chardev first, we need to do
the proper undo if something fails - that is remove netdev too.
We don't want anything to be left over in case attach fails at
some point.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 71 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1b2a075..14af4e1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -932,6 +932,10 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainCCWAddressSetPtr ccwaddrs = NULL;
size_t i;
+ char *charDevAlias = NULL;
+ bool charDevPlugged = false;
+ bool netdevPlugged = false;
+ bool hostPlugged = false;
/* preallocate new slot for device */
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
@@ -970,6 +974,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
return -1;
}
+ if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
+ goto cleanup;
+
switch (actualType) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
@@ -1044,8 +1051,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
break;
- case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ if (!qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Netdev support unavailable"));
+ goto cleanup;
+ }
+
+ if (virAsprintf(&charDevAlias, "char%s", net->info.alias) <
0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -1081,9 +1098,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
}
- if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
- goto cleanup;
-
if (qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
@@ -1143,23 +1157,36 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
}
qemuDomainObjEnterMonitor(driver, vm);
+
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, net->data.vhostuser)
< 0) {
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ virDomainAuditNet(vm, NULL, net, "attach", false);
+ goto cleanup;
+ }
+ charDevPlugged = true;
+ }
+
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
if (qemuMonitorAddNetdev(priv->mon, netstr,
tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize) < 0) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
- goto cleanup;
+ goto try_remove;
}
+ netdevPlugged = true;
} else {
if (qemuMonitorAddHostNetwork(priv->mon, netstr,
tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize) < 0) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
- goto cleanup;
+ goto try_remove;
}
+ hostPlugged = true;
}
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -1262,6 +1289,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
}
VIR_FREE(vhostfd);
VIR_FREE(vhostfdName);
+ VIR_FREE(charDevAlias);
virObjectUnref(cfg);
virDomainCCWAddressSetFree(ccwaddrs);
@@ -1277,7 +1305,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (virAsprintf(&netdev_name, "host%s", net->info.alias)
< 0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
+ if (charDevPlugged &&
+ qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0)
+ VIR_WARN("Failed to remove associated chardev %s",
charDevAlias);
+ if (netdevPlugged &&
+ qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
VIR_WARN("Failed to remove network backend for netdev %s",
netdev_name);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
@@ -1290,7 +1322,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (virAsprintf(&hostnet_name, "host%s", net->info.alias) <
0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
+ if (hostPlugged &&
+ qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
VIR_WARN("Failed to remove network backend for vlan %d, net %s",
vlan, hostnet_name);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
@@ -3320,10 +3353,12 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
virNetDevVPortProfilePtr vport;
virObjectEventPtr event;
char *hostnet_name = NULL;
+ char *charDevAlias = NULL;
size_t i;
int ret = -1;
+ int actualType = virDomainNetGetActualType(net);
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* this function handles all hostdev and netdev cleanup */
ret = qemuDomainRemoveHostDevice(driver, vm,
virDomainNetGetActualHostdev(net));
@@ -3333,9 +3368,11 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
VIR_DEBUG("Removing network interface %s from domain %p %s",
net->info.alias, vm, vm->def->name);
- if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0)
+ if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0 ||
+ virAsprintf(&charDevAlias, "char%s", net->info.alias) < 0)
goto cleanup;
+
qemuDomainObjEnterMonitor(driver, vm);
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
@@ -3358,6 +3395,17 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
goto cleanup;
}
}
+
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ /* vhostuser has a chardev too */
+ if (qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) {
+ /* well, this is a messy situation. Guest visible PCI device has
+ * been removed, netdev too but chardev not. The best seems to be
+ * to just ignore the error and carry on.
+ */
+ }
+ }
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -3382,7 +3430,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
&net->mac));
}
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
ignore_value(virNetDevMacVLanDeleteWithVPortProfile(
net->ifname, &net->mac,
virDomainNetGetActualDirectDev(net),
@@ -3408,6 +3456,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
cleanup:
virObjectUnref(cfg);
+ VIR_FREE(charDevAlias);
VIR_FREE(hostnet_name);
return ret;
}
--
2.8.4
10 a.m.
New subject: [libvirt] [PATCH v2 14/14] qemu_hotplug: Support interface type of vhost-user hotplug
On 10/06/2016 09:39 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1366108
There are couple of things that needs to be done in order to
s/needs/need/
allow vhost-user hotplug. Firstly, vhost-user requires a chardev
which is connected to vhost-user bridge and through which qemu
communicates with the bridge (no acutal guest traffic is sent
s/acutal/actual/
through there, just some metadata). In order to generate proper
chardev alias, we must assign device alias way sooner.
Then, because we are plugging the chardev first, we need to do
the proper undo if something fails - that is remove netdev too.
We don't want anything to be left over in case attach fails at
some point.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 71 +++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 60 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 1b2a075..14af4e1 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -932,6 +932,10 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
virDomainCCWAddressSetPtr ccwaddrs = NULL;
size_t i;
+ char *charDevAlias = NULL;
+ bool charDevPlugged = false;
+ bool netdevPlugged = false;
+ bool hostPlugged = false;
/* preallocate new slot for device */
if (VIR_REALLOC_N(vm->def->nets, vm->def->nnets + 1) < 0)
@@ -970,6 +974,9 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
return -1;
}
+ if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
+ goto cleanup;
+
switch (actualType) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
case VIR_DOMAIN_NET_TYPE_NETWORK:
@@ -1044,8 +1051,18 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
break;
- case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
+ if (!qemuDomainSupportsNetdev(vm->def, priv->qemuCaps, net)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("Netdev support unavailable"));
+ goto cleanup;
+ }
+
+ if (virAsprintf(&charDevAlias, "char%s", net->info.alias) <
0)
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_SERVER:
case VIR_DOMAIN_NET_TYPE_CLIENT:
case VIR_DOMAIN_NET_TYPE_MCAST:
@@ -1081,9 +1098,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
goto cleanup;
}
- if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0)
- goto cleanup;
-
if (qemuDomainMachineIsS390CCW(vm->def) &&
virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_CCW)) {
net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW;
@@ -1143,23 +1157,36 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
}
qemuDomainObjEnterMonitor(driver, vm);
+
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ if (qemuMonitorAttachCharDev(priv->mon, charDevAlias, net->data.vhostuser)
< 0) {
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+ virDomainAuditNet(vm, NULL, net, "attach", false);
+ goto cleanup;
+ }
+ charDevPlugged = true;
+ }
+
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
if (qemuMonitorAddNetdev(priv->mon, netstr,
tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize) < 0) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
- goto cleanup;
+ goto try_remove;
}
+ netdevPlugged = true;
} else {
if (qemuMonitorAddHostNetwork(priv->mon, netstr,
tapfd, tapfdName, tapfdSize,
vhostfd, vhostfdName, vhostfdSize) < 0) {
ignore_value(qemuDomainObjExitMonitor(driver, vm));
virDomainAuditNet(vm, NULL, net, "attach", false);
- goto cleanup;
+ goto try_remove;
}
+ hostPlugged = true;
}
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -1262,6 +1289,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
}
VIR_FREE(vhostfd);
VIR_FREE(vhostfdName);
+ VIR_FREE(charDevAlias);
virObjectUnref(cfg);
virDomainCCWAddressSetFree(ccwaddrs);
@@ -1277,7 +1305,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (virAsprintf(&netdev_name, "host%s", net->info.alias)
< 0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
+ if (charDevPlugged &&
+ qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0)
Adding this wasn't within the NETDEV specific code, so removing it
shouldn't be either...
+ VIR_WARN("Failed to remove associated chardev
%s", charDevAlias);
+ if (netdevPlugged &&
+ qemuMonitorRemoveNetdev(priv->mon, netdev_name) < 0)
VIR_WARN("Failed to remove network backend for netdev %s",
netdev_name);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
@@ -1290,7 +1322,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver,
if (virAsprintf(&hostnet_name, "host%s", net->info.alias) <
0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
- if (qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
+ if (hostPlugged &&
+ qemuMonitorRemoveHostNetwork(priv->mon, vlan, hostnet_name) < 0)
VIR_WARN("Failed to remove network backend for vlan %d, net %s",
vlan, hostnet_name);
ignore_value(qemuDomainObjExitMonitor(driver, vm));
BTW: I know it's existing, but I think the whole think can be
simplified... The only way to have any of those booleans defined to
true is if something was success, so why not (starting at vlan < 0):
char *alias = NULL; <== this would be at the top... and the
VIR_FREE(alias) would be in cleanup:
if (virAsprintf(&alias, "host%s", net->info.alias) < 0)
goto cleanup;
qemuDomainObjEnterMonitor(driver, vm);
if (netdevPlugged && ...)
if (hostPlugged && ...)
-> The error messages could be made generic - add a vlan=%d - who
really cares if it shows up -1, then at least you know which path it was).
if (charDevPlugged && ...)
ignore_value(qemuDomainObjExitMonitor(driver, vm));
goto cleanup;
and yes the move of chardevPlugged was intentional... it was added, so
remove it last.
and the VIR_WARN("Unable to remove network backend"); can go away...
@@ -3320,10 +3353,12 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr
driver,
virNetDevVPortProfilePtr vport;
virObjectEventPtr event;
char *hostnet_name = NULL;
+ char *charDevAlias = NULL;
size_t i;
int ret = -1;
+ int actualType = virDomainNetGetActualType(net);
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
/* this function handles all hostdev and netdev cleanup */
ret = qemuDomainRemoveHostDevice(driver, vm,
virDomainNetGetActualHostdev(net));
@@ -3333,9 +3368,11 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
VIR_DEBUG("Removing network interface %s from domain %p %s",
net->info.alias, vm, vm->def->name);
- if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0)
+ if (virAsprintf(&hostnet_name, "host%s", net->info.alias) < 0
||
+ virAsprintf(&charDevAlias, "char%s", net->info.alias) < 0)
goto cleanup;
+
qemuDomainObjEnterMonitor(driver, vm);
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_NETDEV)) {
if (qemuMonitorRemoveNetdev(priv->mon, hostnet_name) < 0) {
@@ -3358,6 +3395,17 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
goto cleanup;
}
}
+
+ if (actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ /* vhostuser has a chardev too */
+ if (qemuMonitorDetachCharDev(priv->mon, charDevAlias) < 0) {
+ /* well, this is a messy situation. Guest visible PCI device has
+ * been removed, netdev too but chardev not. The best seems to be
+ * to just ignore the error and carry on.
+ */
I'd say - keep the comment, but just make this an ignore_value();
encased call (qemuDomainRemoveDiskDevice does so for objAlias and encAlias)
ACK - although I would prefer that the try_remove code in
qemuDomainAttachNetDevice is cleaned up, it's not "contingent" on the
ACK. The changes to the commit message and just going with the
ignore_value() would be.
John
+ }
+ }
+
if (qemuDomainObjExitMonitor(driver, vm) < 0)
goto cleanup;
@@ -3382,7 +3430,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
&net->mac));
}
- if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) {
ignore_value(virNetDevMacVLanDeleteWithVPortProfile(
net->ifname, &net->mac,
virDomainNetGetActualDirectDev(net),
@@ -3408,6 +3456,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver,
cleanup:
virObjectUnref(cfg);
+ VIR_FREE(charDevAlias);
VIR_FREE(hostnet_name);
return ret;
}
12:50 a.m.
On 06.10.2016 21:38, Michal Privoznik wrote:
v2 of:
https://www.redhat.com/archives/libvir-list/2016-August/msg00806.html
The first patches are mostly code movement and cleanups.
diff to v1:
- Hopefully all John's review suggestions are worked in now
Michal Privoznik (14):
virDomainNetDefParseXML: Realign
virDomainNetGetActualType: Return type is virDomainNetType
qemuBuildInterfaceCommandLine: Move hostdev handling a bit further
qemuBuildInterfaceCommandLine: Move vhostuser handling a bit further
qemuBuildInterfaceCommandLine: Move from if-else forest to switch
qemuDomainAttachNetDevice: Move hostdev handling a bit further
qemuDomainAttachNetDevice: Explicitly list allowed types for hotplug
qemuBuildHostNetStr: Explicitly enumerate net types
qemuBuildChrChardevStr: Introduce @nowait argument
qemuBuildVhostuserCommandLine: Reuse qemuBuildChrChardevStr
qemuBuildInterfaceCommandLine: Pass proper args
qemuBuildVhostuserCommandLine: Unify -netdev creation
qemuBuildHostNetStr: Support VIR_DOMAIN_NET_TYPE_VHOSTUSER
qemu_hotplug: Support interface type of vhost-user hotplug
src/bhyve/bhyve_command.c | 2 +-
src/bhyve/bhyve_process.c | 2 +-
src/conf/domain_conf.c | 16 +-
src/conf/domain_conf.h | 2 +-
src/libxl/libxl_domain.c | 2 +-
src/libxl/libxl_driver.c | 2 +-
src/lxc/lxc_driver.c | 9 +-
src/qemu/qemu_command.c | 182 +++++++++++++--------
src/qemu/qemu_hotplug.c | 130 +++++++++++----
src/qemu/qemu_process.c | 13 +-
.../qemuxml2argv-net-hostdev-fail.xml | 39 +++++
.../qemuxml2argv-net-vhostuser-fail.xml | 36 ++++
.../qemuxml2argv-net-vhostuser-multiq.args | 6 +-
.../qemuxml2argv-net-vhostuser.args | 4 +-
tests/qemuxml2argvtest.c | 7 +
15 files changed, 334 insertions(+), 118 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-hostdev-fail.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser-fail.xml
I've merged patches 10 a 11 as requested, and pushed these.
Thank you John for the review!
Michal
2995
days inactive
3003
days old
31 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik