4:25 p.m.
Patch 1 is a V2 of
http://www.redhat.com/archives/libvir-list/2014-June/msg00382.html
Patch 2 is a result of Laine's comments to V1. It ensures <script>
is only allowed with interface types ethernet and bridge.
Jim Fehlig (2):
libxl: support interface type=network
libxl: limit support for specifying an interface script
src/libxl/libxl_conf.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 5 deletions(-)
--
1.8.4.5
4:25 p.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: support interface type=network
Add support for <interface type='network'> in the libxl driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
V2:
Support both libvirt's traditional managed networks and libvirt
networks that use an existing host bridge.
src/libxl/libxl_conf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index cec37d6..9c453d8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -874,6 +874,7 @@ libxlMakeNic(virDomainDefPtr def,
libxl_device_nic *x_nic)
{
bool ioemu_nic = STREQ(def->os.type, "hvm");
+ virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
/* TODO: Where is mtu stored?
*
@@ -899,16 +900,60 @@ libxlMakeNic(virDomainDefPtr def,
if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
return -1;
- switch (l_nic->type) {
+ switch (actual_type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
- if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
+ if (VIR_STRDUP(x_nic->bridge,
+ virDomainNetGetActualBridgeName(l_nic)) < 0)
return -1;
/* fallthrough */
case VIR_DOMAIN_NET_TYPE_ETHERNET:
if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
return -1;
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ {
+ bool fail = false;
+ char *brname = NULL;
+ virNetworkPtr network;
+ virConnectPtr conn;
+ virErrorPtr errobj;
+
+ if (!(conn = virConnectOpen("xen:///system")))
+ return -1;
+
+ if (!(network =
+ virNetworkLookupByName(conn, l_nic->data.network.name))) {
+ virObjectUnref(conn);
+ return -1;
+ }
+
+ if ((brname = virNetworkGetBridgeName(network))) {
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ fail = true;
+ } else {
+ fail = true;
+ }
+
+ VIR_FREE(brname);
+
+ /* Preserve any previous failure */
+ errobj = virSaveLastError();
+ virNetworkFree(network);
+ virSetError(errobj);
+ virFreeError(errobj);
+ virObjectUnref(conn);
+ if (fail)
+ return -1;
+ 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_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support network device type %s"),
virDomainNetTypeToString(l_nic->type));
--
1.8.4.5
3:35 a.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: support interface type=network
On 06/11/2014 12:25 AM, Jim Fehlig wrote:
Add support for <interface type='network'> in the libxl
driver.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
V2:
Support both libvirt's traditional managed networks and libvirt
networks that use an existing host bridge.
src/libxl/libxl_conf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index cec37d6..9c453d8 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -874,6 +874,7 @@ libxlMakeNic(virDomainDefPtr def,
libxl_device_nic *x_nic)
{
bool ioemu_nic = STREQ(def->os.type, "hvm");
+ virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
/* TODO: Where is mtu stored?
*
@@ -899,16 +900,60 @@ libxlMakeNic(virDomainDefPtr def,
if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
return -1;
- switch (l_nic->type) {
+ switch (actual_type) {
case VIR_DOMAIN_NET_TYPE_BRIDGE:
- if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
+ if (VIR_STRDUP(x_nic->bridge,
+ virDomainNetGetActualBridgeName(l_nic)) < 0)
return -1;
/* fallthrough */
case VIR_DOMAIN_NET_TYPE_ETHERNET:
if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
return -1;
break;
- default:
+ case VIR_DOMAIN_NET_TYPE_NETWORK:
+ {
+ bool fail = false;
+ char *brname = NULL;
+ virNetworkPtr network;
+ virConnectPtr conn;
+ virErrorPtr errobj;
+
+ if (!(conn = virConnectOpen("xen:///system")))
+ return -1;
(The conn is necessary in order to call the functions that retrieve the
bridge device name for the network)
One difference in the way that qemu deals with this is that in most
cases the higher level function already has a conn ptr, and that is
passed down the call stack to the user. There is one case where this
doesn't happen - the autostart of a domain. In that case, qemu calls
virConnectOpen(cfg->uri) to get a conn.
In this case, you are always creating a new conn, even when one may
already exist a few layers up the call stack, and the conn you create is
using the hardcoded "xen:///system" rather than taking something from
config.
As long as this is okay with you, it's okay with me. I just wonder about
two things:
1) does xen support non-privileged libvirt, and if so what would happen
in this case (I suppose it would just fail due to lack of authorization).
2) is the uri configurable anywhere, as it is for libvirt?
+
+ if (!(network =
+ virNetworkLookupByName(conn, l_nic->data.network.name))) {
+ virObjectUnref(conn);
+ return -1;
+ }
+
+ if ((brname = virNetworkGetBridgeName(network))) {
+ if (VIR_STRDUP(x_nic->bridge, brname) < 0)
+ fail = true;
+ } else {
+ fail = true;
+ }
+
+ VIR_FREE(brname);
+
+ /* Preserve any previous failure */
+ errobj = virSaveLastError();
+ virNetworkFree(network);
+ virSetError(errobj);
+ virFreeError(errobj);
+ virObjectUnref(conn);
+ if (fail)
+ return -1;
+ 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_DIRECT:
+ case VIR_DOMAIN_NET_TYPE_HOSTDEV:
+ case VIR_DOMAIN_NET_TYPE_LAST:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("libxenlight does not support network device type %s"),
virDomainNetTypeToString(l_nic->type));
ACK, pending the answers to the above questions.
4:41 p.m.
New subject: [libvirt] [PATCH V2 1/2] libxl: support interface type=network
Laine Stump wrote:
On 06/11/2014 12:25 AM, Jim Fehlig wrote:
> Add support for <interface type='network'> in the libxl driver.
>
> Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
> ---
>
> V2:
> Support both libvirt's traditional managed networks and libvirt
> networks that use an existing host bridge.
>
> src/libxl/libxl_conf.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 48 insertions(+), 3 deletions(-)
>
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index cec37d6..9c453d8 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -874,6 +874,7 @@ libxlMakeNic(virDomainDefPtr def,
> libxl_device_nic *x_nic)
> {
> bool ioemu_nic = STREQ(def->os.type, "hvm");
> + virDomainNetType actual_type = virDomainNetGetActualType(l_nic);
>
> /* TODO: Where is mtu stored?
> *
> @@ -899,16 +900,60 @@ libxlMakeNic(virDomainDefPtr def,
> if (VIR_STRDUP(x_nic->ifname, l_nic->ifname) < 0)
> return -1;
>
> - switch (l_nic->type) {
> + switch (actual_type) {
> case VIR_DOMAIN_NET_TYPE_BRIDGE:
> - if (VIR_STRDUP(x_nic->bridge, l_nic->data.bridge.brname) < 0)
> + if (VIR_STRDUP(x_nic->bridge,
> + virDomainNetGetActualBridgeName(l_nic)) < 0)
> return -1;
> /* fallthrough */
> case VIR_DOMAIN_NET_TYPE_ETHERNET:
> if (VIR_STRDUP(x_nic->script, l_nic->script) < 0)
> return -1;
> break;
> - default:
> + case VIR_DOMAIN_NET_TYPE_NETWORK:
> + {
> + bool fail = false;
> + char *brname = NULL;
> + virNetworkPtr network;
> + virConnectPtr conn;
> + virErrorPtr errobj;
> +
> + if (!(conn = virConnectOpen("xen:///system")))
> + return -1;
>
(The conn is necessary in order to call the functions that retrieve the
bridge device name for the network)
One difference in the way that qemu deals with this is that in most
cases the higher level function already has a conn ptr, and that is
passed down the call stack to the user. There is one case where this
doesn't happen - the autostart of a domain. In that case, qemu calls
virConnectOpen(cfg->uri) to get a conn.
Right, and that depends on whether running privileged or not. From
qemu_conf.c, virQEMUDriverConfigNew:
cfg->uri = privileged ? "qemu:///system" : "qemu:///session";
In this case, you are always creating a new conn, even when one may
already exist a few layers up the call stack,
I originally looked into passing conn to libxlMakeNic, but it is
actually quite a bit of code refactoring. I think that should be done
in conjunction with decomposing libxlDomainStart a bit. Currently that
function builds the domain config and handles the details of starting
the domain. I'd like to pull the build part out, allowing a cleaner
approach to passing conn where needed. E.g. I could see conn being used
in libxlMakeDisk some day.
and the conn you create is
using the hardcoded "xen:///system" rather than taking something from
config.
As long as this is okay with you, it's okay with me. I just wonder about
two things:
I think it is okay.
1) does xen support non-privileged libvirt, and if so what would
happen
in this case (I suppose it would just fail due to lack of authorization).
Only supports privileged.
2) is the uri configurable anywhere, as it is for libvirt?
Do you mean s/libvirt/the qemu driver/ ? As mentioned above, cfg->uri
depends on privileged in the qemu driver? For the libxl driver,
privileged is always true.
> +
> + if (!(network =
> + virNetworkLookupByName(conn, l_nic->data.network.name))) {
> + virObjectUnref(conn);
> + return -1;
> + }
> +
> + if ((brname = virNetworkGetBridgeName(network))) {
> + if (VIR_STRDUP(x_nic->bridge, brname) < 0)
> + fail = true;
> + } else {
> + fail = true;
> + }
> +
> + VIR_FREE(brname);
> +
> + /* Preserve any previous failure */
> + errobj = virSaveLastError();
> + virNetworkFree(network);
> + virSetError(errobj);
> + virFreeError(errobj);
> + virObjectUnref(conn);
> + if (fail)
> + return -1;
> + 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_DIRECT:
> + case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> + case VIR_DOMAIN_NET_TYPE_LAST:
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("libxenlight does not support network device type
%s"),
> virDomainNetTypeToString(l_nic->type));
>
ACK, pending the answers to the above questions.
Thanks for asking the questions, which made me rethink this and feel
confident it is okay :-). I'll push this, along with 2/2.
Regards,
Jim
4:25 p.m.
New subject: [libvirt] [PATCH V2 2/2] libxl: limit support for specifying an interface script
Generally, <interface> ... <script> is only supported for
type='ethernet'. Due to the long and pervasive use of
<interface type='bridge'>
...
<script path='foo'/>
</interface>
in Xen domain configuration, it was agreed to allow the use
of <script> with type='bridge' for backwards compatibility. See
the following discussion thread
http://www.redhat.com/archives/libvir-list/2013-April/msg00755.html
This patch limits the use of <script> to interface types ethernet
and bridge, raising an unsupported config error if <script> is
specified for all other interface types.
While at it, use VIR_ERR_CONFIG_UNSUPPORTED instead of
VIR_ERR_INTERNAL_ERROR when reporting unsupported interface types.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 9c453d8..6a025f0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -881,6 +881,14 @@ libxlMakeNic(virDomainDefPtr def,
* x_nics[i].mtu = 1492;
*/
+ if (l_nic->script && !(actual_type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actual_type == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("specifying a script is only supported with "
+ "interface types bridge and ethernet"));
+ return -1;
+ }
+
libxl_device_nic_init(x_nic);
virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
@@ -954,8 +962,8 @@ libxlMakeNic(virDomainDefPtr def,
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
case VIR_DOMAIN_NET_TYPE_LAST:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("libxenlight does not support network device type %s"),
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported interface type %s"),
virDomainNetTypeToString(l_nic->type));
return -1;
}
--
1.8.4.5
3:36 a.m.
New subject: [libvirt] [PATCH V2 2/2] libxl: limit support for specifying an interface script
On 06/11/2014 12:25 AM, Jim Fehlig wrote:
Generally, <interface> ... <script> is only supported
for
type='ethernet'. Due to the long and pervasive use of
<interface type='bridge'>
...
<script path='foo'/>
</interface>
in Xen domain configuration, it was agreed to allow the use
of <script> with type='bridge' for backwards compatibility. See
the following discussion thread
http://www.redhat.com/archives/libvir-list/2013-April/msg00755.html
This patch limits the use of <script> to interface types ethernet
and bridge, raising an unsupported config error if <script> is
specified for all other interface types.
While at it, use VIR_ERR_CONFIG_UNSUPPORTED instead of
VIR_ERR_INTERNAL_ERROR when reporting unsupported interface types.
Signed-off-by: Jim Fehlig <jfehlig(a)suse.com>
---
src/libxl/libxl_conf.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 9c453d8..6a025f0 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -881,6 +881,14 @@ libxlMakeNic(virDomainDefPtr def,
* x_nics[i].mtu = 1492;
*/
+ if (l_nic->script && !(actual_type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ actual_type == VIR_DOMAIN_NET_TYPE_ETHERNET)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("specifying a script is only supported with "
+ "interface types bridge and ethernet"));
+ return -1;
+ }
+
libxl_device_nic_init(x_nic);
virMacAddrGetRaw(&l_nic->mac, x_nic->mac);
@@ -954,8 +962,8 @@ libxlMakeNic(virDomainDefPtr def,
case VIR_DOMAIN_NET_TYPE_DIRECT:
case VIR_DOMAIN_NET_TYPE_HOSTDEV:
case VIR_DOMAIN_NET_TYPE_LAST:
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("libxenlight does not support network device type %s"),
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported interface type %s"),
virDomainNetTypeToString(l_nic->type));
return -1;
}
ACK.
4:23 p.m.
Jim Fehlig wrote:
Patch 1 is a V2 of
http://www.redhat.com/archives/libvir-list/2014-June/msg00382.html
Patch 2 is a result of Laine's comments to V1. It ensures <script>
is only allowed with interface types ethernet and bridge.
Jim Fehlig (2):
libxl: support interface type=network
libxl: limit support for specifying an interface script
src/libxl/libxl_conf.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 5 deletions(-)
Any comments on this version? Thanks!
Regards,
Jim
3809
days inactive
3818
days old
6 comments
2 participants
participants (2)
-
Jim Fehlig -
Laine Stump