9:32 a.m.
This is not a blurb.
Ján Tomko (4):
qemuBuildHostNetStr: use type_sep earlier
qemuBuildHostNetStr: do not start options with a comma
qemuBuildHostNetStr: move stray VIR_DOMAIN_NET_TYPE_INTERNAL
qemuBuildHostNetStr: remove dead code
src/qemu/qemu_command.c | 49 +++++++++++++++++++++----------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
--
2.7.3
9:32 a.m.
New subject: [libvirt] [PATCH 1/4] qemuBuildHostNetStr: use type_sep earlier
When hotplugging networks with ancient QEMUs not supporting
QEMU_CAPS_NETDEV, we use space instead of a comma as the separator
between the network type and other options.
Except for "user", all the network types pass other options
and use up the first separator by the time we get to the section
that adds the alias (or vlan for QEMUs without CAPS_NETDEV).
Since the alias/vlan is mandatory, convert all preceding code to add
the separator at the end, removing the need to rewrite type_sep for
all types but NET_TYPE_USER.
---
src/qemu/qemu_command.c | 28 +++++++++++-----------------
1 file changed, 11 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 21fd85c..eba7ba9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3674,7 +3674,7 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
/* for one tapfd 'fd=' shall be used,
* for more than one 'fds=' is the right choice */
if (tapfdSize == 1) {
- virBufferAsprintf(&buf, "fd=%s", tapfd[0]);
+ virBufferAsprintf(&buf, "fd=%s,", tapfd[0]);
} else {
virBufferAddLit(&buf, "fds=");
for (i = 0; i < tapfdSize; i++) {
@@ -3682,49 +3682,45 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
virBufferAddChar(&buf, ':');
virBufferAdd(&buf, tapfd[i], -1);
}
+ virBufferAddChar(&buf, ',');
}
- type_sep = ',';
is_tap = true;
break;
case VIR_DOMAIN_NET_TYPE_CLIENT:
- virBufferAsprintf(&buf, "socket%cconnect=%s:%d",
+ virBufferAsprintf(&buf, "socket%cconnect=%s:%d,",
type_sep,
net->data.socket.address,
net->data.socket.port);
- type_sep = ',';
break;
case VIR_DOMAIN_NET_TYPE_SERVER:
- virBufferAsprintf(&buf, "socket%clisten=%s:%d",
+ virBufferAsprintf(&buf, "socket%clisten=%s:%d,",
type_sep,
net->data.socket.address ? net->data.socket.address
: "",
net->data.socket.port);
- type_sep = ',';
break;
case VIR_DOMAIN_NET_TYPE_MCAST:
- virBufferAsprintf(&buf, "socket%cmcast=%s:%d",
+ virBufferAsprintf(&buf, "socket%cmcast=%s:%d,",
type_sep,
net->data.socket.address,
net->data.socket.port);
- type_sep = ',';
break;
case VIR_DOMAIN_NET_TYPE_UDP:
- virBufferAsprintf(&buf, "socket%cudp=%s:%d,localaddr=%s:%d",
+ virBufferAsprintf(&buf, "socket%cudp=%s:%d,localaddr=%s:%d,",
type_sep,
net->data.socket.address,
net->data.socket.port,
net->data.socket.localaddr,
net->data.socket.localport);
- type_sep = ',';
break;
case VIR_DOMAIN_NET_TYPE_USER:
case VIR_DOMAIN_NET_TYPE_INTERNAL:
- virBufferAddLit(&buf, "user");
+ virBufferAsprintf(&buf, "user%c", type_sep);
break;
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
@@ -3733,12 +3729,11 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
return NULL;
case VIR_DOMAIN_NET_TYPE_VHOSTUSER:
- virBufferAsprintf(&buf, "vhost-user%cchardev=char%s",
+ virBufferAsprintf(&buf, "vhost-user%cchardev=char%s,",
type_sep,
net->info.alias);
- type_sep = ',';
if (net->driver.virtio.queues > 1)
- virBufferAsprintf(&buf, ",queues=%u",
+ virBufferAsprintf(&buf, "queues=%u,",
net->driver.virtio.queues);
break;
@@ -3747,13 +3742,12 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
if (vlan >= 0) {
- virBufferAsprintf(&buf, "%cvlan=%d", type_sep, vlan);
+ virBufferAsprintf(&buf, "vlan=%d", vlan);
if (net->info.alias)
virBufferAsprintf(&buf, ",name=host%s",
net->info.alias);
} else {
- virBufferAsprintf(&buf, "%cid=host%s",
- type_sep, net->info.alias);
+ virBufferAsprintf(&buf, "id=host%s", net->info.alias);
}
if (is_tap) {
--
2.7.3
7:16 a.m.
New subject: [libvirt] [PATCH 1/4] qemuBuildHostNetStr: use type_sep earlier
On Fri, Oct 14, 2016 at 04:32:15PM +0200, Ján Tomko wrote:
When hotplugging networks with ancient QEMUs not supporting
QEMU_CAPS_NETDEV, we use space instead of a comma as the separator
between the network type and other options.
Except for "user", all the network types pass other options
and use up the first separator by the time we get to the section
that adds the alias (or vlan for QEMUs without CAPS_NETDEV).
Since the alias/vlan is mandatory, convert all preceding code to add
the separator at the end, removing the need to rewrite type_sep for
all types but NET_TYPE_USER.
---
ACK
Pavel
9:32 a.m.
New subject: [libvirt] [PATCH 2/4] qemuBuildHostNetStr: do not start options with a comma
Put the comma at the end and trim it later for consistency.
---
src/qemu/qemu_command.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index eba7ba9..80ebe51 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3742,19 +3742,18 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
}
if (vlan >= 0) {
- virBufferAsprintf(&buf, "vlan=%d", vlan);
+ virBufferAsprintf(&buf, "vlan=%d,", vlan);
if (net->info.alias)
- virBufferAsprintf(&buf, ",name=host%s",
- net->info.alias);
+ virBufferAsprintf(&buf, "name=host%s,", net->info.alias);
} else {
- virBufferAsprintf(&buf, "id=host%s", net->info.alias);
+ virBufferAsprintf(&buf, "id=host%s,", net->info.alias);
}
if (is_tap) {
if (vhostfdSize) {
- virBufferAddLit(&buf, ",vhost=on,");
+ virBufferAddLit(&buf, "vhost=on,");
if (vhostfdSize == 1) {
- virBufferAsprintf(&buf, "vhostfd=%s", vhostfd[0]);
+ virBufferAsprintf(&buf, "vhostfd=%s,", vhostfd[0]);
} else {
virBufferAddLit(&buf, "vhostfds=");
for (i = 0; i < vhostfdSize; i++) {
@@ -3762,14 +3761,16 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
virBufferAddChar(&buf, ':');
virBufferAdd(&buf, vhostfd[i], -1);
}
+ virBufferAddChar(&buf, ',');
}
}
if (net->tune.sndbuf_specified)
- virBufferAsprintf(&buf, ",sndbuf=%lu", net->tune.sndbuf);
+ virBufferAsprintf(&buf, "sndbuf=%lu,", net->tune.sndbuf);
}
virObjectUnref(cfg);
+ virBufferTrim(&buf, ",", -1);
if (virBufferCheckError(&buf) < 0)
return NULL;
--
2.7.3
9:32 a.m.
New subject: [libvirt] [PATCH 3/4] qemuBuildHostNetStr: move stray VIR_DOMAIN_NET_TYPE_INTERNAL
This network type is only used by the vbox driver and it does not
make sense to group it with VIR_DOMAIN_NET_TYPE_USER.
Introduced by commit 1dcbef8.
---
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 80ebe51..19ee652 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3719,7 +3719,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
case VIR_DOMAIN_NET_TYPE_USER:
- case VIR_DOMAIN_NET_TYPE_INTERNAL:
virBufferAsprintf(&buf, "user%c", type_sep);
break;
@@ -3737,6 +3736,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
net->driver.virtio.queues);
break;
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ /* Not supported by QEMU driver */
case VIR_DOMAIN_NET_TYPE_LAST:
break;
}
--
2.7.3
7:33 a.m.
New subject: [libvirt] [PATCH 3/4] qemuBuildHostNetStr: move stray VIR_DOMAIN_NET_TYPE_INTERNAL
On Fri, Oct 14, 2016 at 04:32:17PM +0200, Ján Tomko wrote:
This network type is only used by the vbox driver and it does not
make sense to group it with VIR_DOMAIN_NET_TYPE_USER.
Introduced by commit 1dcbef8.
---
src/qemu/qemu_command.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 80ebe51..19ee652 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3719,7 +3719,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
break;
case VIR_DOMAIN_NET_TYPE_USER:
- case VIR_DOMAIN_NET_TYPE_INTERNAL:
virBufferAsprintf(&buf, "user%c", type_sep);
break;
@@ -3737,6 +3736,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
net->driver.virtio.queues);
break;
+ case VIR_DOMAIN_NET_TYPE_INTERNAL:
+ /* Not supported by QEMU driver */
case VIR_DOMAIN_NET_TYPE_LAST:
break;
}
This one is tricky. The net type *internal* was introduced for vbox driver,
but there was no check in other drivers to refuse this type. Before commit
1dcbef8 this code handled all not listed cases the same way as net type *user*
and that goes back before the *internal* type was introduced. So basically for
qemu driver we've handled *internal* as *user* since it was introduced in
libvirt.
Personaly I don't like that fact and IMO we can add a code to
${driver}DomainDefValidate() to refuse that type except vbox driver.
The argument to do it is that the net type *internal* is not documented
so there is a chance that it's not used at all for other drivers except vbox.
This patch resolves in this error while starting a guest:
error: internal error: process exited while connecting to monitor:
2016-10-21T12:28:46.841029Z qemu-system-x86_64: -netdev id=hostnet1: Parameter
'type' is missing
Pavel
9:32 a.m.
New subject: [libvirt] [PATCH 4/4] qemuBuildHostNetStr: remove dead code
This function is never called for VIR_DOMAIN_NET_TYPE_HOSTDEV,
and the dead code comment agrees.
Introduced by commit 1dcbef8a.
---
src/qemu/qemu_command.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 19ee652..adeb906 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3722,11 +3722,6 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
virBufferAsprintf(&buf, "user%c", type_sep);
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:
virBufferAsprintf(&buf, "vhost-user%cchardev=char%s,",
type_sep,
@@ -3736,6 +3731,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,
net->driver.virtio.queues);
break;
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ /* Should have been handled earlier via PCI/USB hotplug code. */
case VIR_DOMAIN_NET_TYPE_INTERNAL:
/* Not supported by QEMU driver */
case VIR_DOMAIN_NET_TYPE_LAST:
--
2.7.3
2954
days inactive
2961
days old
8 comments
2 participants
participants (2)
-
Ján Tomko -
Pavel Hrdina