[libvirt] [PATCH] Xen: Do not generate net ifname if domain is inactive

The xend driver will generate a virDomainNetDef ifname if one is not specified in xend sexpr, even if domain is inactive. The result is network interface XML containing 'vif-1.Y' on dev attribute of target element, e.g. <interface type='bridge'> <target dev='vif-1.0'/> ... This patch changes the behavior to only generate the ifname if not specified in xend sexpr *and* domain is not inactive (id != -1). --- src/xenxs/xen_sxpr.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/xenxs/xen_sxpr.c b/src/xenxs/xen_sxpr.c index d2ec370..66df786 100644 --- a/src/xenxs/xen_sxpr.c +++ b/src/xenxs/xen_sxpr.c @@ -547,12 +547,17 @@ xenParseSxprNets(virDomainDefPtr def, } tmp = sexpr_node(node, "device/vif/vifname"); - if (!tmp) { + /* If vifname is specified in xend config, include it in net + * definition regardless of domain state. If vifname is not + * specified, only generate one if domain is active (id != -1). */ + if (tmp) { + if (!(net->ifname = strdup(tmp))) + goto no_memory; + } else if (def->id != -1) { snprintf(buf, sizeof(buf), "vif%d.%d", def->id, vif_index); - tmp = buf; + if (!(net->ifname = strdup(buf))) + goto no_memory; } - if (!(net->ifname = strdup(tmp))) - goto no_memory; tmp = sexpr_node(node, "device/vif/mac"); if (tmp) { -- 1.7.3.1

On 04/28/2011 11:53 AM, Jim Fehlig wrote:
The xend driver will generate a virDomainNetDef ifname if one is not specified in xend sexpr, even if domain is inactive. The result is network interface XML containing 'vif-1.Y' on dev attribute of target element, e.g.
<interface type='bridge'> <target dev='vif-1.0'/> ...
This patch changes the behavior to only generate the ifname if not specified in xend sexpr *and* domain is not inactive (id != -1).
Makes sense for the rationale. However...
+ } else if (def->id != -1) { snprintf(buf, sizeof(buf), "vif%d.%d", def->id, vif_index); - tmp = buf; + if (!(net->ifname = strdup(buf)))
Rather than using snprintf and strdup, can't we just use: if (virAsprintf(&net->ifname, "vif%d.%d", def->id, vif_index) < 0) goto no_memory; -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Eric Blake wrote:
On 04/28/2011 11:53 AM, Jim Fehlig wrote:
The xend driver will generate a virDomainNetDef ifname if one is not specified in xend sexpr, even if domain is inactive. The result is network interface XML containing 'vif-1.Y' on dev attribute of target element, e.g.
<interface type='bridge'> <target dev='vif-1.0'/> ...
This patch changes the behavior to only generate the ifname if not specified in xend sexpr *and* domain is not inactive (id != -1).
Makes sense for the rationale. However...
+ } else if (def->id != -1) { snprintf(buf, sizeof(buf), "vif%d.%d", def->id, vif_index); - tmp = buf; + if (!(net->ifname = strdup(buf)))
Rather than using snprintf and strdup, can't we just use:
if (virAsprintf(&net->ifname, "vif%d.%d", def->id, vif_index) < 0) goto no_memory;
Yes, good point. Here is a V2. Regards, Jim

On 04/28/2011 02:45 PM, Jim Fehlig wrote:
Rather than using snprintf and strdup, can't we just use:
if (virAsprintf(&net->ifname, "vif%d.%d", def->id, vif_index) < 0) goto no_memory;
Yes, good point. Here is a V2.
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Jim Fehlig